Title: [218827] trunk/Source
Revision
218827
Author
cdu...@apple.com
Date
2017-06-26 20:33:12 -0700 (Mon, 26 Jun 2017)

Log Message

WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains() is inefficient
https://bugs.webkit.org/show_bug.cgi?id=173850

Reviewed by Ryosuke Niwa.

Source/WebCore:

* loader/ResourceLoadStatisticsStore.cpp:
(WebCore::ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords):
* loader/ResourceLoadStatisticsStore.h:

Source/WebKit2:

Update WebsiteDataRecord::matchesTopPrivatelyControlledDomain() to rely on
SecurityOriginData::host rather than SecurityOriginData::securityOrigin()->host().
SecurityOriginData::securityOrigin() is expensive and it seems unnecessary to call
it here since we already have the host.

Also update WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains() to return
domains as a HashSet rather than a Vector to avoid having duplicate domains.

* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores):
* UIProcess/WebProcessProxy.h:
* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::removeDataRecords):
* UIProcess/WebsiteData/WebsiteDataRecord.cpp:
(WebKit::WebsiteDataRecord::matchesTopPrivatelyControlledDomain):
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains):
(WebKit::WebsiteDataStore::removeDataForTopPrivatelyControlledDomains):
* UIProcess/WebsiteData/WebsiteDataStore.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (218826 => 218827)


--- trunk/Source/WebCore/ChangeLog	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebCore/ChangeLog	2017-06-27 03:33:12 UTC (rev 218827)
@@ -1,3 +1,14 @@
+2017-06-26  Chris Dumez  <cdu...@apple.com>
+
+        WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains() is inefficient
+        https://bugs.webkit.org/show_bug.cgi?id=173850
+
+        Reviewed by Ryosuke Niwa.
+
+        * loader/ResourceLoadStatisticsStore.cpp:
+        (WebCore::ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords):
+        * loader/ResourceLoadStatisticsStore.h:
+
 2017-06-26  Antti Koivisto  <an...@apple.com>
 
         REGRESSION (AsyncImageDecoding): A tab with the WWDC keynote paused is killed for using excessive power (Image thrashing)

Modified: trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp (218826 => 218827)


--- trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp	2017-06-27 03:33:12 UTC (rev 218827)
@@ -362,7 +362,7 @@
     return prevalentResources;
 }
 
-void ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords(const Vector<String>& prevalentResourceDomains)
+void ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords(const HashSet<String>& prevalentResourceDomains)
 {
     auto locker = holdLock(m_statisticsLock);
     for (auto& prevalentResourceDomain : prevalentResourceDomains) {

Modified: trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h (218826 => 218827)


--- trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h	2017-06-27 03:33:12 UTC (rev 218827)
@@ -77,7 +77,7 @@
 
     WEBCORE_EXPORT bool hasHadRecentUserInteraction(ResourceLoadStatistics&) const;
     WEBCORE_EXPORT Vector<String> topPrivatelyControlledDomainsToRemoveWebsiteDataFor();
-    WEBCORE_EXPORT void updateStatisticsForRemovedDataRecords(const Vector<String>& prevalentResourceDomains);
+    WEBCORE_EXPORT void updateStatisticsForRemovedDataRecords(const HashSet<String>& prevalentResourceDomains);
 
     WEBCORE_EXPORT void handleFreshStartWithEmptyOrNoStore(HashSet<String>&& topPrivatelyControlledDomainsToGrandfather);
     WEBCORE_EXPORT bool shouldRemoveDataRecords() const;

Modified: trunk/Source/WebKit2/ChangeLog (218826 => 218827)


--- trunk/Source/WebKit2/ChangeLog	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/ChangeLog	2017-06-27 03:33:12 UTC (rev 218827)
@@ -1,3 +1,30 @@
+2017-06-26  Chris Dumez  <cdu...@apple.com>
+
+        WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains() is inefficient
+        https://bugs.webkit.org/show_bug.cgi?id=173850
+
+        Reviewed by Ryosuke Niwa.
+
+        Update WebsiteDataRecord::matchesTopPrivatelyControlledDomain() to rely on
+        SecurityOriginData::host rather than SecurityOriginData::securityOrigin()->host().
+        SecurityOriginData::securityOrigin() is expensive and it seems unnecessary to call
+        it here since we already have the host.
+
+        Also update WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains() to return
+        domains as a HashSet rather than a Vector to avoid having duplicate domains.
+
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores):
+        * UIProcess/WebProcessProxy.h:
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::removeDataRecords):
+        * UIProcess/WebsiteData/WebsiteDataRecord.cpp:
+        (WebKit::WebsiteDataRecord::matchesTopPrivatelyControlledDomain):
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains):
+        (WebKit::WebsiteDataStore::removeDataForTopPrivatelyControlledDomains):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
 2017-06-26  Jeremy Jones  <jere...@apple.com>
 
         Invalidate WebVideoFullscreenManager when WebPage is destroyed.

Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (218826 => 218827)


