[webkit-changes] [257904] trunk/Source/WebKit

Wed, 04 Mar 2020 23:07:26 -0800

Title: [257904] trunk/Source/WebKit
Revision
257904
Author
cdu...@apple.com
Date
2020-03-04 23:06:14 -0800 (Wed, 04 Mar 2020)

Log Message

WebsiteDataStore methods often create process pools and launch network processes unnecessarily
https://bugs.webkit.org/show_bug.cgi?id=208619

Reviewed by Alex Christensen.

These methods were potentially creating a temporary WebProcessPool object and launching a network
process only to go change something in memory in the network process. Since the newly created
WebProcessPool is temporary, it gets destroyed as soon as we get out of the for loop and the
network go away too. Therefore, the information in memory of the new network process would not
survive. Those methods should never be created a process pool and this patch fixes this.

* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::setMaxStatisticsEntries):
(WebKit::WebsiteDataStore::setPruneEntriesDownTo):
(WebKit::WebsiteDataStore::setGrandfatheringTime):
(WebKit::WebsiteDataStore::setMinimumTimeBetweenDataRecordsRemoval):
(WebKit::WebsiteDataStore::setPrevalentResource):
(WebKit::WebsiteDataStore::setVeryPrevalentResource):
(WebKit::WebsiteDataStore::setShouldClassifyResourcesBeforeDataRecordsRemoval):
(WebKit::WebsiteDataStore::setNotifyPagesWhenDataRecordsWereScanned):
(WebKit::WebsiteDataStore::setIsRunningResourceLoadStatisticsTest):
(WebKit::WebsiteDataStore::setNotifyPagesWhenTelemetryWasCaptured):
(WebKit::WebsiteDataStore::getAllStorageAccessEntries):
(WebKit::WebsiteDataStore::setTimeToLiveUserInteraction):
(WebKit::WebsiteDataStore::setCacheMaxAgeCapForPrevalentResources):
(WebKit::WebsiteDataStore::setResourceLoadStatisticsDebugMode):
(WebKit::WebsiteDataStore::clearResourceLoadStatisticsInWebProcesses):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (257903 => 257904)


--- trunk/Source/WebKit/ChangeLog	2020-03-05 07:00:19 UTC (rev 257903)
+++ trunk/Source/WebKit/ChangeLog	2020-03-05 07:06:14 UTC (rev 257904)
@@ -1,3 +1,33 @@
+2020-03-04  Chris Dumez  <cdu...@apple.com>
+
+        WebsiteDataStore methods often create process pools and launch network processes unnecessarily
+        https://bugs.webkit.org/show_bug.cgi?id=208619
+
+        Reviewed by Alex Christensen.
+
+        These methods were potentially creating a temporary WebProcessPool object and launching a network
+        process only to go change something in memory in the network process. Since the newly created
+        WebProcessPool is temporary, it gets destroyed as soon as we get out of the for loop and the
+        network go away too. Therefore, the information in memory of the new network process would not
+        survive. Those methods should never be created a process pool and this patch fixes this.
+
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::setMaxStatisticsEntries):
+        (WebKit::WebsiteDataStore::setPruneEntriesDownTo):
+        (WebKit::WebsiteDataStore::setGrandfatheringTime):
+        (WebKit::WebsiteDataStore::setMinimumTimeBetweenDataRecordsRemoval):
+        (WebKit::WebsiteDataStore::setPrevalentResource):
+        (WebKit::WebsiteDataStore::setVeryPrevalentResource):
+        (WebKit::WebsiteDataStore::setShouldClassifyResourcesBeforeDataRecordsRemoval):
+        (WebKit::WebsiteDataStore::setNotifyPagesWhenDataRecordsWereScanned):
+        (WebKit::WebsiteDataStore::setIsRunningResourceLoadStatisticsTest):
+        (WebKit::WebsiteDataStore::setNotifyPagesWhenTelemetryWasCaptured):
+        (WebKit::WebsiteDataStore::getAllStorageAccessEntries):
+        (WebKit::WebsiteDataStore::setTimeToLiveUserInteraction):
+        (WebKit::WebsiteDataStore::setCacheMaxAgeCapForPrevalentResources):
+        (WebKit::WebsiteDataStore::setResourceLoadStatisticsDebugMode):
+        (WebKit::WebsiteDataStore::clearResourceLoadStatisticsInWebProcesses):
+
 2020-03-04  Simon Fraser  <simon.fra...@apple.com>
 
         Make m_slowRepaintObjects a WeakHashSet

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (257903 => 257904)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2020-03-05 07:00:19 UTC (rev 257903)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2020-03-05 07:06:14 UTC (rev 257904)
@@ -1162,7 +1162,7 @@
 
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setMaxStatisticsEntries(m_sessionID, maximumEntryCount, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1172,9 +1172,8 @@
     
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools()) {
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setPruneEntriesDownTo(m_sessionID, pruneTargetCount, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
-    }
 }
 
 void WebsiteDataStore::setGrandfatheringTime(Seconds seconds, CompletionHandler<void()>&& completionHandler)
