Changeset 248144 in webkit

Timestamp:

Aug 1, 2019 6:59:08 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Do not send NetworkProcessProxy::LogTestingEvent message if we are not testing
​https://bugs.webkit.org/show_bug.cgi?id=200360

Patch by Alex Christensen <​achristensen@webkit.org> on 2019-08-01
Reviewed by Tim Horton.

Source/WebKit:

Sending this message causes instantiation of the default WebsiteDataStore in the UIProcess,
which causes more memory to be used than is needed if we are browsing without the default WebsiteDataStore.

Covered by an API test.

• NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:

(WebKit::WebResourceLoadStatisticsStore::logTestingEvent):

• NetworkProcess/NetworkSession.cpp:

(WebKit::NetworkSession::NetworkSession):

• NetworkProcess/NetworkSession.h:

(WebKit::NetworkSession::enableResourceLoadStatisticsLogTestingEvent const):

• NetworkProcess/NetworkSessionCreationParameters.cpp:

(WebKit::NetworkSessionCreationParameters::encode const):
(WebKit::NetworkSessionCreationParameters::decode):

• NetworkProcess/NetworkSessionCreationParameters.h:
• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::ensureNetworkProcess):

• UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:

(WebKit::WebsiteDataStore::parameters):

• UIProcess/WebsiteData/WebsiteDataStore.h:

(WebKit::WebsiteDataStore::hasStatisticsTestingCallback const):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:

(cleanupITPDatabase):
(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.h (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.messages.in (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkSession.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkSession.h (modified) (2 diffs)
• Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.cpp (modified) (4 diffs)
• Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (4 diffs)
• Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm (modified) (1 diff)
• Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (3 diffs)
• Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm (modified) (4 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-08-01  Alex Christensen  <achristensen@webkit.org>
+
+        Do not send NetworkProcessProxy::LogTestingEvent message if we are not testing
+        https://bugs.webkit.org/show_bug.cgi?id=200360
+
+        Reviewed by Tim Horton.
+
+        Sending this message causes instantiation of the default WebsiteDataStore in the UIProcess,
+        which causes more memory to be used than is needed if we are browsing without the default WebsiteDataStore.
+
+        Covered by an API test.
+
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::logTestingEvent):
+        * NetworkProcess/NetworkSession.cpp:
+        (WebKit::NetworkSession::NetworkSession):
+        * NetworkProcess/NetworkSession.h:
+        (WebKit::NetworkSession::enableResourceLoadStatisticsLogTestingEvent const):
+        * NetworkProcess/NetworkSessionCreationParameters.cpp:
+        (WebKit::NetworkSessionCreationParameters::encode const):
+        (WebKit::NetworkSessionCreationParameters::decode):
+        * NetworkProcess/NetworkSessionCreationParameters.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::ensureNetworkProcess):
+        * UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:
+        (WebKit::WebsiteDataStore::parameters):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+        (WebKit::WebsiteDataStore::hasStatisticsTestingCallback const):
+
 2019-08-01  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp

@@ -1007 +1007 @@
     ASSERT(RunLoop::isMain());
 
-    if (m_networkSession)
+    if (m_networkSession && m_networkSession->enableResourceLoadStatisticsLogTestingEvent())
         m_networkSession->networkProcess().parentProcessConnection()->send(Messages::NetworkProcessProxy::LogTestingEvent(m_networkSession->sessionID(), event), 0);
 }

trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

@@ -1160 +1160 @@
 }
 
