Changeset 244971 in webkit

Timestamp:

May 6, 2019 1:25:27 PM (5 years ago)

Author:

Chris Dumez

Message:

Add assertions to CachedFrame to help figure out crash in CachedFrame constructor
​https://bugs.webkit.org/show_bug.cgi?id=197621

Reviewed by Geoffrey Garen.

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (1 diff)
• dom/Document.h (modified) (2 diffs)
• history/CachedFrame.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Geoffrey Garen.
+
+        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>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -8131 +8131 @@
 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);
 }

trunk/Source/WebCore/dom/Document.h

@@ -1463 +1463 @@
 #endif
 
+    // For debugging rdar://problem/49877867.
+    void setMayBeDetachedFromFrame(bool mayBeDetachedFromFrame) { m_mayBeDetachedFromFrame = mayBeDetachedFromFrame; }
+
     Logger& logger();
 
@@ -2060 +2063 @@
     bool m_hasEvaluatedUserAgentScripts { false };
     bool m_isRunningUserScripts { false };
+    bool m_mayBeDetachedFromFrame { true };
 #if ENABLE(APPLE_PAY)
     bool m_hasStartedApplePaySession { false };

trunk/Source/WebCore/history/CachedFrame.cpp

@@ -144 +144 @@
     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());
 
@@ -194 +204 @@
 #endif
 
+    m_document->setMayBeDetachedFromFrame(true);
     m_document->detachFromCachedFrame(*this);