Changeset 247190 in webkit

Timestamp:

Jul 5, 2019 5:33:44 PM (5 years ago)

Author:

Chris Dumez

Message:

Fix thread safety bug in WebResourceLoadStatisticsTelemetry::calculateAndSubmit()
​https://bugs.webkit.org/show_bug.cgi?id=199536

Reviewed by Youenn Fablet.

The resourceLoadStatisticsStore is an object that is created / used / destroyed on the background
queue. It is therefore not safe to create a WeakPtr for it and use that WeakPtr on the main thread.

• NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp:

(WebKit::WebResourceLoadStatisticsTelemetry::calculateAndSubmit):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-05  Chris Dumez  <cdumez@apple.com>
+
+        Fix thread safety bug in WebResourceLoadStatisticsTelemetry::calculateAndSubmit()
+        https://bugs.webkit.org/show_bug.cgi?id=199536
+
+        Reviewed by Youenn Fablet.
+
+        The resourceLoadStatisticsStore is an object that is created / used / destroyed on the background
+        queue. It is therefore not safe to create a WeakPtr for it and use that WeakPtr on the main thread.
+
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp:
+        (WebKit::WebResourceLoadStatisticsTelemetry::calculateAndSubmit):
+
 2019-07-05  Michael Catanzaro  <mcatanzaro@igalia.com>
 

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

@@ -239 +239 @@
     
     // Dispatch on the main thread to make sure the WebPageProxy we're using doesn't go away.
-    RunLoop::main().dispatch([sortedPrevalentResources = WTFMove(sortedPrevalentResources), sortedPrevalentResourcesWithoutUserInteraction = WTFMove(sortedPrevalentResourcesWithoutUserInteraction), prevalentResourcesDaysSinceUserInteraction = WTFMove(prevalentResourcesDaysSinceUserInteraction), resourceLoadStatisticsStore = makeWeakPtr(resourceLoadStatisticsStore)] () {
-        if (!resourceLoadStatisticsStore)
-            return;
-
+    RunLoop::main().dispatch([sortedPrevalentResources = WTFMove(sortedPrevalentResources), sortedPrevalentResourcesWithoutUserInteraction = WTFMove(sortedPrevalentResourcesWithoutUserInteraction), prevalentResourcesDaysSinceUserInteraction = WTFMove(prevalentResourcesDaysSinceUserInteraction), store = makeRef(resourceLoadStatisticsStore.store())] () {
         auto webPageProxy = WebPageProxy::nonEphemeralWebPageProxy();
         if (!webPageProxy) {
             if (notifyPagesWhenTelemetryWasCaptured)
-                notifyPages(0, 0, 0, resourceLoadStatisticsStore->store());
+                notifyPages(0, 0, 0, store);
             return;
         }
         
         if (notifyPagesWhenTelemetryWasCaptured) {
-            notifyPages(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, prevalentResourcesDaysSinceUserInteraction.size(), resourceLoadStatisticsStore->store());
+            notifyPages(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, prevalentResourcesDaysSinceUserInteraction.size(), store);
             // The notify pages function is for testing so we don't need to do an actual submission.
             return;
@@ -264 +261 @@
             webPageProxy->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceLoadStatisticsTelemetryKey(), "medianPrevalentResourcesWithUserInteractionDaysSinceUserInteraction"_s, median(prevalentResourcesDaysSinceUserInteraction), significantFiguresForLoggedValues, ShouldSample::No);
         
-        submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, resourceLoadStatisticsStore->store());
+        submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, store);
     });
 }