--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp	2017-06-27 03:33:12 UTC (rev 218827)
@@ -209,19 +209,19 @@
     return globalPageMap().get(pageID);
 }
 
-void WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(OptionSet<WebsiteDataType> dataTypes, Vector<String>&& topPrivatelyControlledDomains, bool shouldNotifyPage, Function<void (const Vector<String>&)>&& completionHandler)
+void WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(OptionSet<WebsiteDataType> dataTypes, Vector<String>&& topPrivatelyControlledDomains, bool shouldNotifyPage, Function<void (const HashSet<String>&)>&& completionHandler)
 {
     // We expect this to be called on the main thread so we get the default website data store.
     ASSERT(RunLoop::isMain());
     
     struct CallbackAggregator : ThreadSafeRefCounted<CallbackAggregator> {
-        explicit CallbackAggregator(Function<void(Vector<String>)>&& completionHandler)
+        explicit CallbackAggregator(Function<void(HashSet<String>)>&& completionHandler)
             : completionHandler(WTFMove(completionHandler))
         {
         }
-        void addDomainsWithDeletedWebsiteData(const Vector<String>& domains)
+        void addDomainsWithDeletedWebsiteData(const HashSet<String>& domains)
         {
-            domainsWithDeletedWebsiteData.appendVector(domains);
+            domainsWithDeletedWebsiteData.add(domains.begin(), domains.end());
         }
         
         void addPendingCallback()
@@ -244,8 +244,8 @@
         }
         
         unsigned pendingCallbacks = 0;
-        Function<void(Vector<String>)> completionHandler;
-        Vector<String> domainsWithDeletedWebsiteData;
+        Function<void(HashSet<String>)> completionHandler;
+        HashSet<String> domainsWithDeletedWebsiteData;
     };
     
     RefPtr<CallbackAggregator> callbackAggregator = adoptRef(new CallbackAggregator(WTFMove(completionHandler)));
@@ -257,7 +257,7 @@
             continue;
         visitedSessionIDs.add(dataStore.sessionID());
         callbackAggregator->addPendingCallback();
