Attachment 362547 Details for Bug 194871 – Fix attempt

[patch] Fix attempt

bug-194871-20190220145716.patch (text/plain), 4.80 KB, created by Ryosuke Niwa on 2019-02-20 14:57:17 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-02-20 14:57:17 PST

Size: 4.80 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 241840)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2019-02-20  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Crash in DOMWindowExtension::suspendForPageCache
>+        https://bugs.webkit.org/show_bug.cgi?id=194871
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This is a speculative fix for a crash in DOMWindowExtension::suspendForPageCache.
>+
>+        We think it's possible for DOMWindowExtension::suspendForPageCache notifying the clients via
>+        dispatchWillDisconnectDOMWindowExtensionFromGlobalObject to remove other DOMWindowExtension's.
>+        Check that each DOMWindowProperty is still in m_properties before invoking suspendForPageCache
>+        to avoid the crash.
>+
>+        * page/DOMWindow.cpp:
>+        (WebCore::DOMWindow::willDestroyCachedFrame):
>+        (WebCore::DOMWindow::willDestroyDocumentInFrame):
>+        (WebCore::DOMWindow::willDetachDocumentFromFrame):
>+        (WebCore::DOMWindow::suspendForPageCache):
>+        (WebCore::DOMWindow::resumeFromPageCache):
>+        * page/DOMWindowExtension.cpp:
>+        (WebCore::DOMWindowExtension::suspendForPageCache):
>+
> 2019-02-18  Babak Shafiei  <bshafiei@apple.com>
> 
>         Apply patch. rdar://problem/48122553
>Index: Source/WebCore/page/DOMWindow.cpp
>===================================================================
>--- Source/WebCore/page/DOMWindow.cpp	(revision 241829)
>+++ Source/WebCore/page/DOMWindow.cpp	(working copy)
>@@ -456,16 +456,20 @@ void DOMWindow::willDestroyCachedFrame()
> {
>     // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
>     // unregister themselves from the DOMWindow as a result of the call to willDestroyGlobalObjectInCachedFrame.
>-    for (auto& property : copyToVector(m_properties))
>-        property->willDestroyGlobalObjectInCachedFrame();
>+    for (auto* property : copyToVector(m_properties)) {
>+        if (m_properties.contains(property))
>+            property->willDestroyGlobalObjectInCachedFrame();
>+    }
> }
> 
> void DOMWindow::willDestroyDocumentInFrame()
> {
>     // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
>     // unregister themselves from the DOMWindow as a result of the call to willDestroyGlobalObjectInFrame.
>-    for (auto& property : copyToVector(m_properties))
>-        property->willDestroyGlobalObjectInFrame();
>+    for (auto* property : copyToVector(m_properties)) {
>+        if (m_properties.contains(property))
>+            property->willDestroyGlobalObjectInFrame();
>+    }
> }
> 
> void DOMWindow::willDetachDocumentFromFrame()
>@@ -475,8 +479,10 @@ void DOMWindow::willDetachDocumentFromFr
> 
>     // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
>     // unregister themselves from the DOMWindow as a result of the call to willDetachGlobalObjectFromFrame.
>-    for (auto& property : copyToVector(m_properties))
>-        property->willDetachGlobalObjectFromFrame();
>+    for (auto& property : copyToVector(m_properties)) {
>+        if (m_properties.contains(property))
>+            property->willDetachGlobalObjectFromFrame();
>+    }
> 
>     if (m_performance)
>         m_performance->clearResourceTimings();
>@@ -520,16 +526,20 @@ void DOMWindow::resetUnlessSuspendedForD
> 
> void DOMWindow::suspendForPageCache()
> {
>-    for (auto& property : copyToVector(m_properties))
>-        property->suspendForPageCache();
>+    for (auto* property : copyToVector(m_properties)) {
>+        if (m_properties.contains(property))
>+            property->suspendForPageCache();
>+    }
> 
>     m_suspendedForDocumentSuspension = true;
> }
> 
> void DOMWindow::resumeFromPageCache()
> {
>-    for (auto& property : copyToVector(m_properties))
>-        property->resumeFromPageCache();
>+    for (auto* property : copyToVector(m_properties)) {
>+        if (m_properties.contains(property))
>+            property->resumeFromPageCache();
>+    }
> 
>     m_suspendedForDocumentSuspension = false;
> }
>Index: Source/WebCore/page/DOMWindowExtension.cpp
>===================================================================
>--- Source/WebCore/page/DOMWindowExtension.cpp	(revision 241829)
>+++ Source/WebCore/page/DOMWindowExtension.cpp	(working copy)
>@@ -48,11 +48,11 @@ void DOMWindowExtension::suspendForPageC
>     // Calling out to the client might result in this DOMWindowExtension being destroyed
>     // while there is still work to do.
>     Ref<DOMWindowExtension> protectedThis(*this);
>-    
>-    Frame* frame = this->frame();
>+
>+    auto frame = makeRef(*this->frame());
>     frame->loader().client().dispatchWillDisconnectDOMWindowExtensionFromGlobalObject(this);
> 
>-    m_disconnectedFrame = frame;
>+    m_disconnectedFrame = WTFMove(frame);
> 
>     DOMWindowProperty::suspendForPageCache();
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 194871: 362547