Commit b99b7cfb authored by Daniel Cheng's avatar Daniel Cheng Committed by Chromium LUCI CQ

[navigation] Clean up null checks in FrameLoader::DetachDocument()

As of https://crrev.com/c/1684998, FrameLoader::Init() no longer calls
FrameLoader::DetachDocument(). Since this was the only case where the
Document and DocumentLoader could be null, FrameLoader::DetachDocument()
no longer needs to handle this case.

Bug: 855189
Change-Id: I10fe2b6ecc6765e73675676dda72c666401b5f9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617001
Auto-Submit: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841703}
parent cbb76171
...@@ -1140,6 +1140,9 @@ void FrameLoader::DidAccessInitialDocument() { ...@@ -1140,6 +1140,9 @@ void FrameLoader::DidAccessInitialDocument() {
bool FrameLoader::DetachDocument( bool FrameLoader::DetachDocument(
SecurityOrigin* committing_origin, SecurityOrigin* committing_origin,
base::Optional<Document::UnloadEventTiming>* timing) { base::Optional<Document::UnloadEventTiming>* timing) {
DCHECK(frame_->GetDocument());
DCHECK(document_loader_);
PluginScriptForbiddenScope forbid_plugin_destructor_scripting; PluginScriptForbiddenScope forbid_plugin_destructor_scripting;
ClientNavigationState* client_navigation = client_navigation_.get(); ClientNavigationState* client_navigation = client_navigation_.get();
...@@ -1157,8 +1160,7 @@ bool FrameLoader::DetachDocument( ...@@ -1157,8 +1160,7 @@ bool FrameLoader::DetachDocument(
// both when unloading itself and when unloading its descendants. // both when unloading itself and when unloading its descendants.
IgnoreOpensDuringUnloadCountIncrementer ignore_opens_during_unload( IgnoreOpensDuringUnloadCountIncrementer ignore_opens_during_unload(
frame_->GetDocument()); frame_->GetDocument());
if (document_loader_) DispatchUnloadEvent(committing_origin, timing);
DispatchUnloadEvent(committing_origin, timing);
frame_->DetachChildren(); frame_->DetachChildren();
// The previous calls to dispatchUnloadEvent() and detachChildren() can // The previous calls to dispatchUnloadEvent() and detachChildren() can
// execute arbitrary script via things like unload events. If the executed // execute arbitrary script via things like unload events. If the executed
...@@ -1168,14 +1170,13 @@ bool FrameLoader::DetachDocument( ...@@ -1168,14 +1170,13 @@ bool FrameLoader::DetachDocument(
return false; return false;
// FrameNavigationDisabler should prevent another load from starting. // FrameNavigationDisabler should prevent another load from starting.
DCHECK_EQ(client_navigation_.get(), client_navigation); DCHECK_EQ(client_navigation_.get(), client_navigation);
// detachFromFrame() will abort XHRs that haven't completed, which can trigger // Detaching the document loader will abort XHRs that haven't completed, which
// event listeners for 'abort'. These event listeners might call // can trigger event listeners for 'abort'. These event listeners might call
// window.stop(), which will in turn detach the provisional document loader. // window.stop(), which will in turn detach the provisional document loader.
// At this point, the provisional document loader should not detach, because // At this point, the provisional document loader should not detach, because
// then the FrameLoader would not have any attached DocumentLoaders. This is // then the FrameLoader would not have any attached DocumentLoaders. This is
// guaranteed by FrameNavigationDisabler above. // guaranteed by FrameNavigationDisabler above.
if (document_loader_) DetachDocumentLoader(document_loader_, true);
DetachDocumentLoader(document_loader_, true);
// 'abort' listeners can also detach the frame. // 'abort' listeners can also detach the frame.
if (!frame_->Client()) if (!frame_->Client())
return false; return false;
...@@ -1185,8 +1186,7 @@ bool FrameLoader::DetachDocument( ...@@ -1185,8 +1186,7 @@ bool FrameLoader::DetachDocument(
// No more events will be dispatched so detach the Document. // No more events will be dispatched so detach the Document.
// TODO(yoav): Should we also be nullifying domWindow's document (or // TODO(yoav): Should we also be nullifying domWindow's document (or
// domWindow) since the doc is now detached? // domWindow) since the doc is now detached?
if (frame_->GetDocument()) frame_->GetDocument()->Shutdown();
frame_->GetDocument()->Shutdown();
document_loader_ = nullptr; document_loader_ = nullptr;
return true; return true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment