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

Fri, 15 Dec 2017 13:28:10 -0800

Title: [225984] trunk/Source/WebKit
Revision
225984
Author
[email protected]
Date
2017-12-15 13:27:24 -0800 (Fri, 15 Dec 2017)

Log Message

Avoid waking plugin process up unnecessarily
https://bugs.webkit.org/show_bug.cgi?id=180819
<rdar://problem/36051548>

Reviewed by Geoffrey Garen.

WebKit purges data from origins marked as prevalent on an hourly interval. This includes waking up plugins
and removing relevant data stored in those plugins. This causes multiple plugin processes to be spawned,
even though the user is not interacting with any plugins.

Instead, we should delay removing data from plugins until they are loaded due to the user interacting with
a website using a plugin.

Make the following changes:
1. When looking for plugin data related to prevalent sites, only examine plugin data if the relevant plugin
   is already running.
2. When the state of the active plugins changes, trigger a data removal check.

* Shared/WebsiteData/WebsiteDataFetchOption.h: Add a new option 'DoNotCreateProcesses'
* UIProcess/Plugins/PluginProcessManager.cpp:
(WebKit::PluginProcessManager::getPluginProcessConnection): Pass new argument 'Launch', since we always
want to launch processes in this case.
(WebKit::PluginProcessManager::fetchWebsiteData): Pass a new parameter 'processAccessType' so that the
caller can specify if they only want to examine already-active plugins, or if they want to spawn new
processes. Call the completion handler if we need to exit early when no process exists.
(WebKit::PluginProcessManager::deleteWebsiteData): Always launch new processes.
(WebKit::PluginProcessManager::deleteWebsiteDataForHostNames): Ditto.
(WebKit::PluginProcessManager::getOrCreatePluginProcess): Accept new parameter indicating if new processes
shoudl be launched. Also switch to C++ style loops.
* UIProcess/Plugins/PluginProcessManager.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores): Use the
new 'DoNotCreatePrcesses' access type.
* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::removeDataRecords): Update the active plugin count when we
remove data.
(WebKit::WebResourceLoadStatisticsStore::shouldRemoveDataRecords const): Return true if the active plugin
process count changed since the last time data was removed.
* UIProcess/WebResourceLoadStatisticsStore.h:
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::fetchDataAndApply): Check fetch option for new 'DoNotCreateProcesses' flag,
and tell the PluginProcessManager which mode we are using.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (225983 => 225984)


--- trunk/Source/WebKit/ChangeLog	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/ChangeLog	2017-12-15 21:27:24 UTC (rev 225984)
@@ -1,3 +1,48 @@
+2017-12-15  Brent Fulgham  <[email protected]>
+
+        Avoid waking plugin process up unnecessarily
+        https://bugs.webkit.org/show_bug.cgi?id=180819
+        <rdar://problem/36051548>
+
+        Reviewed by Geoffrey Garen.
+
+        WebKit purges data from origins marked as prevalent on an hourly interval. This includes waking up plugins
+        and removing relevant data stored in those plugins. This causes multiple plugin processes to be spawned,
+        even though the user is not interacting with any plugins.
+
+        Instead, we should delay removing data from plugins until they are loaded due to the user interacting with
+        a website using a plugin.
+
+        Make the following changes:
+        1. When looking for plugin data related to prevalent sites, only examine plugin data if the relevant plugin
+           is already running.
+        2. When the state of the active plugins changes, trigger a data removal check.
+
+        * Shared/WebsiteData/WebsiteDataFetchOption.h: Add a new option 'DoNotCreateProcesses'
+        * UIProcess/Plugins/PluginProcessManager.cpp:
+        (WebKit::PluginProcessManager::getPluginProcessConnection): Pass new argument 'Launch', since we always
+        want to launch processes in this case.
+        (WebKit::PluginProcessManager::fetchWebsiteData): Pass a new parameter 'processAccessType' so that the
+        caller can specify if they only want to examine already-active plugins, or if they want to spawn new
+        processes. Call the completion handler if we need to exit early when no process exists.
+        (WebKit::PluginProcessManager::deleteWebsiteData): Always launch new processes.
+        (WebKit::PluginProcessManager::deleteWebsiteDataForHostNames): Ditto.
+        (WebKit::PluginProcessManager::getOrCreatePluginProcess): Accept new parameter indicating if new processes
+        shoudl be launched. Also switch to C++ style loops.
+        * UIProcess/Plugins/PluginProcessManager.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores): Use the
+        new 'DoNotCreatePrcesses' access type.
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::removeDataRecords): Update the active plugin count when we
+        remove data.
+        (WebKit::WebResourceLoadStatisticsStore::shouldRemoveDataRecords const): Return true if the active plugin
+        process count changed since the last time data was removed.
+        * UIProcess/WebResourceLoadStatisticsStore.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::fetchDataAndApply): Check fetch option for new 'DoNotCreateProcesses' flag,
+        and tell the PluginProcessManager which mode we are using.
+
 2017-12-15  Brady Eidson  <[email protected]>
 
         Make sure only WebsiteDataStores with valid SessionIDs register themselves.

Modified: trunk/Source/WebKit/Shared/WebsiteData/WebsiteDataFetchOption.h (225983 => 225984)


--- trunk/Source/WebKit/Shared/WebsiteData/WebsiteDataFetchOption.h	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/Shared/WebsiteData/WebsiteDataFetchOption.h	2017-12-15 21:27:24 UTC (rev 225984)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -23,16 +23,14 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef WebsiteDataFetchOption_h
-#define WebsiteDataFetchOption_h
+#pragma once
 
 namespace WebKit {
 
 enum class WebsiteDataFetchOption {
     ComputeSizes = 1 << 0,
+    DoNotCreateProcesses = 1 << 1,
 };
 
 }
 
-
-#endif // WebsiteDataFetchOption_h

Modified: trunk/Source/WebKit/UIProcess/Plugins/PluginProcessManager.cpp (225983 => 225984)


--- trunk/Source/WebKit/UIProcess/Plugins/PluginProcessManager.cpp	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/UIProcess/Plugins/PluginProcessManager.cpp	2017-12-15 21:27:24 UTC (rev 225984)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,6 +29,7 @@
 #if ENABLE(NETSCAPE_PLUGIN_API)
 
 #include "PluginProcessProxy.h"
+#include "WebsiteDataFetchOption.h"
 #include <wtf/CryptographicallyRandomNumber.h>
 #include <wtf/NeverDestroyed.h>
 #include <wtf/StdLibExtras.h>
@@ -96,9 +97,14 @@
     m_pluginProcesses.remove(vectorIndex);
 }
 
-void PluginProcessManager::fetchWebsiteData(const PluginModuleInfo& plugin, WTF::Function<void (Vector<String>)>&& completionHandler)
+void PluginProcessManager::fetchWebsiteData(const PluginModuleInfo& plugin, OptionSet<WebsiteDataFetchOption> fetchOptions, WTF::Function<void (Vector<String>)>&& completionHandler)
 {
-    PluginProcessProxy* pluginProcess = getOrCreatePluginProcess(pluginProcessToken(plugin, PluginProcessTypeNormal, PluginProcessSandboxPolicyNormal));
+    auto token = pluginProcessToken(plugin, PluginProcessTypeNormal, PluginProcessSandboxPolicyNormal);
+    auto pluginProcess = fetchOptions.contains(WebsiteDataFetchOption::DoNotCreateProcesses) ? getPluginProcess(token) : getOrCreatePluginProcess(token);
+    if (!pluginProcess) {
+        completionHandler({ });
+        return;
+    }
 
     pluginProcess->fetchWebsiteData(WTFMove(completionHandler));
 }
@@ -106,7 +112,6 @@
 void PluginProcessManager::deleteWebsiteData(const PluginModuleInfo& plugin, std::chrono::system_clock::time_point modifiedSince, WTF::Function<void ()>&& completionHandler)
 {
     PluginProcessProxy* pluginProcess = getOrCreatePluginProcess(pluginProcessToken(plugin, PluginProcessTypeNormal, PluginProcessSandboxPolicyNormal));
-
     pluginProcess->deleteWebsiteData(modifiedSince, WTFMove(completionHandler));
 }
 
@@ -116,19 +121,25 @@
     pluginProcess->deleteWebsiteDataForHostNames(hostNames, WTFMove(completionHandler));
 }
 
-PluginProcessProxy* PluginProcessManager::getOrCreatePluginProcess(uint64_t pluginProcessToken)
+PluginProcessProxy* PluginProcessManager::getPluginProcess(uint64_t pluginProcessToken)
 {
-    for (size_t i = 0; i < m_pluginProcesses.size(); ++i) {
-        if (m_pluginProcesses[i]->pluginProcessToken() == pluginProcessToken)
-            return m_pluginProcesses[i].get();
+    for (auto pluginProcess : m_pluginProcesses) {
+        if (pluginProcess->pluginProcessToken() == pluginProcessToken)
+            return pluginProcess.get();
     }
 
-    for (size_t i = 0; i < m_pluginProcessTokens.size(); ++i) {
-        auto& attributesAndToken = m_pluginProcessTokens[i];
+    return nullptr;
+}
+
+PluginProcessProxy* PluginProcessManager::getOrCreatePluginProcess(uint64_t pluginProcessToken)
+{
+    if (auto existingProcess = getPluginProcess(pluginProcessToken))
+        return existingProcess;
+
+    for (auto& attributesAndToken : m_pluginProcessTokens) {
         if (attributesAndToken.second == pluginProcessToken) {
             auto pluginProcess = PluginProcessProxy::create(this, attributesAndToken.first, attributesAndToken.second);
             PluginProcessProxy* pluginProcessPtr = pluginProcess.ptr();
-
             m_pluginProcesses.append(WTFMove(pluginProcess));
             return pluginProcessPtr;
         }

Modified: trunk/Source/WebKit/UIProcess/Plugins/PluginProcessManager.h (225983 => 225984)


--- trunk/Source/WebKit/UIProcess/Plugins/PluginProcessManager.h	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/UIProcess/Plugins/PluginProcessManager.h	2017-12-15 21:27:24 UTC (rev 225984)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -47,6 +47,7 @@
 class PluginInfoStore;
 class PluginProcessProxy;
 class WebProcessProxy;
+enum class WebsiteDataFetchOption;
 
 class PluginProcessManager {
     WTF_MAKE_NONCOPYABLE(PluginProcessManager);
@@ -59,7 +60,7 @@
     void getPluginProcessConnection(uint64_t pluginProcessToken, Ref<Messages::WebProcessProxy::GetPluginProcessConnection::DelayedReply>&&);
     void removePluginProcessProxy(PluginProcessProxy*);
 
-    void fetchWebsiteData(const PluginModuleInfo&, WTF::Function<void (Vector<String>)>&& completionHandler);
+    void fetchWebsiteData(const PluginModuleInfo&, OptionSet<WebsiteDataFetchOption>, WTF::Function<void (Vector<String>)>&& completionHandler);
     void deleteWebsiteData(const PluginModuleInfo&, std::chrono::system_clock::time_point modifiedSince, WTF::Function<void ()>&& completionHandler);
     void deleteWebsiteDataForHostNames(const PluginModuleInfo&, const Vector<String>& hostNames, WTF::Function<void ()>&& completionHandler);
 
@@ -74,6 +75,7 @@
 private:
     PluginProcessManager();
 
+    PluginProcessProxy* getPluginProcess(uint64_t pluginProcessToken);
     PluginProcessProxy* getOrCreatePluginProcess(uint64_t pluginProcessToken);
 
     Vector<std::pair<PluginProcessAttributes, uint64_t>> m_pluginProcessTokens;

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (225983 => 225984)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2017-12-15 21:27:24 UTC (rev 225984)
@@ -49,6 +49,7 @@
 #include "WebProcessProxyMessages.h"
 #include "WebUserContentControllerProxy.h"
 #include "WebsiteData.h"
+#include "WebsiteDataFetchOption.h"
 #include <WebCore/DiagnosticLoggingKeys.h>
 #include <WebCore/PublicSuffix.h>
 #include <WebCore/SuddenTermination.h>
@@ -244,6 +245,7 @@
     };
     
     RefPtr<CallbackAggregator> callbackAggregator = adoptRef(new CallbackAggregator(WTFMove(completionHandler)));
+    OptionSet<WebsiteDataFetchOption> fetchOptions = WebsiteDataFetchOption::DoNotCreateProcesses;
 
     HashSet<PAL::SessionID> visitedSessionIDs;
     for (auto& page : globalPageMap()) {
@@ -252,7 +254,7 @@
             continue;
         visitedSessionIDs.add(dataStore.sessionID());
         callbackAggregator->addPendingCallback();
-        dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, { }, topPrivatelyControlledDomains, [callbackAggregator, shouldNotifyPage, page](HashSet<String>&& domainsWithDeletedWebsiteData) {
+        dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, fetchOptions, topPrivatelyControlledDomains, [callbackAggregator, shouldNotifyPage, page](HashSet<String>&& domainsWithDeletedWebsiteData) {
             // When completing the task, we should be getting called on the main thread.
             ASSERT(RunLoop::isMain());

Modified: trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp (225983 => 225984)


--- trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp	2017-12-15 21:27:24 UTC (rev 225984)
@@ -27,6 +27,8 @@
 #include "WebResourceLoadStatisticsStore.h"
 
 #include "Logging.h"
+#include "PluginProcessManager.h"
+#include "PluginProcessProxy.h"
 #include "WebProcessMessages.h"
 #include "WebProcessProxy.h"
 #include "WebResourceLoadStatisticsStoreMessages.h"
@@ -185,6 +187,12 @@
     if (!shouldRemoveDataRecords())
         return;
 
+#if ENABLE(NETSCAPE_PLUGIN_API)
+    m_activePluginTokens.clear();
+    for (auto plugin : PluginProcessManager::singleton().pluginProcesses())
+        m_activePluginTokens.add(plugin->pluginProcessToken());
+#endif
+
     auto prevalentResourceDomains = topPrivatelyControlledDomainsToRemoveWebsiteDataFor();
     if (prevalentResourceDomains.isEmpty())
         return;
@@ -627,6 +635,13 @@
     if (m_dataRecordsBeingRemoved)
         return false;
 
+#if ENABLE(NETSCAPE_PLUGIN_API)
+    for (auto plugin : PluginProcessManager::singleton().pluginProcesses()) {
+        if (!m_activePluginTokens.contains(plugin->pluginProcessToken()))
+            return true;
+    }
+#endif
+
     return !m_lastTimeDataRecordsWereRemoved || MonotonicTime::now() >= (m_lastTimeDataRecordsWereRemoved + m_parameters.minimumTimeBetweenDataRecordsRemoval);
 }

Modified: trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h (225983 => 225984)


--- trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	2017-12-15 21:27:24 UTC (rev 225984)
@@ -30,6 +30,7 @@
 #include "ResourceLoadStatisticsPersistentStorage.h"
 #include "WebsiteDataType.h"
 #include <wtf/CompletionHandler.h>
+#include <wtf/HashSet.h>
 #include <wtf/MonotonicTime.h>
 #include <wtf/RunLoop.h>
 #include <wtf/Vector.h>
@@ -203,6 +204,9 @@
 
     Parameters m_parameters;
 
+#if ENABLE(NETSCAPE_PLUGIN_API)
+    HashSet<uint64_t> m_activePluginTokens;
+#endif
     bool m_dataRecordsBeingRemoved { false };
 
     Function<void (const String&)> m_statisticsTestingCallback;

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (225983 => 225984)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-12-15 21:18:59 UTC (rev 225983)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-12-15 21:27:24 UTC (rev 225984)
@@ -524,7 +524,7 @@
                 }
 
                 auto plugin = m_plugins.takeLast();
-                PluginProcessManager::singleton().fetchWebsiteData(plugin, [this](Vector<String> hostNames) {
+                PluginProcessManager::singleton().fetchWebsiteData(plugin, m_callbackAggregator->fetchOptions, [this](Vector<String> hostNames) {
                     for (auto& hostName : hostNames)
                         m_hostNames.add(WTFMove(hostName));
                     fetchWebsiteDataForNextPlugin();

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to