Title: [234036] branches/safari-606-branch/Source/WebKit
Revision
234036
Author
bshaf...@apple.com
Date
2018-07-20 01:05:57 -0700 (Fri, 20 Jul 2018)

Log Message

Cherry-pick r234020. rdar://problem/42417121

    [ITP] Crash under ResourceLoadStatisticsMemoryStore::removeDataRecords()
    https://bugs.webkit.org/show_bug.cgi?id=187821
    <rdar://problem/42112693>

    Reviewed by David Kilzer.

    In two cases, ResourceLoadStatisticsMemoryStore (which lives on a background queue) needs to call WebPageProxy
    operations on the main thread and then dispatch back on the background queue when the operation completes.
    However, it is possible for the ResourceLoadStatisticsMemoryStore to get destroyed on the background queue
    during this time and we would then crash when trying to use m_workQueue to re-dispatch. To address the issue,
    I now ref the work queue in the lambda so that we're guaranteed to be able to re-dispatch to the background
    queue. When we're back on the background queue, we'll realize that weakThis in gone and we'll call the callback
    and return early.

    Note that I am not checking weakThis on the main thread as this would not be safe. weakThis should only be
    used on the background queue.

    * UIProcess/ResourceLoadStatisticsMemoryStore.cpp:
    (WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords):
    (WebKit::ResourceLoadStatisticsMemoryStore::grandfatherExistingWebsiteData):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234020 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-606-branch/Source/WebKit/ChangeLog (234035 => 234036)


--- branches/safari-606-branch/Source/WebKit/ChangeLog	2018-07-20 08:05:55 UTC (rev 234035)
+++ branches/safari-606-branch/Source/WebKit/ChangeLog	2018-07-20 08:05:57 UTC (rev 234036)
@@ -1,5 +1,56 @@
 2018-07-20  Babak Shafiei  <bshaf...@apple.com>
 
+        Cherry-pick r234020. rdar://problem/42417121
+
+    [ITP] Crash under ResourceLoadStatisticsMemoryStore::removeDataRecords()
+    https://bugs.webkit.org/show_bug.cgi?id=187821
+    <rdar://problem/42112693>
+    
+    Reviewed by David Kilzer.
+    
+    In two cases, ResourceLoadStatisticsMemoryStore (which lives on a background queue) needs to call WebPageProxy
+    operations on the main thread and then dispatch back on the background queue when the operation completes.
+    However, it is possible for the ResourceLoadStatisticsMemoryStore to get destroyed on the background queue
+    during this time and we would then crash when trying to use m_workQueue to re-dispatch. To address the issue,
+    I now ref the work queue in the lambda so that we're guaranteed to be able to re-dispatch to the background
+    queue. When we're back on the background queue, we'll realize that weakThis in gone and we'll call the callback
+    and return early.
+    
+    Note that I am not checking weakThis on the main thread as this would not be safe. weakThis should only be
+    used on the background queue.
+    
+    * UIProcess/ResourceLoadStatisticsMemoryStore.cpp:
+    (WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords):
+    (WebKit::ResourceLoadStatisticsMemoryStore::grandfatherExistingWebsiteData):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234020 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-07-19  Chris Dumez  <cdu...@apple.com>
+
+            [ITP] Crash under ResourceLoadStatisticsMemoryStore::removeDataRecords()
+            https://bugs.webkit.org/show_bug.cgi?id=187821
+            <rdar://problem/42112693>
+
+            Reviewed by David Kilzer.
+
+            In two cases, ResourceLoadStatisticsMemoryStore (which lives on a background queue) needs to call WebPageProxy
+            operations on the main thread and then dispatch back on the background queue when the operation completes.
+            However, it is possible for the ResourceLoadStatisticsMemoryStore to get destroyed on the background queue
+            during this time and we would then crash when trying to use m_workQueue to re-dispatch. To address the issue,
+            I now ref the work queue in the lambda so that we're guaranteed to be able to re-dispatch to the background
+            queue. When we're back on the background queue, we'll realize that weakThis in gone and we'll call the callback
+            and return early.
+
+            Note that I am not checking weakThis on the main thread as this would not be safe. weakThis should only be
+            used on the background queue.
+
+            * UIProcess/ResourceLoadStatisticsMemoryStore.cpp:
+            (WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords):
+            (WebKit::ResourceLoadStatisticsMemoryStore::grandfatherExistingWebsiteData):
+
+2018-07-20  Babak Shafiei  <bshaf...@apple.com>
+
         Cherry-pick r233992. rdar://problem/42417109
 
     Update iOS fullscreen alert text again

Modified: branches/safari-606-branch/Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp (234035 => 234036)


--- branches/safari-606-branch/Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp	2018-07-20 08:05:55 UTC (rev 234035)
+++ branches/safari-606-branch/Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp	2018-07-20 08:05:57 UTC (rev 234036)
@@ -245,8 +245,8 @@
     setDataRecordsBeingRemoved(true);
 
     RunLoop::main().dispatch([prevalentResourceDomains = crossThreadCopy(prevalentResourceDomains), callback = WTFMove(callback), this, weakThis = makeWeakPtr(*this)] () mutable {
-        WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(prevalentResourceDomains), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [callback = WTFMove(callback), this, weakThis = WTFMove(weakThis)](const HashSet<String>& domainsWithDeletedWebsiteData) mutable {
-            m_workQueue->dispatch([topDomains = crossThreadCopy(domainsWithDeletedWebsiteData), callback = WTFMove(callback), this, weakThis = WTFMove(weakThis)] () mutable {
+        WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(prevalentResourceDomains), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [callback = WTFMove(callback), this, weakThis = WTFMove(weakThis), workQueue = m_workQueue.copyRef()](const HashSet<String>& domainsWithDeletedWebsiteData) mutable {
+            workQueue->dispatch([topDomains = crossThreadCopy(domainsWithDeletedWebsiteData), callback = WTFMove(callback), this, weakThis = WTFMove(weakThis)] () mutable {
                 if (!weakThis) {
                     callback();
                     return;
@@ -486,8 +486,8 @@
     RunLoop::main().dispatch([this, weakThis = makeWeakPtr(*this), callback = WTFMove(callback)] () mutable {
         // FIXME: This method being a static call on WebProcessProxy is wrong.
         // It should be on the data store that this object belongs to.
-        WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData(WebResourceLoadStatisticsStore::monitoredDataTypes(), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [this, weakThis = WTFMove(weakThis), callback = WTFMove(callback)] (HashSet<String>&& topPrivatelyControlledDomainsWithWebsiteData) mutable {
-            m_workQueue->dispatch([this, weakThis = WTFMove(weakThis), topDomains = crossThreadCopy(topPrivatelyControlledDomainsWithWebsiteData), callback = WTFMove(callback)] () mutable {
+        WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData(WebResourceLoadStatisticsStore::monitoredDataTypes(), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [this, weakThis = WTFMove(weakThis), callback = WTFMove(callback), workQueue = m_workQueue.copyRef()] (HashSet<String>&& topPrivatelyControlledDomainsWithWebsiteData) mutable {
+            workQueue->dispatch([this, weakThis = WTFMove(weakThis), topDomains = crossThreadCopy(topPrivatelyControlledDomainsWithWebsiteData), callback = WTFMove(callback)] () mutable {
                 if (!weakThis) {
                     callback();
                     return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to