Attachment 372135 Details for Bug 198814 – Patch

[patch] Patch

bug-198814-20190614123331.patch (text/plain), 10.46 KB, created by youenn fablet on 2019-06-14 12:33:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-06-14 12:33:32 PDT

Size: 10.46 KB

patch

obsolete

>Subversion Revision: 246434
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index be102fd6f527c897fee6b4ee4f6da1a16187e712..5475ae5604acbebbb79072d875280cae4d897867 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-06-14  Youenn Fablet  <youenn@apple.com>
>+
>+        WebResourceLoadStatisticsStore should not use its network session if invalidated
>+        https://bugs.webkit.org/show_bug.cgi?id=198814
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tell WebResourceLoadStatisticsStore that its network session is invalidated.
>+        WebResourceLoadStatisticsStore will then clear its reference to the network session.
>+
>+        * NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp:
>+        (WebKit::ResourceLoadStatisticsMemoryStore::updateCookieBlocking):
>+        Added for test purposes to trigger further cookie processing.
>+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
>+        (WebKit::WebResourceLoadStatisticsStore::invalidateAndCancel):
>+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
>+        * NetworkProcess/NetworkSession.cpp:
>+        (WebKit::NetworkSession::invalidateAndCancel):
>+
> 2019-06-13  Megan Gardner  <megan_gardner@apple.com>
> 
>         Fix selection hang in Mail
>diff --git a/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp b/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp
>index a5f2624e4334ce9039faa5e2d1e5c41fd0d087c6..b8ee09578a3064ee8e63ffa277bc5182c8720c26 100644
>--- a/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp
>+++ b/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp
>@@ -758,7 +758,7 @@ void ResourceLoadStatisticsMemoryStore::updateCookieBlocking(CompletionHandler<v
>             domainsToBlock.append(resourceStatistic.registrableDomain);
>     }
> 
>-    if (domainsToBlock.isEmpty()) {
>+    if (domainsToBlock.isEmpty() && !debugModeEnabled()) {
>         completionHandler();
>         return;
>     }
>diff --git a/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp b/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp
>index 914d35bde8a003dae0d61b06491a08b720c0bdaa..b668df23075ba1728b7f4b6b53b21ab9bf78b3fc 100644
>--- a/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp
>+++ b/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp
>@@ -1021,6 +1021,11 @@ NetworkSession* WebResourceLoadStatisticsStore::networkSession()
>     return m_networkSession.get();
> }
> 
>+void WebResourceLoadStatisticsStore::invalidateAndCancel()
>+{
>+    m_networkSession = nullptr;
>+}
>+
> void WebResourceLoadStatisticsStore::deleteWebsiteDataForRegistrableDomains(OptionSet<WebsiteDataType> dataTypes, HashMap<RegistrableDomain, WebsiteDataToRemove>&& domainsToRemoveWebsiteDataFor, bool shouldNotifyPage, CompletionHandler<void(const HashSet<RegistrableDomain>&)>&& completionHandler)
> {
>     ASSERT(RunLoop::isMain());
>diff --git a/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h b/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h
>index a8c23e09b3f727848c11a77a7ee5f1925573f5b3..f19a73d6fbc2ea258bc998ead5d2275c2fca09a1 100644
>--- a/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h
>+++ b/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h
>@@ -176,6 +176,7 @@ public:
>     void notifyResourceLoadStatisticsProcessed();
> 
>     NetworkSession* networkSession();
>+    void invalidateAndCancel();
> 
>     void sendDiagnosticMessageWithValue(const String& message, const String& description, unsigned value, unsigned sigDigits, WebCore::ShouldSample) const;
>     void notifyPageStatisticsTelemetryFinished(unsigned totalPrevalentResources, unsigned totalPrevalentResourcesWithUserInteraction, unsigned top3SubframeUnderTopFrameOrigins) const;
>diff --git a/Source/WebKit/NetworkProcess/NetworkSession.cpp b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>index 3544fa4cc5f83e9744f4dff36415e2caa42a459e..8d4626edecaf4f9f2596e467c6342e2801659883 100644
>--- a/Source/WebKit/NetworkProcess/NetworkSession.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>@@ -100,11 +100,19 @@ void NetworkSession::invalidateAndCancel()
> {
>     for (auto* task : m_dataTaskSet)
>         task->invalidateAndCancel();
>+#if ENABLE(RESOURCE_LOAD_STATISTICS)
>+    if (m_resourceLoadStatistics)
>+        m_resourceLoadStatistics->invalidateAndCancel();
>+#endif
>+#if !ASSERT_DISABLED
>+    m_isInvalidated = true;
>+#endif
> }
> 
> #if ENABLE(RESOURCE_LOAD_STATISTICS)
> void NetworkSession::setResourceLoadStatisticsEnabled(bool enable)
> {
>+    ASSERT(!m_isInvalidated);
>     if (!enable) {
>         m_resourceLoadStatistics = nullptr;
>         return;
>diff --git a/Source/WebKit/NetworkProcess/NetworkSession.h b/Source/WebKit/NetworkProcess/NetworkSession.h
>index f9d22ab0aeb8a7b8f1ff7a86c266f1d3d9ba74e9..d97775fe6b319bb48b302e7218118739bc9bffa8 100644
>--- a/Source/WebKit/NetworkProcess/NetworkSession.h
>+++ b/Source/WebKit/NetworkProcess/NetworkSession.h
>@@ -127,6 +127,9 @@ protected:
>     PrefetchCache m_prefetchCache;
> 
>     Ref<StorageManager> m_storageManager;
>+#if !ASSERT_DISABLED
>+    bool m_isInvalidated { false };
>+#endif
> };
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>index 26aea09ea4b4d6df3da194192a5277cc47d40ccd..af4218d8191290a6125934be924454c2c8eda2ee 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>@@ -480,6 +480,17 @@ - (void)_getAllStorageAccessEntriesFor:(WKWebView *)webView completionHandler:(v
> #endif
> }
> 
>+- (void)_scheduleCookieBlockingUpdate:(void (^)(void))completionHandler
>+{
>+#if ENABLE(RESOURCE_LOAD_STATISTICS)
>+    _websiteDataStore->websiteDataStore().scheduleCookieBlockingUpdate([completionHandler = makeBlockPtr(completionHandler)]() {
>+        completionHandler();
>+    });
>+#else
>+    completionHandler();
>+#endif
>+}
>+
> - (void)_setPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(void))completionHandler
> {
> #if ENABLE(RESOURCE_LOAD_STATISTICS)
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>index 237a3704ece8b9c2cb8b01c270b38fb2612e4cad..e52d93f0fe9e5444eb206720cf57944ba9bdca8a 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>@@ -67,6 +67,7 @@ typedef NS_OPTIONS(NSUInteger, _WKWebsiteDataStoreFetchOptions) {
> + (void)_allowWebsiteDataRecordsForAllOrigins WK_API_AVAILABLE(macos(10.13.4), ios(11.3));
> - (bool)_hasRegisteredServiceWorker WK_API_AVAILABLE(macos(10.14), ios(12.0));
> 
>+- (void)_scheduleCookieBlockingUpdate:(void (^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> - (void)_setPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> - (void)_getIsPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(BOOL))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index e3aa4e9379b70b74b726ca9930cf354aaef9b7ce..47441f34f9b606388d6e25e1aeea2bfbf4e7c782 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-06-14  Youenn Fablet  <youenn@apple.com>
>+
>+        WebResourceLoadStatisticsStore should not use its network session if invalidated
>+        https://bugs.webkit.org/show_bug.cgi?id=198814
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:
>+        (TEST):
>+
> 2019-06-14  Youenn Fablet  <youenn@apple.com>
> 
>         import-w3c-tests should respect WEBKIT_OUTPUTDIR
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>index e1509585ce847677ce52b143788e0394d2b0903c..d2b79c3b7e97ba240a58ed2b391810e563c9dbee 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>@@ -28,10 +28,12 @@
> #import "PlatformUtilities.h"
> #import "TestNavigationDelegate.h"
> #import <WebKit/WKFoundation.h>
>+#import <WebKit/WKPreferencesPrivate.h>
> #import <WebKit/WKProcessPoolPrivate.h>
> #import <WebKit/WKWebViewConfigurationPrivate.h>
> #import <WebKit/WKWebsiteDataRecordPrivate.h>
> #import <WebKit/WKWebsiteDataStorePrivate.h>
>+#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
> #import <wtf/RetainPtr.h>
> 
> static bool finishedNavigation = false;
>@@ -281,3 +283,37 @@ TEST(ResourceLoadStatistics, EnableDisableITP)
>     TestWebKitAPI::Util::run(&doneFlag);
> }
> 
>+TEST(ResourceLoadStatistics, RemoveSessionID)
>+{
>+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    auto websiteDataStoreConfiguration = adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]);
>+    configuration.get().websiteDataStore = [[[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()] autorelease];
>+
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+
>+    // We load a resource so that the NetworkSession stays alive a little bit longer after the session is removed.
>+
>+    [webView loadHTMLString:@"<a id='link' href='http://webkit.org' download>Click me!</a>" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView _test_waitForDidFinishNavigation];
>+
>+    static bool doneFlag = false;
>+    [webView evaluateJavaScript:@"document.getElementById('link').click();" completionHandler: ^(id, NSError*) {
>+        doneFlag = true;
>+    }];
>+    TestWebKitAPI::Util::run(&doneFlag);
>+
>+    [configuration.get().websiteDataStore _setResourceLoadStatisticsEnabled:YES];
>+    [configuration.get().websiteDataStore _setResourceLoadStatisticsDebugMode:YES];
>+
>+    // Trigger ITP tasks.
>+    [configuration.get().websiteDataStore _scheduleCookieBlockingUpdate: ^(void) { }];
>+    // Trigger removing of the sessionID.
>+    TestWebKitAPI::Util::spinRunLoop(2);
>+    [webView _close];
>+    webView = nullptr;
>+    configuration = nullptr;
>+
>+    auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>+    [webView2 loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView2 _test_waitForDidFinishNavigation];
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 198814: 372002 | 372009 | 372135