Attachment 372004 Details for Bug 198797 – Patch

[patch] Patch

bug-198797-20190612172256.patch (text/plain), 11.74 KB, created by Brent Fulgham on 2019-06-12 17:22:56 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2019-06-12 17:22:56 PDT

Size: 11.74 KB

patch

obsolete

>Subversion Revision: 246374
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index baf2ae92aec8ddcb6db2e4315058df8d6104cf50..724da6d47dfb075c1e2735fc8509997ed77c96e7 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-06-12  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.
>+
>+        * UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
>+        (-[WKWebsiteDataStore _processStatisticsAndDataRecords:]):
>+        * UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::ensureNetworkProcess):
>+        (WebKit::WebProcessPool::networkProcessCrashed):
>+        (WebKit::WebProcessPool::setResourceLoadStatisticsEnabled):
>+        * UIProcess/WebProcessPool.h:
>+
> 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/UIProcess/API/Cocoa/WKWebsiteDataStore.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>index 26aea09ea4b4d6df3da194192a5277cc47d40ccd..5ea7228c8432404b97594e3e7ae5c6f464c3e0eb 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
>@@ -502,6 +502,18 @@ - (void)_getIsPrevalentDomain:(NSURL *)domain completionHandler:(void (^)(BOOL))
> #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..2a53825a94e92c29027acce9f5cd50b8a830bf23 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h
>@@ -69,6 +69,7 @@ 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)_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..e1e3ab0932ff8339838c94e96522161784b5062f 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -631,6 +631,11 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
>     if (m_didNetworkProcessCrash) {
>         m_didNetworkProcessCrash = false;
>         reinstateNetworkProcessAssertionState(*m_networkProcess);
>+
>+#if ENABLE(RESOURCE_LOAD_STATISTICS)
>+        if (m_itpIsEnabled && !withWebsiteDataStore && !m_websiteDataStore)
>+            m_networkProcess->send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(true), 0);
>+#endif
>     }
> 
>     if (withWebsiteDataStore) {
>@@ -670,6 +675,12 @@ void WebProcessPool::networkProcessCrashed(NetworkProcessProxy& networkProcessPr
>     auto& newNetworkProcess = ensureNetworkProcess();
>     for (auto& reply : pendingReplies)
>         newNetworkProcess.getNetworkProcessConnection(*reply.first, WTFMove(reply.second));
>+
>+#if ENABLE(RESOURCE_LOAD_STATISTICS)
>+    // Make sure the newly-spawned NetworkProcess is in the right ITP state.
>+    if (m_itpIsEnabled && !m_websiteDataStore)
>+        newNetworkProcess.send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(true), 0);
>+#endif
> }
> 
> void WebProcessPool::getNetworkProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply&& reply)
>@@ -1487,6 +1498,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/Tools/ChangeLog b/Tools/ChangeLog
>index e1ab018beccdf46679dbf9426a8698bc512e3a61..6980f6178480d24a4e9c52fdce3b9867a65b4cb5 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-12  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:
>+        (TEST):
>+
> 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..e604e402d7357e89fec0ed315a0704d9632808d5 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm
>@@ -281,3 +281,113 @@ 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.
>+    WKProcessPool *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];
>+    
>+    // 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);
>+
>+    // 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::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);
>+}

Actions: View | Formatted Diff | Diff

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