+void NetworkProcess::setResourceLoadStatisticsLogTestingEvent(bool enabled)
+{
+    forEachNetworkSession([enabled](auto& networkSession) {
+        networkSession.setResourceLoadStatisticsLogTestingEvent(enabled);
+    });
+}
+
 void NetworkProcess::setResourceLoadStatisticsDebugMode(PAL::SessionID sessionID, bool debugMode, CompletionHandler<void()>&& completionHandler)
 {

trunk/Source/WebKit/NetworkProcess/NetworkProcess.h

@@ -247 +247 @@
     void setNotifyPagesWhenTelemetryWasCaptured(PAL::SessionID, bool value, CompletionHandler<void()>&&);
     void setResourceLoadStatisticsEnabled(bool);
+    void setResourceLoadStatisticsLogTestingEvent(bool);
     void setResourceLoadStatisticsDebugMode(PAL::SessionID, bool debugMode, CompletionHandler<void()>&&d);
     void setShouldClassifyResourcesBeforeDataRecordsRemoval(PAL::SessionID, bool value, CompletionHandler<void()>&&);

trunk/Source/WebKit/NetworkProcess/NetworkProcess.messages.in

@@ -92 +92 @@
     DumpResourceLoadStatistics(PAL::SessionID sessionID) -> (String dumpedStatistics) Async
     SetResourceLoadStatisticsEnabled(bool enabled)
+    SetResourceLoadStatisticsLogTestingEvent(bool enabled)
     UpdatePrevalentDomainsToBlockCookiesFor(PAL::SessionID sessionID, Vector<WebCore::RegistrableDomain> domainsToBlock) -> () Async
     IsGrandfathered(PAL::SessionID sessionID, WebCore::RegistrableDomain targetDomain) -> (bool isGrandfathered) Async

trunk/Source/WebKit/NetworkProcess/NetworkSession.cpp

@@ -82 +82 @@
     : m_sessionID(parameters.sessionID)
     , m_networkProcess(networkProcess)
+#if ENABLE(RESOURCE_LOAD_STATISTICS)
+    , m_enableResourceLoadStatisticsLogTestingEvent(parameters.enableResourceLoadStatisticsLogTestingEvent)
+#endif
     , m_adClickAttribution(makeUniqueRef<AdClickAttributionManager>(parameters.sessionID))
     , m_storageManager(StorageManager::create(String(parameters.localStorageDirectory)))

trunk/Source/WebKit/NetworkProcess/NetworkSession.h

@@ -91 +91 @@
     void logDiagnosticMessageWithValue(const String& message, const String& description, unsigned value, unsigned significantFigures, WebCore::ShouldSample);
     void notifyPageStatisticsTelemetryFinished(unsigned totalPrevalentResources, unsigned totalPrevalentResourcesWithUserInteraction, unsigned top3SubframeUnderTopFrameOrigins);
+    bool enableResourceLoadStatisticsLogTestingEvent() const { return m_enableResourceLoadStatisticsLogTestingEvent; }
+    void setResourceLoadStatisticsLogTestingEvent(bool log) { m_enableResourceLoadStatisticsLogTestingEvent = log; }
 #endif
     void storeAdClickAttribution(WebCore::AdClickAttribution&&);
@@ -129 +131 @@
     EnableResourceLoadStatisticsDebugMode m_enableResourceLoadStatisticsDebugMode { EnableResourceLoadStatisticsDebugMode::No };
     WebCore::RegistrableDomain m_resourceLoadStatisticsManualPrevalentResource;
+    bool m_enableResourceLoadStatisticsLogTestingEvent;
 #endif
     UniqueRef<AdClickAttributionManager> m_adClickAttribution;

trunk/Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.cpp

@@ -51 +51 @@
         , { }, { }
 #endif
-        , { }, { }, false, { }, { }, { }, { }, { }, { }, { }, { }, { }
+        , { }, { }, false, false, { }, { }, { }, { }, { }, { }, { }, { }, { }
     };
 }
@@ -81 +81 @@
     encoder << resourceLoadStatisticsDirectoryExtensionHandle;
     encoder << enableResourceLoadStatistics;
+    encoder << enableResourceLoadStatisticsLogTestingEvent;
     encoder << shouldIncludeLocalhostInResourceLoadStatistics;
     encoder << enableResourceLoadStatisticsDebugMode;
@@ -186 +187 @@
     decoder >> enableResourceLoadStatistics;
     if (!enableResourceLoadStatistics)
+        return WTF::nullopt;
+
+    Optional<bool> enableResourceLoadStatisticsLogTestingEvent;
+    decoder >> enableResourceLoadStatisticsLogTestingEvent;
+    if (!enableResourceLoadStatisticsLogTestingEvent)
         return WTF::nullopt;
 
@@ -258 +264 @@
         , WTFMove(*resourceLoadStatisticsDirectoryExtensionHandle)
         , WTFMove(*enableResourceLoadStatistics)
