Attachment 372086 Details for Bug 198797 – Patch

[patch] Patch

bug-198797-20190613171739.patch (text/plain), 16.30 KB, created by Brent Fulgham on 2019-06-13 17:17:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2019-06-13 17:17:41 PDT

Size: 16.30 KB

patch

obsolete

>Subversion Revision: 246374
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index baf2ae92aec8ddcb6db2e4315058df8d6104cf50..b2584b48d7c5c4ac7aa59ce9d094860e120a2c60 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-06-13  Brent Fulgham  <bfulgham@apple.com>
>+
>+        Ensure ITP state is relayed to Network Process on restart
>+        https://bugs.webkit.org/show_bug.cgi?id=198797
>+        <rdar://problem/51646944>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Now that the ITP state is maintained in the Network Process, we have to make sure that we update
>+        that process with current ITP state if the Network Process crashes and is relaunched. This wasn't a
>+        problem in earlier releases because we tracked all ITP state in the UIProcess.
>+
>+        This patch does the following:
>+        1. Add a new method to WKWebsiteDataStore to allow us to trigger statistics processing, which has the
>+           side effect of syncing ITP state persistently so that it will be available after bouncing the process.
>+        2. Adds a new test that sets a tracking domain, bounces the process, and confirms the state is still
>+           consistent.
>+
>+        Tested by TestWebKitAPI.
>+
>+        * NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:
>+        (WebKit::ResourceLoadStatisticsStore::processStatisticsAndDataRecords):
>+        * UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
>+        (-[WKWebsiteDataStore _clearPrevalentDomain:completionHandler:]):
>+        (-[WKWebsiteDataStore _processStatisticsAndDataRecords:]):
>+        * UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::ensureNetworkProcess):
>+        (WebKit::WebProcessPool::setResourceLoadStatisticsEnabled):
>+        * UIProcess/WebProcessPool.h:
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::setResourceLoadStatisticsEnabled):
>+
> 2019-06-12  Ludovico de Nittis  <ludovico.denittis@collabora.com>
> 
>         [GTK] GTK_STOCK_* types have been deprecated since GTK 3.10
>diff --git a/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp b/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp
>index b7d25dec302905be36c511834e549b3db29ba156..f8078b3083d57a6f7b41652ddabfa537b82ed072 100644
>--- a/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp
>+++ b/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp
>@@ -246,6 +246,8 @@ void ResourceLoadStatisticsStore::processStatisticsAndDataRecords()
>         pruneStatisticsIfNeeded();
>         syncStorageIfNeeded();
> 
>+        logTestingEvent("Storage Synced"_s);
>+
>         if (!m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned)
>             return;
> 
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>index 26aea09ea4b4d6df3da194192a5277cc47d40ccd..75331c2688600a9f4459e5b161a14bfd33b907c1 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -502,6 +502,28 @@ - (void)_getIsPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(BOOL))
> #endif
> }
> 
>+- (void)_clearPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(void))completionHandler
>+{
>+#if ENABLE(RESOURCE_LOAD_STATISTICS)
>+    _websiteDataStore->websiteDataStore().clearPrevalentResource(URL(domain), [completionHandler = makeBlockPtr(completionHandler)]() {
>+        completionHandler();
>+    });
>+#else
>+    completionHandler();
>+#endif
>+}
>+
>+- (void)_processStatisticsAndDataRecords:(void (^)(void))completionHandler
>+{
>+#if ENABLE(RESOURCE_LOAD_STATISTICS)
>+    _websiteDataStore->websiteDataStore().scheduleStatisticsAndDataRecordsProcessing([completionHandler = makeBlockPtr(completionHandler)]() {
>+        completionHandler();
>+    });
>+#else
>+    completionHandler();
>+#endif
>+}
>+
> - (bool)_hasRegisteredServiceWorker
> {
>     return WebKit::ServiceWorkerProcessProxy::hasRegisteredServiceWorkers(_websiteDataStore->websiteDataStore().serviceWorkerRegistrationDirectory());
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>index 237a3704ece8b9c2cb8b01c270b38fb2612e4cad..0c9527af4ea034beb798752d01680cce280a9088 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -69,6 +69,8 @@ typedef NS_OPTIONS(NSUInteger, _WKWebsiteDataStoreFetchOptions) {
> 
> - (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));
>+- (void)_clearPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(void))completionHandler;
>+- (void)_processStatisticsAndDataRecords:(void (^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> @property (nullable, nonatomic, weak) id <_WKWebsiteDataStoreDelegate> _delegate WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 43d7700e5f7f467bebbd99a9eed1577c7b44b21d..7b0650462174902e0678d6247150614e7e494f67 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -582,7 +582,7 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
> 
>     SandboxExtension::createHandleForReadWriteDirectory(parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory, parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectoryExtensionHandle);
> 
>-    bool enableResourceLoadStatistics = false;
>+    bool enableResourceLoadStatistics = m_itpIsEnabled;
>     bool shouldIncludeLocalhost = true;
>     bool enableResourceLoadStatisticsDebugMode = false;
>     WebCore::RegistrableDomain manualPrevalentResource { };
>@@ -1487,6 +1487,7 @@ void WebProcessPool::setShouldUseFontSmoothing(bool useFontSmoothing)
> 
> void WebProcessPool::setResourceLoadStatisticsEnabled(bool enabled)
> {
>+    m_itpIsEnabled = enabled;
>     sendToAllProcesses(Messages::WebProcess::SetResourceLoadStatisticsEnabled(enabled));
> #if ENABLE(RESOURCE_LOAD_STATISTICS)
>     sendToNetworkingProcess(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(enabled));
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
>index 1d29825c834f104ed1979a4001a250aea6b6e614..a79b1cf424afb2276da11530dfcf3d097976d0a4 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.h
>+++ b/Source/WebKit/UIProcess/WebProcessPool.h
>@@ -727,6 +727,7 @@ private:
>     bool m_shouldTakeUIBackgroundAssertion;
>     bool m_shouldMakeNextWebProcessLaunchFailForTesting { false };
>     bool m_shouldMakeNextNetworkProcessLaunchFailForTesting { false };
>+    bool m_itpIsEnabled { false };
> 
>     UserObservablePageCounter m_userObservablePageCounter;
>     ProcessSuppressionDisabledCounter m_processSuppressionDisabledForPageCounter;
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index c28a640f7773d40e2089094db68ca13ba76e3f52..83e3a1f0ae84bd0921b3987084d7ed481a99c92f 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -1875,6 +1875,8 @@ void WebsiteDataStore::setResourceLoadStatisticsEnabled(bool enabled)
>         processPool->setResourceLoadStatisticsEnabled(false);
>         processPool->sendToNetworkingProcess(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(false));
>     }
>+
>+    m_resourceLoadStatisticsEnabled = false;
> #else
>     UNUSED_PARAM(enabled);
> #endif
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index e1ab018beccdf46679dbf9426a8698bc512e3a61..700ff6f19e68db63e6772962dc3d69664ba7afbb 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-06-13  Brent Fulgham  <bfulgham@apple.com>
>+
>+        Ensure ITP state is relayed to Network Process on restart
>+        https://bugs.webkit.org/show_bug.cgi?id=198797
>+        <rdar://problem/51646944>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:
>+        (cleanupITPDatabase): Added.
>+        (TEST:EnableDisableITP): Update to use cleanup method.
>+        (TEST:NetworkProcessRestart): Added.
>+
> 2019-06-12  Aakash Jain  <aakash_jain@apple.com>
> 
>         Replace double-quotes with single quotes in loadConfig.py
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>index e1509585ce847677ce52b143788e0394d2b0903c..24e82c8be2fcd92e0d66843fb9416505e12aa1c9 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>@@ -206,10 +206,43 @@ TEST(ResourceLoadStatistics, IPCAfterStoreDestruction)
>     TestWebKitAPI::Util::run(&finishedNavigation);
> }
> 
>+static void cleanupITPDatabase(WKWebsiteDataStore *dataStore)
>+{
>+    [dataStore _setResourceLoadStatisticsEnabled:YES];
>+
>+    // Make sure 'evil.com' is not in our data set.
>+    static bool doneFlag;
>+    [dataStore _clearPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(void) {
>+        doneFlag = true;
>+    }];
>+    
>+    static bool dataSyncCompleted;
>+    [dataStore _setResourceLoadStatisticsTestingCallback:^(WKWebsiteDataStore *, NSString *message) {
>+        if (![message isEqualToString:@"Storage Synced"])
>+            return;
>+
>+        dataSyncCompleted = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&doneFlag);
>+
>+    // Trigger ITP to process its data to force a sync to persistent storage.
>+    [dataStore _processStatisticsAndDataRecords: ^(void) {
>+        doneFlag = true;
>+    }];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+    TestWebKitAPI::Util::run(&dataSyncCompleted);
>+    
>+    TestWebKitAPI::Util::spinRunLoop(1);
>+
>+    [dataStore _setResourceLoadStatisticsEnabled:NO];
>+}
>+
> TEST(ResourceLoadStatistics, EnableDisableITP)
> {
>     // Ensure the shared process pool exists so the data store operations we're about to do work with it.
>-    WKProcessPool *sharedProcessPool = [WKProcessPool _sharedProcessPool];
>+    auto *sharedProcessPool = [WKProcessPool _sharedProcessPool];
>     auto *dataStore = [WKWebsiteDataStore defaultDataStore];
> 
>     auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>@@ -221,6 +254,8 @@ TEST(ResourceLoadStatistics, EnableDisableITP)
>     [webView loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>     [webView _test_waitForDidFinishNavigation];
> 
>+    cleanupITPDatabase(dataStore);
>+
>     // ITP should be off, no URLs are prevalent.
>     static bool doneFlag;
>     [dataStore _getIsPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(BOOL prevalent) {
>@@ -281,3 +316,123 @@ TEST(ResourceLoadStatistics, EnableDisableITP)
>     TestWebKitAPI::Util::run(&doneFlag);
> }
> 
>+TEST(ResourceLoadStatistics, NetworkProcessRestart)
>+{
>+    // Ensure the shared process pool exists so the data store operations we're about to do work with it.
>+    auto *sharedProcessPool = [WKProcessPool _sharedProcessPool];
>+    auto *dataStore = [WKWebsiteDataStore defaultDataStore];
>+
>+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [configuration setProcessPool: sharedProcessPool];
>+    configuration.get().websiteDataStore = dataStore;
>+    
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    
>+    [webView loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView _test_waitForDidFinishNavigation];
>+
>+    cleanupITPDatabase(dataStore);
>+
>+    // Turn it on
>+    [dataStore _setResourceLoadStatisticsEnabled:YES];
>+    
>+    [webView loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView _test_waitForDidFinishNavigation];
>+    
>+    // ITP should be on, but nothing was registered as prevalent yet.
>+    static bool doneFlag;
>+    [dataStore _getIsPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(BOOL prevalent) {
>+        EXPECT_FALSE(prevalent);
>+        doneFlag = true;
>+    }];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+    
>+    // Teach ITP about a bad origin:
>+    doneFlag = false;
>+    [dataStore _setPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(void) {
>+        doneFlag = true;
>+    }];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+    
>+    [webView loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView _test_waitForDidFinishNavigation];
>+    
>+    // ITP should be on, and know about 'evil.com'
>+    doneFlag = false;
>+    [dataStore _getIsPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(BOOL prevalent) {
>+        EXPECT_TRUE(prevalent);
>+        doneFlag = true;
>+    }];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+
>+    static bool dataSyncCompleted;
>+    [dataStore _setResourceLoadStatisticsTestingCallback:^(WKWebsiteDataStore *, NSString *message) {
>+        if (![message isEqualToString:@"Storage Synced"])
>+            return;
>+        dataSyncCompleted = true;
>+    }];
>+
>+    // Tell ITP to process data so it will sync the new 'bad guy' to persistent storage:
>+    [dataStore _processStatisticsAndDataRecords: ^(void) {
>+        doneFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&doneFlag);
>+    TestWebKitAPI::Util::run(&dataSyncCompleted);
>+
>+    TestWebKitAPI::Util::spinRunLoop(1);
>+
>+    [configuration.get().processPool _terminateNetworkProcess];
>+
>+    auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+
>+    [webView2 loadHTMLString:@"WebKit Test 2" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView2 _test_waitForDidFinishNavigation];
>+
>+    // ITP should be on, and know about 'evil.com'
>+    doneFlag = false;
>+    [dataStore _getIsPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(BOOL prevalent) {
>+        EXPECT_TRUE(prevalent);
>+        doneFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&doneFlag);
>+
>+    // Turn it off
>+    [dataStore _setResourceLoadStatisticsEnabled:NO];
>+    
>+    [webView loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView _test_waitForDidFinishNavigation];
>+    
>+    // ITP should be off, no URLs are prevalent.
>+    doneFlag = false;
>+    [dataStore _getIsPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(BOOL prevalent) {
>+        EXPECT_FALSE(prevalent);
>+        doneFlag = true;
>+    }];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+
>+    TestWebKitAPI::Util::spinRunLoop(1);
>+    
>+    [configuration.get().processPool _terminateNetworkProcess];
>+    
>+    auto webView3 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    
>+    [webView3 loadHTMLString:@"WebKit Test 3" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView3 _test_waitForDidFinishNavigation];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+    
>+    // ITP should still be off, and should not know about 'evil.com'
>+    doneFlag = false;
>+    [dataStore _getIsPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(BOOL prevalent) {
>+        EXPECT_FALSE(prevalent);
>+        doneFlag = true;
>+    }];
>+    
>+    TestWebKitAPI::Util::run(&doneFlag);
>+}

Flags:

youennf: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 198797: 372004 | 372023 | 372071 | 372086