Skip to content

Commit

Permalink
Add temporary RELEASE_ASSERTs during subframe creation.
Browse files Browse the repository at this point in the history
Trying to track down how we're creating and attaching subframes during
DocumentLoader detachment. This CL adds a temporary flag on Document to track
when its Frame is detaching the DocumentLoader. This flag gets checked during
subframe creation which should never happen while an ancestor Document is being
unloaded.

BUG=519752

Review URL: https://codereview.chromium.org/1420173004

Cr-Commit-Position: refs/heads/master@{#357479}
  • Loading branch information
bokand authored and Commit bot committed Nov 3, 2015
1 parent 1de025a commit 3f68fa5
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
1 change: 1 addition & 0 deletions third_party/WebKit/Source/core/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ class AutofocusTask final : public ExecutionContextTask {
Document::Document(const DocumentInit& initializer, DocumentClassFlags documentClasses)
: ContainerNode(0, CreateDocument)
, TreeScope(*this)
, m_detachingDocumentLoader(false)
, m_hasNodesWithPlaceholderStyle(false)
, m_evaluateMediaQueriesOnStyleRecalc(false)
, m_pendingSheetLayout(NoLayoutWithPendingSheets)
Expand Down
3 changes: 3 additions & 0 deletions third_party/WebKit/Source/core/dom/Document.h
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,9 @@ class CORE_EXPORT Document : public ContainerNode, public TreeScope, public Secu
WebTaskRunner* loadingTaskRunner() const;
WebTaskRunner* timerTaskRunner() const;

// TODO(bokan): Temporary to help track down crash in crbug.com/519752.
bool m_detachingDocumentLoader;

protected:
Document(const DocumentInit&, DocumentClassFlags = DefaultDocumentClass);

Expand Down
22 changes: 22 additions & 0 deletions third_party/WebKit/Source/core/frame/LocalFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,30 @@ inline float parentTextZoomFactor(LocalFrame* frame)

} // namespace

// TODO(bokan): Temporary to help track down crash in crbug.com/519752
static void checkCanLoad(Document* doc)
{
if (!doc)
return;

// I suspect this check wont trip since it's checked a little higher up in the call stack
// in HTMLFrameOwnerElement::loadOrRedirectSubframe, unless we're somehow navigating the
// frame from an unexpected direction.
RELEASE_ASSERT(!doc->unloadStarted());

// I added this flag that gets set to true just before detaching the document loader. This
// should trip and will hopefully illuminate why the loadEventProgress state isn't stopping
// navigation.
RELEASE_ASSERT(!doc->m_detachingDocumentLoader);

checkCanLoad(doc->parentDocument());
}

PassRefPtrWillBeRawPtr<LocalFrame> LocalFrame::create(FrameLoaderClient* client, FrameHost* host, FrameOwner* owner)
{
if (owner && owner->isLocal())
checkCanLoad(&toHTMLFrameOwnerElement(owner)->document());

RefPtrWillBeRawPtr<LocalFrame> frame = adoptRefWillBeNoop(new LocalFrame(client, host, owner));
InspectorInstrumentation::frameAttachedToParent(frame.get());
return frame.release();
Expand Down
8 changes: 8 additions & 0 deletions third_party/WebKit/Source/core/loader/FrameLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1047,8 +1047,16 @@ bool FrameLoader::prepareForCommit()
if (pdl != m_provisionalDocumentLoader)
return false;
if (m_documentLoader) {
// TODO(bokan): Temporarily added this flag to help track down how we're attaching
// new frames during the DocumentLoader detachment. crbug.com/519752.
if (m_frame->document())
m_frame->document()->m_detachingDocumentLoader = true;

FrameNavigationDisabler navigationDisabler(m_frame);
detachDocumentLoader(m_documentLoader);

if (m_frame->document())
m_frame->document()->m_detachingDocumentLoader = false;
}
// detachFromFrame() will abort XHRs that haven't completed, which can
// trigger event listeners for 'abort'. These event listeners might detach
Expand Down

0 comments on commit 3f68fa5

Please sign in to comment.