@@ -1183,7 +1182,7 @@
 
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setGrandfatheringTime(m_sessionID, seconds, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1193,7 +1192,7 @@
 
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setMinimumTimeBetweenDataRecordsRemoval(m_sessionID, seconds, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
     
@@ -1254,9 +1253,8 @@
     
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools()) {
+    for (auto& processPool : ensureProcessPools())
         processPool->ensureNetworkProcess().setPrevalentResource(m_sessionID, WebCore::RegistrableDomain { url }, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
-    }
 }
 
 void WebsiteDataStore::setPrevalentResourceForDebugMode(const URL& url, CompletionHandler<void()>&& completionHandler)
@@ -1305,8 +1303,8 @@
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     
     for (auto& processPool : processPools()) {
-        if (auto* process = processPool->networkProcess())
-            process->setVeryPrevalentResource(m_sessionID, WebCore::RegistrableDomain { url }, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
+        if (auto* networkProcess = processPool->networkProcess())
+            networkProcess->setVeryPrevalentResource(m_sessionID, WebCore::RegistrableDomain { url }, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
     }
 }
 
@@ -1316,7 +1314,7 @@
     
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setShouldClassifyResourcesBeforeDataRecordsRemoval(m_sessionID, value, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1617,7 +1615,7 @@
 {
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
 
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setNotifyPagesWhenDataRecordsWereScanned(m_sessionID, value, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1626,7 +1624,7 @@
     useExplicitITPState();
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setIsRunningResourceLoadStatisticsTest(m_sessionID, value, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1634,7 +1632,7 @@
 {
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setNotifyPagesWhenTelemetryWasCaptured(m_sessionID, value, [processPool, callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1645,9 +1643,8 @@
         completionHandler({ });
         return;
     }
-    
-    auto& networkProcess = webPage->process().processPool().ensureNetworkProcess();
-    networkProcess.getAllStorageAccessEntries(m_sessionID, WTFMove(completionHandler));
+
+    webPage->process().processPool().ensureNetworkProcess().getAllStorageAccessEntries(m_sessionID, WTFMove(completionHandler));
 }
 
 
@@ -1655,7 +1652,7 @@
 {
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setTimeToLiveUserInteraction(m_sessionID, seconds, [callbackAggregator = callbackAggregator.copyRef()] { });
 }
 
@@ -1867,7 +1864,7 @@
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setCacheMaxAgeCapForPrevalentResources(m_sessionID, seconds, [callbackAggregator = callbackAggregator.copyRef()] { });
 #else
     UNUSED_PARAM(seconds);
@@ -2072,7 +2069,7 @@
 
     auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     
-    for (auto& processPool : ensureProcessPools())
+    for (auto& processPool : processPools())
         processPool->ensureNetworkProcess().setResourceLoadStatisticsDebugMode(m_sessionID, enabled, [callbackAggregator = callbackAggregator.copyRef()] { });
 #else
     UNUSED_PARAM(enabled);
@@ -2110,7 +2107,7 @@
 void WebsiteDataStore::clearResourceLoadStatisticsInWebProcesses(CompletionHandler<void()>&& callback)
 {
     if (resourceLoadStatisticsEnabled()) {
-        for (auto& processPool : ensureProcessPools())
+        for (auto& processPool : processPools())
             processPool->clearResourceLoadStatistics();
     }
     callback();

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to