-        dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, { }, topPrivatelyControlledDomains, [callbackAggregator, shouldNotifyPage, page](Vector<String>&& domainsWithDeletedWebsiteData) {
+        dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, { }, 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/WebKit2/UIProcess/WebProcessProxy.h (218826 => 218827)


--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.h	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.h	2017-06-27 03:33:12 UTC (rev 218827)
@@ -141,7 +141,7 @@
     void fetchWebsiteData(WebCore::SessionID, OptionSet<WebsiteDataType>, Function<void(WebsiteData)>&& completionHandler);
     void deleteWebsiteData(WebCore::SessionID, OptionSet<WebsiteDataType>, std::chrono::system_clock::time_point modifiedSince, Function<void()>&& completionHandler);
     void deleteWebsiteDataForOrigins(WebCore::SessionID, OptionSet<WebsiteDataType>, const Vector<WebCore::SecurityOriginData>&, Function<void()>&& completionHandler);
-    static void deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(OptionSet<WebsiteDataType>, Vector<String>&& topPrivatelyControlledDomains, bool shouldNotifyPages, Function<void (const Vector<String>&)>&& completionHandler);
+    static void deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(OptionSet<WebsiteDataType>, Vector<String>&& topPrivatelyControlledDomains, bool shouldNotifyPages, Function<void (const HashSet<String>&)>&& completionHandler);
     static void topPrivatelyControlledDomainsWithWebsiteData(OptionSet<WebsiteDataType> dataTypes, bool shouldNotifyPage, Function<void(HashSet<String>&&)>&& completionHandler);
     static void notifyPageStatisticsAndDataRecordsProcessed();
 

Modified: trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp (218826 => 218827)


--- trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp	2017-06-27 03:33:12 UTC (rev 218827)
@@ -119,9 +119,9 @@
 
     // Switch to the main thread to get the default website data store
     RunLoop::main().dispatch([prevalentResourceDomains = CrossThreadCopier<Vector<String>>::copy(prevalentResourceDomains), this, protectedThis = makeRef(*this)] () mutable {
-        WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(dataTypesToRemove, WTFMove(prevalentResourceDomains), notifyPages, [this, protectedThis = WTFMove(protectedThis)](const Vector<String>& domainsWithDeletedWebsiteData) mutable {
+        WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(dataTypesToRemove, WTFMove(prevalentResourceDomains), notifyPages, [this, protectedThis = WTFMove(protectedThis)](const HashSet<String>& domainsWithDeletedWebsiteData) mutable {
             // But always touch the ResourceLoadStatistics store on the worker queue.
-            m_statisticsQueue->dispatch([protectedThis = WTFMove(protectedThis), topDomains = CrossThreadCopier<Vector<String>>::copy(domainsWithDeletedWebsiteData)] () mutable {
+            m_statisticsQueue->dispatch([protectedThis = WTFMove(protectedThis), topDomains = CrossThreadCopier<HashSet<String>>::copy(domainsWithDeletedWebsiteData)] () mutable {
                 protectedThis->coreStore().updateStatisticsForRemovedDataRecords(topDomains);
                 protectedThis->coreStore().dataRecordsWereRemoved();
             });

Modified: trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataRecord.cpp (218826 => 218827)


--- trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataRecord.cpp	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataRecord.cpp	2017-06-27 03:33:12 UTC (rev 218827)
@@ -130,7 +130,7 @@
     }
 
     for (const auto& dataRecordOriginData : origins) {
-        if (hostIsInDomain(dataRecordOriginData.securityOrigin().get().host(), topPrivatelyControlledDomain))
+        if (hostIsInDomain(dataRecordOriginData.host, topPrivatelyControlledDomain))
             return true;
     }
 

Modified: trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp (218826 => 218827)


--- trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-06-27 03:33:12 UTC (rev 218827)
@@ -507,16 +507,16 @@
     callbackAggregator->callIfNeeded();
 }
 
-void WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, Vector<String>&&)>&& completionHandler)
+void WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, HashSet<String>&&)>&& completionHandler)
 {
     fetchData(dataTypes, fetchOptions, [topPrivatelyControlledDomains, completionHandler = WTFMove(completionHandler)](auto&& existingDataRecords) {
         Vector<WebsiteDataRecord> matchingDataRecords;
-        Vector<String> domainsWithMatchingDataRecords;
+        HashSet<String> domainsWithMatchingDataRecords;
         for (auto&& dataRecord : existingDataRecords) {
             for (auto& topPrivatelyControlledDomain : topPrivatelyControlledDomains) {
                 if (dataRecord.matchesTopPrivatelyControlledDomain(topPrivatelyControlledDomain)) {
                     matchingDataRecords.append(WTFMove(dataRecord));
-                    domainsWithMatchingDataRecords.append(topPrivatelyControlledDomain);
+                    domainsWithMatchingDataRecords.add(topPrivatelyControlledDomain);
                     break;
                 }
             }
@@ -1076,9 +1076,9 @@
     callbackAggregator->callIfNeeded();
 }
 
-void WebsiteDataStore::removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<String>&&)>&& completionHandler)
+void WebsiteDataStore::removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(HashSet<String>&&)>&& completionHandler)
 {
-    fetchDataForTopPrivatelyControlledDomains(dataTypes, fetchOptions, topPrivatelyControlledDomains, [dataTypes, completionHandler = WTFMove(completionHandler), this, protectedThis = makeRef(*this)](Vector<WebsiteDataRecord>&& websiteDataRecords, Vector<String>&& domainsWithDataRecords) mutable {
+    fetchDataForTopPrivatelyControlledDomains(dataTypes, fetchOptions, topPrivatelyControlledDomains, [dataTypes, completionHandler = WTFMove(completionHandler), this, protectedThis = makeRef(*this)](Vector<WebsiteDataRecord>&& websiteDataRecords, HashSet<String>&& domainsWithDataRecords) mutable {
         this->removeData(dataTypes, websiteDataRecords, [domainsWithDataRecords = WTFMove(domainsWithDataRecords), completionHandler = WTFMove(completionHandler)]() mutable {
             completionHandler(WTFMove(domainsWithDataRecords));
         });

Modified: trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h (218826 => 218827)


--- trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h	2017-06-27 03:28:00 UTC (rev 218826)
+++ trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h	2017-06-27 03:33:12 UTC (rev 218827)
@@ -94,11 +94,11 @@
     static void cloneSessionData(WebPageProxy& sourcePage, WebPageProxy& newPage);
 
     void fetchData(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, Function<void(Vector<WebsiteDataRecord>)>&& completionHandler);
-    void fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, Vector<String>&&)>&& completionHandler);
+    void fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, HashSet<String>&&)>&& completionHandler);
     void topPrivatelyControlledDomainsWithWebsiteData(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, Function<void(HashSet<String>&&)>&& completionHandler);
     void removeData(OptionSet<WebsiteDataType>, std::chrono::system_clock::time_point modifiedSince, Function<void()>&& completionHandler);
     void removeData(OptionSet<WebsiteDataType>, const Vector<WebsiteDataRecord>&, Function<void()>&& completionHandler);
-    void removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<String>&&)>&& completionHandler);
+    void removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<String>& topPrivatelyControlledDomains, Function<void(HashSet<String>&&)>&& completionHandler);
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
     void shouldPartitionCookiesForTopPrivatelyOwnedDomains(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to