Changeset 218753 in webkit

Timestamp:

Jun 23, 2017 11:05:22 AM (7 years ago)

Author:

Brent Fulgham

Message:

Avoid moving the same vector multiple times
​https://bugs.webkit.org/show_bug.cgi?id=173748
<rdar://problem/32936804>

Reviewed by Chris Dumez.

We discovered that a Vector<String> was being moved inside a loop, causing it to be moved more than once.
We should never do this!

• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores):
Do not perform a move at each step of the iteration.

• UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains): Receive vector of top privately
controlled domains as a const reference. Copy this vector into the completion handler. Do not move
origins out of the vector in the inner loop.
(WebKit::WebsiteDataStore::removeDataForTopPrivatelyControlledDomains): Receive vector of top privately
controlled domains as a const reference.

• UIProcess/WebsiteData/WebsiteDataStore.h:

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebProcessProxy.cpp (modified) (1 diff)
• UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (2 diffs)
• UIProcess/WebsiteData/WebsiteDataStore.h (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-06-23  Brent Fulgham  <bfulgham@apple.com>
+
+        Avoid moving the same vector multiple times
+        https://bugs.webkit.org/show_bug.cgi?id=173748
+        <rdar://problem/32936804>
+
+        Reviewed by Chris Dumez.
+
+        We discovered that a Vector<String> was being moved inside a loop, causing it to be moved more than once.
+        We should never do this!
+
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores):
+        Do not perform a move at each step of the iteration.
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains): Receive vector of top privately
+        controlled domains as a const reference. Copy this vector into the completion handler. Do not move
+        origins out of the vector in the inner loop.
+        (WebKit::WebsiteDataStore::removeDataForTopPrivatelyControlledDomains): Receive vector of top privately
+        controlled domains as a const reference.
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
 2017-06-23  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp

@@ -258 +258 @@
         visitedSessionIDs.add(dataStore.sessionID());
         callbackAggregator->addPendingCallback();
-        dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, { }, WTFMove(topPrivatelyControlledDomains), [callbackAggregator, shouldNotifyPage, page](Vector<String>&& domainsWithDeletedWebsiteData) {
+        dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, { }, topPrivatelyControlledDomains, [callbackAggregator, shouldNotifyPage, page](Vector<String>&& domainsWithDeletedWebsiteData) {
             // When completing the task, we should be getting called on the main thread.
             ASSERT(isMainThread());

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

@@ -508 +508 @@
 }
 
-void WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, Vector<String>&& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, Vector<String>&&)>&& completionHandler)
-{
-    fetchData(dataTypes, fetchOptions, [topPrivatelyControlledDomains = WTFMove(topPrivatelyControlledDomains), completionHandler = WTFMove(completionHandler)](auto&& existingDataRecords) {
+void WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, Vector<String>&&)>&& completionHandler)
+{
+    fetchData(dataTypes, fetchOptions, [topPrivatelyControlledDomains, completionHandler = WTFMove(completionHandler)](auto&& existingDataRecords) {
         Vector<WebsiteDataRecord> matchingDataRecords;
         Vector<String> domainsWithMatchingDataRecords;
         for (auto&& dataRecord : existingDataRecords) {
-            for (auto&& topPrivatelyControlledDomain : topPrivatelyControlledDomains) {
+            for (auto& topPrivatelyControlledDomain : topPrivatelyControlledDomains) {
                 if (dataRecord.matchesTopPrivatelyControlledDomain(topPrivatelyControlledDomain)) {
                     matchingDataRecords.append(WTFMove(dataRecord));
@@ -1077 +1077 @@
 }
 
-void WebsiteDataStore::removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, Vector<String>&& topPrivatelyControlledDomains, Function<void(Vector<String>&&)>&& completionHandler)
-{
-    fetchDataForTopPrivatelyControlledDomains(dataTypes, fetchOptions, WTFMove(topPrivatelyControlledDomains), [dataTypes, completionHandler = WTFMove(completionHandler), this, protectedThis = makeRef(*this)](Vector<WebsiteDataRecord>&& websiteDataRecords, Vector<String>&& domainsWithDataRecords) mutable {
+void WebsiteDataStore::removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<String>&&)>&& completionHandler)
+{
+    fetchDataForTopPrivatelyControlledDomains(dataTypes, fetchOptions, topPrivatelyControlledDomains, [dataTypes, completionHandler = WTFMove(completionHandler), this, protectedThis = makeRef(*this)](Vector<WebsiteDataRecord>&& websiteDataRecords, Vector<String>&& domainsWithDataRecords) mutable {
         this->removeData(dataTypes, websiteDataRecords, [domainsWithDataRecords = WTFMove(domainsWithDataRecords), completionHandler = WTFMove(completionHandler)]() mutable {
             completionHandler(WTFMove(domainsWithDataRecords));

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

@@ -95 +95 @@
 
     void fetchData(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, Function<void(Vector<WebsiteDataRecord>)>&& completionHandler);
-    void fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, Vector<String>&& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, Vector<String>&&)>&& completionHandler);
+    void fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, Vector<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>, Vector<String>&& topPrivatelyControlledDomains, Function<void(Vector<String>&&)>&& completionHandler);
+    void removeDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<String>&&)>&& completionHandler);
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)