Attachment 373543 Details for Bug 199536 – Patch

[patch] Patch

bug-199536-20190705150829.patch (text/plain), 3.96 KB, created by Chris Dumez on 2019-07-05 15:08:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-07-05 15:08:31 PDT

Size: 3.96 KB

patch

obsolete

>Subversion Revision: 247155
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9a39854285d381f6d12c7f1f03811802823981ba..91fc947cc5990e1d7b16e3c792133d95c11e008a 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,16 @@
>+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 NOBODY (OOPS!).
>+
>+        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-04  Chris Dumez  <cdumez@apple.com>
> 
>         Simplify logic that handles registering WebProcessProxy objects with their WebsiteDataStore
>diff --git a/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp b/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp
>index 75391582b1a586355ea67ff9af24878c4095a035..1c0e776c19054c1338f4251b44b73461546921f2 100644
>--- a/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp
>+++ b/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsTelemetry.cpp
>@@ -238,19 +238,16 @@ void WebResourceLoadStatisticsTelemetry::calculateAndSubmit(const ResourceLoadSt
>     }
>     
>     // 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;
>         }
>@@ -263,7 +260,7 @@ void WebResourceLoadStatisticsTelemetry::calculateAndSubmit(const ResourceLoadSt
>         if (prevalentResourcesDaysSinceUserInteraction.size() > 1)
>             webPageProxy->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceLoadStatisticsTelemetryKey(), "medianPrevalentResourcesWithUserInteractionDaysSinceUserInteraction"_s, median(prevalentResourcesDaysSinceUserInteraction), significantFiguresForLoggedValues, ShouldSample::No);
>         
>-        submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, resourceLoadStatisticsStore->store());
>+        submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, store);
>     });
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 199536: 373543 | 373553