Title: [244971] trunk/Source/WebCore
Revision
244971
Author
cdu...@apple.com
Date
2019-05-06 13:25:27 -0700 (Mon, 06 May 2019)

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

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (244970 => 244971)


--- trunk/Source/WebCore/ChangeLog	2019-05-06 20:24:29 UTC (rev 244970)
+++ trunk/Source/WebCore/ChangeLog	2019-05-06 20:25:27 UTC (rev 244971)
@@ -1,3 +1,20 @@
+2019-05-06  Chris Dumez  <cdu...@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  <zdober...@igalia.com>
 
         [GLib] WebCore::MainThreadSharedTimer should use the appropriate GSource priority, name

Modified: trunk/Source/WebCore/dom/Document.cpp (244970 => 244971)


--- trunk/Source/WebCore/dom/Document.cpp	2019-05-06 20:24:29 UTC (rev 244970)
+++ trunk/Source/WebCore/dom/Document.cpp	2019-05-06 20:25:27 UTC (rev 244971)
@@ -8130,6 +8130,10 @@
 
 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);
 }
 

Modified: trunk/Source/WebCore/dom/Document.h (244970 => 244971)


--- trunk/Source/WebCore/dom/Document.h	2019-05-06 20:24:29 UTC (rev 244970)
+++ trunk/Source/WebCore/dom/Document.h	2019-05-06 20:25:27 UTC (rev 244971)
@@ -1462,6 +1462,9 @@
     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 @@
 
     bool m_hasEvaluatedUserAgentScripts { false };
     bool m_isRunningUserScripts { false };
+    bool m_mayBeDetachedFromFrame { true };
 #if ENABLE(APPLE_PAY)
     bool m_hasStartedApplePaySession { false };
 #endif

Modified: trunk/Source/WebCore/history/CachedFrame.cpp (244970 => 244971)


--- trunk/Source/WebCore/history/CachedFrame.cpp	2019-05-06 20:24:29 UTC (rev 244970)
+++ trunk/Source/WebCore/history/CachedFrame.cpp	2019-05-06 20:25:27 UTC (rev 244971)
@@ -143,10 +143,20 @@
     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 @@
     }
 #endif
 
+    m_document->setMayBeDetachedFromFrame(true);
     m_document->detachFromCachedFrame(*this);
 
     ASSERT_WITH_SECURITY_IMPLICATION(!m_documentLoader->isLoading());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to