Changeset 240346 in webkit

Timestamp:

Jan 23, 2019 10:44:34 AM (5 years ago)

Author:

Brent Fulgham

Message:

REGRESSION (r240243): CrashTracer: WebKitTestRunnerApp at com.apple.WebKit: WebKit::WebResourceLoadStatisticsStore::sendDiagnosticMessageWithValue const + 32
​https://bugs.webkit.org/show_bug.cgi?id=193723
<rdar://problem/47476802>

Reviewed by David Kilzer.

The new code added in r240243 could attempt to submit telemetry after the relevant
WebResourceLoadStatisticsStore was destroyed. We should guard against this possibility.

• 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-01-23  Brent Fulgham  <bfulgham@apple.com>
+
+        REGRESSION (r240243): CrashTracer: WebKitTestRunnerApp at com.apple.WebKit: WebKit::WebResourceLoadStatisticsStore::sendDiagnosticMessageWithValue const + 32
+        https://bugs.webkit.org/show_bug.cgi?id=193723
+        <rdar://problem/47476802>
+
+        Reviewed by David Kilzer.
+
+        The new code added in r240243 could attempt to submit telemetry after the relevant
+        WebResourceLoadStatisticsStore was destroyed. We should guard against this possibility.
+
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp:
+        (WebKit::WebResourceLoadStatisticsTelemetry::calculateAndSubmit):
+
 2019-01-23  Antti Koivisto  <antti@apple.com>
 

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

@@ -242 +242 @@
     
     // 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] () {
+    RunLoop::main().dispatch([sortedPrevalentResources = WTFMove(sortedPrevalentResources), sortedPrevalentResourcesWithoutUserInteraction = WTFMove(sortedPrevalentResourcesWithoutUserInteraction), prevalentResourcesDaysSinceUserInteraction = WTFMove(prevalentResourcesDaysSinceUserInteraction), resourceLoadStatisticsStore = makeWeakPtr(resourceLoadStatisticsStore)] () {
+        if (!resourceLoadStatisticsStore)
+            return;
+
         auto webPageProxy = WebPageProxy::nonEphemeralWebPageProxy();
         if (!webPageProxy) {
             if (notifyPagesWhenTelemetryWasCaptured)
-                notifyPages(0, 0, 0, resourceLoadStatisticsStore.store());
+                notifyPages(0, 0, 0, resourceLoadStatisticsStore->store());
             return;
         }
         
         if (notifyPagesWhenTelemetryWasCaptured) {
-            notifyPages(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, prevalentResourcesDaysSinceUserInteraction.size(), resourceLoadStatisticsStore.store());
+            notifyPages(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, prevalentResourcesDaysSinceUserInteraction.size(), resourceLoadStatisticsStore->store());
             // The notify pages function is for testing so we don't need to do an actual submission.
             return;
@@ -264 +267 @@
             webPageProxy->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceLoadStatisticsTelemetryKey(), "medianPrevalentResourcesWithUserInteractionDaysSinceUserInteraction"_s, median(prevalentResourcesDaysSinceUserInteraction), significantFiguresForLoggedValues, ShouldSample::No);
         
-        submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, resourceLoadStatisticsStore.store());
+        submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, resourceLoadStatisticsStore->store());
     });
 }