Commit d6d31fa8 authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot

Explicitly clear LocalDOMWindow::document_ when reusing the window

This ensures that, during navigation commit, it doesn't matter whether
we are reusing a Document or not. Code that needs to catch and handle
that case can null-check LocalDOMWindow::document().

This requires adding a separate LocalDOMWindow::Initialize() step,
because LocalDOMWindow::InstallNewDocument() can no longer detect
whether this a window reuse case or not.

Bug: 1106273
Change-Id: I23021d505d6a2dc2f54f435359ce5490e4ed561e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302939
Auto-Submit: Nate Chapin <japhet@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789525}
parent ea3a0803
...@@ -248,6 +248,12 @@ void LocalDOMWindow::BindContentSecurityPolicy() { ...@@ -248,6 +248,12 @@ void LocalDOMWindow::BindContentSecurityPolicy() {
GetContentSecurityPolicyDelegate()); GetContentSecurityPolicyDelegate());
} }
void LocalDOMWindow::Initialize() {
GetAgent()->AttachContext(this);
if (auto* agent_metrics = GetFrame()->GetPage()->GetAgentMetricsCollector())
agent_metrics->DidAttachWindow(*this);
}
void LocalDOMWindow::AcceptLanguagesChanged() { void LocalDOMWindow::AcceptLanguagesChanged() {
if (navigator_) if (navigator_)
navigator_->SetLanguagesDirty(); navigator_->SetLanguagesDirty();
...@@ -666,27 +672,13 @@ bool LocalDOMWindow::HasInsecureContextInAncestors() { ...@@ -666,27 +672,13 @@ bool LocalDOMWindow::HasInsecureContextInAncestors() {
Document* LocalDOMWindow::InstallNewDocument(const DocumentInit& init) { Document* LocalDOMWindow::InstallNewDocument(const DocumentInit& init) {
DCHECK_EQ(init.GetFrame(), GetFrame()); DCHECK_EQ(init.GetFrame(), GetFrame());
DCHECK(!document_ || !document_->IsActive()); DCHECK(!document_);
bool is_first_document = !document_;
// Explicitly null document_ here so that it is always null when Document's
// constructor is running. This ensures that no code running from the
// constructor obeserves a situation where dom_window_->document() is a
// a different Document.
document_ = nullptr;
document_ = init.CreateDocument(); document_ = init.CreateDocument();
document_->Initialize(); document_->Initialize();
if (!GetFrame()) if (!GetFrame())
return document_; return document_;
if (is_first_document) {
GetAgent()->AttachContext(this);
if (auto* agent_metrics = GetFrame()->GetPage()->GetAgentMetricsCollector())
agent_metrics->DidAttachWindow(*this);
}
GetFrame()->GetScriptController().UpdateDocument(); GetFrame()->GetScriptController().UpdateDocument();
document_->GetViewportData().UpdateViewportDescription(); document_->GetViewportData().UpdateViewportDescription();
if (FrameScheduler* frame_scheduler = GetFrame()->GetFrameScheduler()) { if (FrameScheduler* frame_scheduler = GetFrame()->GetFrameScheduler()) {
......
...@@ -110,6 +110,9 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow, ...@@ -110,6 +110,9 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
LocalFrame* GetFrame() const { return To<LocalFrame>(DOMWindow::GetFrame()); } LocalFrame* GetFrame() const { return To<LocalFrame>(DOMWindow::GetFrame()); }
void Initialize();
void ClearForReuse() { document_ = nullptr; }
// Bind Content Security Policy to this window. This will cause the // Bind Content Security Policy to this window. This will cause the
// CSP to resolve the 'self' attribute and all policies will then be // CSP to resolve the 'self' attribute and all policies will then be
// applied to this document. // applied to this document.
......
...@@ -732,6 +732,7 @@ void LocalFrame::SetDOMWindow(LocalDOMWindow* dom_window) { ...@@ -732,6 +732,7 @@ void LocalFrame::SetDOMWindow(LocalDOMWindow* dom_window) {
} }
GetScriptController().ClearWindowProxy(); GetScriptController().ClearWindowProxy();
dom_window_ = dom_window; dom_window_ = dom_window;
dom_window->Initialize();
} }
Document* LocalFrame::GetDocument() const { Document* LocalFrame::GetDocument() const {
...@@ -1823,6 +1824,7 @@ void LocalFrame::ForceSynchronousDocumentInstall( ...@@ -1823,6 +1824,7 @@ void LocalFrame::ForceSynchronousDocumentInstall(
// Any Document requires Shutdown() before detach, even the initial empty // Any Document requires Shutdown() before detach, even the initial empty
// document. // document.
GetDocument()->Shutdown(); GetDocument()->Shutdown();
DomWindow()->ClearForReuse();
DomWindow()->InstallNewDocument( DomWindow()->InstallNewDocument(
DocumentInit::Create() DocumentInit::Create()
......
...@@ -1688,6 +1688,8 @@ void DocumentLoader::CommitNavigation() { ...@@ -1688,6 +1688,8 @@ void DocumentLoader::CommitNavigation() {
frame_->DomWindow()->SetOriginPolicyIds(ids); frame_->DomWindow()->SetOriginPolicyIds(ids);
} }
} else {
frame_->DomWindow()->ClearForReuse();
} }
// Now that we have the final window and Agent, ensure the security origin has // Now that we have the final window and Agent, ensure the security origin has
......
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