+        , WTFMove(*enableResourceLoadStatisticsLogTestingEvent)
         , WTFMove(*shouldIncludeLocalhostInResourceLoadStatistics)
         , WTFMove(*enableResourceLoadStatisticsDebugMode)

trunk/Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.h

@@ -85 +85 @@
     SandboxExtension::Handle resourceLoadStatisticsDirectoryExtensionHandle;
     bool enableResourceLoadStatistics { false };
+    bool enableResourceLoadStatisticsLogTestingEvent { false };
     bool shouldIncludeLocalhostInResourceLoadStatistics { true };
     bool enableResourceLoadStatisticsDebugMode { false };

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm

@@ -457 +457 @@
 
     if (callback) {
-        _websiteDataStore->websiteDataStore().enableResourceLoadStatisticsAndSetTestingCallback([callback = makeBlockPtr(callback), self](const String& event) {
-            callback(self, (NSString *)event);
+        _websiteDataStore->websiteDataStore().setStatisticsTestingCallback([callback = makeBlockPtr(callback), self](const String& event) {
+            callback(self, event);
         });
         return;

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -595 +595 @@
 
     bool enableResourceLoadStatistics = m_shouldEnableITPForDefaultSessions;
+    bool enableResourceLoadStatisticsLogTestingEvent = false;
     bool shouldIncludeLocalhost = true;
     bool enableResourceLoadStatisticsDebugMode = false;
@@ -600 +601 @@
     if (withWebsiteDataStore) {
         enableResourceLoadStatistics = withWebsiteDataStore->resourceLoadStatisticsEnabled();
+#if ENABLE(RESOURCE_LOAD_STATISTICS)
+        enableResourceLoadStatisticsLogTestingEvent = withWebsiteDataStore->hasStatisticsTestingCallback();
+#endif
         if (enableResourceLoadStatistics) {
             auto networkSessionParameters = withWebsiteDataStore->parameters().networkSessionParameters;
@@ -611 +615 @@
     } else if (m_websiteDataStore) {
         enableResourceLoadStatistics = m_websiteDataStore->resourceLoadStatisticsEnabled();
+#if ENABLE(RESOURCE_LOAD_STATISTICS)
+        enableResourceLoadStatisticsLogTestingEvent = m_websiteDataStore->websiteDataStore().hasStatisticsTestingCallback();
+#endif
         if (enableResourceLoadStatistics) {
             auto networkSessionParameters = m_websiteDataStore->websiteDataStore().parameters().networkSessionParameters;
@@ -623 +630 @@
 
     parameters.defaultDataStoreParameters.networkSessionParameters.enableResourceLoadStatistics = enableResourceLoadStatistics;
+    parameters.defaultDataStoreParameters.networkSessionParameters.enableResourceLoadStatisticsLogTestingEvent = enableResourceLoadStatisticsLogTestingEvent;
     parameters.defaultDataStoreParameters.networkSessionParameters.shouldIncludeLocalhostInResourceLoadStatistics = shouldIncludeLocalhost;
     parameters.defaultDataStoreParameters.networkSessionParameters.enableResourceLoadStatisticsDebugMode = enableResourceLoadStatisticsDebugMode;

trunk/Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm

@@ -135 +135 @@
         WTFMove(resourceLoadStatisticsDirectoryHandle),
         false,
+        false,
         shouldIncludeLocalhostInResourceLoadStatistics,
         enableResourceLoadStatisticsDebugMode,

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -1846 +1846 @@
 
     if (enabled) {
-        enableResourceLoadStatisticsAndSetTestingCallback(nullptr);
+        m_resourceLoadStatisticsEnabled = true;
+        
+        resolveDirectoriesIfNecessary();
+        
+        for (auto& processPool : processPools(std::numeric_limits<size_t>::max(), false)) {
+            processPool->setResourceLoadStatisticsEnabled(true);
+            processPool->sendToNetworkingProcess(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(true));
+        }
         return;
     }
@@ -1860 +1867 @@
 #endif
 }
+
+#if ENABLE(RESOURCE_LOAD_STATISTICS)
+void WebsiteDataStore::setStatisticsTestingCallback(Function<void(const String&)>&& callback)
+{
+    if (callback) {
+        for (auto& processPool : processPools(std::numeric_limits<size_t>::max(), false))
+            processPool->sendToNetworkingProcess(Messages::NetworkProcess::SetResourceLoadStatisticsLogTestingEvent(true));
+    }
+    
+    m_statisticsTestingCallback = WTFMove(callback);
+}
+#endif
 
 void WebsiteDataStore::setResourceLoadStatisticsDebugMode(bool enabled)
@@ -1882 +1901 @@
 
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
-void WebsiteDataStore::enableResourceLoadStatisticsAndSetTestingCallback(Function<void (const String&)>&& callback)
-{
-    ASSERT(!m_sessionID.isEphemeral());
-
-    m_resourceLoadStatisticsEnabled = true;
-    setStatisticsTestingCallback(WTFMove(callback));
-
-    resolveDirectoriesIfNecessary();
-
-    for (auto& processPool : processPools(std::numeric_limits<size_t>::max(), false)) {
-        processPool->setResourceLoadStatisticsEnabled(true);
-        processPool->sendToNetworkingProcess(Messages::NetworkProcess::SetResourceLoadStatisticsEnabled(true));
-    }
-}
-
 void WebsiteDataStore::logTestingEvent(const String& event)
 {

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h

@@ -172 +172 @@
     void setPrevalentResourceForDebugMode(const URL&, CompletionHandler<void()>&&);
     void setShouldClassifyResourcesBeforeDataRecordsRemoval(bool, CompletionHandler<void()>&&);
-    void setStatisticsTestingCallback(WTF::Function<void(const String&)>&& callback) { m_statisticsTestingCallback = WTFMove(callback); }
+    void setStatisticsTestingCallback(Function<void(const String&)>&&);
+    bool hasStatisticsTestingCallback() const { return !!m_statisticsTestingCallback; }
     void setVeryPrevalentResource(const URL&, CompletionHandler<void()>&&);
     void setSubframeUnderTopFrameDomain(const URL& subframe, const URL& topFrame);
@@ -208 +209 @@
     void clearPendingCookies();
 
-    void enableResourceLoadStatisticsAndSetTestingCallback(Function<void (const String&)>&& callback);
-
     void setBoundInterfaceIdentifier(String&& identifier) { m_boundInterfaceIdentifier = WTFMove(identifier); }
     const String& boundInterfaceIdentifier() { return m_boundInterfaceIdentifier; }

trunk/Tools/ChangeLog

@@ +1 @@
+2019-08-01  Alex Christensen  <achristensen@webkit.org>
+
+        Do not send NetworkProcessProxy::LogTestingEvent message if we are not testing
+        https://bugs.webkit.org/show_bug.cgi?id=200360
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:
+        (cleanupITPDatabase):
+        (TEST):
+
 2019-08-01  Alex Christensen  <achristensen@webkit.org>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm

@@ -28 +28 @@
 #import "PlatformUtilities.h"
 #import "TestNavigationDelegate.h"
+#import "TestWKWebView.h"
 #import <WebKit/WKFoundation.h>
 #import <WebKit/WKPreferencesPrivate.h>
@@ -215 +216 @@
     // 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) {
@@ -225 +222 @@
 
         dataSyncCompleted = true;
+    }];
+    [dataStore _clearPrevalentDomain:[NSURL URLWithString:@"http://evil.com"] completionHandler: ^(void) {
+        doneFlag = true;
     }];
 
@@ -474 +474 @@
     TestWebKitAPI::Util::run(&doneFlag);
 }
+
+TEST(ResourceLoadStatistics, NoMessagesWhenNotTesting)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    WKWebsiteDataStore *dataStore = [[[WKWebsiteDataStore alloc] _initWithConfiguration:[[[_WKWebsiteDataStoreConfiguration alloc] init] autorelease]] autorelease];
+    [configuration setWebsiteDataStore:dataStore];
+    [dataStore _setResourceLoadStatisticsEnabled:YES];
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100) configuration:configuration.get()]);
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+    EXPECT_FALSE([WKWebsiteDataStore _defaultDataStoreExists]);
+}