Attachment 369155 Details for Bug 197621 – Patch

[patch] Patch

bug-197621-20190506121953.patch (text/plain), 4.03 KB, created by Chris Dumez on 2019-05-06 12:19:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-06 12:19:53 PDT

Size: 4.03 KB

patch

obsolete

>Subversion Revision: 244966
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c6c1f7679f545d94a40166b2a4b7d364ff85b7b1..7885540541bd59400f613c8481290db9f171fb91 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-05-06  Chris Dumez  <cdumez@apple.com>
>+
>+        Add assertions to CachedFrame to help figure out crash in CachedFrame constructor
>+        https://bugs.webkit.org/show_bug.cgi?id=197621
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add release assertions to try and figure out who is sometimes detaching the document from its
>+        frame while constructing CachedFrames for its descendants.
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::detachFromFrame):
>+        * dom/Document.h:
>+        (WebCore::Document::setMayBeDetachedFromFrame):
>+        * history/CachedFrame.cpp:
>+        (WebCore::CachedFrame::CachedFrame):
>+
> 2019-05-06  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [GLib] WebCore::MainThreadSharedTimer should use the appropriate GSource priority, name
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 9136d9cf78c5c109605c39b54b0a0f6a3fd9c230..966179dfc2424a26472ec9f1c5420378bd89a29d 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -8130,6 +8130,10 @@ bool Document::registerCSSProperty(CSSRegisteredCustomProperty&& prop)
> 
> void Document::detachFromFrame()
> {
>+    // Assertion to help pinpint rdar://problem/49877867. If this hits, the crash trace should tell us
>+    // which piece of code is detaching the document from its frame while constructing the CachedFrames.
>+    RELEASE_ASSERT(m_mayBeDetachedFromFrame);
>+
>     observeFrame(nullptr);
> }
> 
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 941d5f5dbdf53ff042aad02301c256c6e62d0dc5..dddc83d09b94c58517c4a9a32e6f6e527002715a 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -1462,6 +1462,9 @@ public:
>     TextAutoSizing& textAutoSizing();
> #endif
> 
>+    // For debugging rdar://problem/49877867.
>+    void setMayBeDetachedFromFrame(bool mayBeDetachedFromFrame) { m_mayBeDetachedFromFrame = mayBeDetachedFromFrame; }
>+
>     Logger& logger();
> 
>     void hasStorageAccess(Ref<DeferredPromise>&& passedPromise);
>@@ -2059,6 +2062,7 @@ private:
> 
>     bool m_hasEvaluatedUserAgentScripts { false };
>     bool m_isRunningUserScripts { false };
>+    bool m_mayBeDetachedFromFrame { true };
> #if ENABLE(APPLE_PAY)
>     bool m_hasStartedApplePaySession { false };
> #endif
>diff --git a/Source/WebCore/history/CachedFrame.cpp b/Source/WebCore/history/CachedFrame.cpp
>index e461649960db3a54c6f0614ffdc6617b803b4407..6aba92803e06beeee001921f1babd90a63f5ac82 100644
>--- a/Source/WebCore/history/CachedFrame.cpp
>+++ b/Source/WebCore/history/CachedFrame.cpp
>@@ -143,10 +143,20 @@ CachedFrame::CachedFrame(Frame& frame)
>     ASSERT(m_view);
>     ASSERT(m_document->pageCacheState() == Document::InPageCache);
> 
>+    RELEASE_ASSERT(m_document->domWindow());
>+    RELEASE_ASSERT(m_document->frame());
>+    RELEASE_ASSERT(m_document->domWindow()->frame());
>+
>+    // FIXME: We have evidence that constructing CachedFrames for descendant frames may detach the document from its frame (rdar://problem/49877867).
>+    // This sets the flag to help find the guilty code.
>+    m_document->setMayBeDetachedFromFrame(false);
>+
>     // Create the CachedFrames for all Frames in the FrameTree.
>     for (Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
>         m_childFrames.append(std::make_unique<CachedFrame>(*child));
> 
>+    RELEASE_ASSERT(m_document->domWindow());
>+    RELEASE_ASSERT(m_document->frame());
>     RELEASE_ASSERT(m_document->domWindow()->frame());
> 
>     // Active DOM objects must be suspended before we cache the frame script data.
>@@ -193,6 +203,7 @@ CachedFrame::CachedFrame(Frame& frame)
>     }
> #endif
> 
>+    m_document->setMayBeDetachedFromFrame(true);
>     m_document->detachFromCachedFrame(*this);
> 
>     ASSERT_WITH_SECURITY_IMPLICATION(!m_documentLoader->isLoading());

Actions: View | Formatted Diff | Diff

Attachments on bug 197621: 369155