Clean up usage of HTMLImportsController in Document
There are some common usages of Document.ImportsController() to
see if the document is an import or not, or if it is, to access
its master document.
It required some internal knowledge about how HTMLImportsController
is used for a master document and its imports, and made code hard
to understand.
Added some APIs in Document class so its users do not have to
access .ImportsController() directly.
Bug: 673669, 746150
Change-Id: I4850c78988c224fb989045191f44f0f4d9ef0373
Reviewed-on: https://chromium-review.googlesource.com/571109
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Dominic Cooney <dominicc@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498396}diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
index aa260b3..64434b97 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
@@ -15,7 +15,6 @@
#include "core/events/ErrorEvent.h"
#include "core/html/HTMLElement.h"
#include "core/html/custom/CustomElement.h"
-#include "core/html/imports/HTMLImportsController.h"
#include "platform/bindings/ScriptState.h"
#include "platform/bindings/V8BindingMacros.h"
#include "platform/bindings/V8PrivateProperty.h"
@@ -172,10 +171,7 @@
{
v8::TryCatch try_catch(script_state_->GetIsolate());
- bool is_import_document =
- document.ImportsController() &&
- document.ImportsController()->Master() != document;
- if (is_import_document) {
+ if (document.IsHTMLImport()) {
// V8HTMLElement::constructorCustom() can only refer to
// window.document() which is not the import document. Create
// elements in import documents ahead of time so they end up in
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp b/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp
index e11a8d2..758b0aa 100644
--- a/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp
@@ -58,12 +58,8 @@
Node* V8GCController::OpaqueRootForGC(v8::Isolate*, Node* node) {
DCHECK(node);
- if (node->isConnected()) {
- Document& document = node->GetDocument();
- if (HTMLImportsController* controller = document.ImportsController())
- return controller->Master();
- return &document;
- }
+ if (node->isConnected())
+ return &node->GetDocument().MasterDocument();
if (node->IsAttributeNode()) {
Node* owner_element = ToAttr(node)->ownerElement();
diff --git a/third_party/WebKit/Source/core/css/MediaValues.cpp b/third_party/WebKit/Source/core/css/MediaValues.cpp
index b1e30d8..69c64731 100644
--- a/third_party/WebKit/Source/core/css/MediaValues.cpp
+++ b/third_party/WebKit/Source/core/css/MediaValues.cpp
@@ -12,7 +12,6 @@
#include "core/frame/LocalFrame.h"
#include "core/frame/LocalFrameView.h"
#include "core/frame/Settings.h"
-#include "core/html/imports/HTMLImportsController.h"
#include "core/layout/LayoutObject.h"
#include "core/layout/api/LayoutViewItem.h"
#include "core/page/ChromeClient.h"
@@ -234,12 +233,4 @@
}
}
-LocalFrame* MediaValues::FrameFrom(Document& document) {
- Document* executing_document = document.ImportsController()
- ? document.ImportsController()->Master()
- : &document;
- DCHECK(executing_document);
- return executing_document->GetFrame();
-}
-
} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/MediaValues.h b/third_party/WebKit/Source/core/css/MediaValues.h
index b5317831..71be56af 100644
--- a/third_party/WebKit/Source/core/css/MediaValues.h
+++ b/third_party/WebKit/Source/core/css/MediaValues.h
@@ -95,7 +95,6 @@
static int CalculateAvailableHoverTypes(LocalFrame*);
static DisplayShape CalculateDisplayShape(LocalFrame*);
static ColorSpaceGamut CalculateColorGamut(LocalFrame*);
- static LocalFrame* FrameFrom(Document&);
};
} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/MediaValuesCached.cpp b/third_party/WebKit/Source/core/css/MediaValuesCached.cpp
index 5a3681a..0c45e77 100644
--- a/third_party/WebKit/Source/core/css/MediaValuesCached.cpp
+++ b/third_party/WebKit/Source/core/css/MediaValuesCached.cpp
@@ -36,7 +36,7 @@
Document& document)
: MediaValuesCached::MediaValuesCachedData() {
DCHECK(IsMainThread());
- LocalFrame* frame = MediaValues::FrameFrom(document);
+ LocalFrame* frame = document.GetFrameOfMasterDocument();
// TODO(hiroshige): Clean up |frame->view()| conditions.
DCHECK(!frame || frame->View());
if (frame && frame->View()) {
diff --git a/third_party/WebKit/Source/core/css/MediaValuesDynamic.cpp b/third_party/WebKit/Source/core/css/MediaValuesDynamic.cpp
index 068dab914..b34cb85 100644
--- a/third_party/WebKit/Source/core/css/MediaValuesDynamic.cpp
+++ b/third_party/WebKit/Source/core/css/MediaValuesDynamic.cpp
@@ -15,7 +15,7 @@
namespace blink {
MediaValues* MediaValuesDynamic::Create(Document& document) {
- return MediaValuesDynamic::Create(FrameFrom(document));
+ return MediaValuesDynamic::Create(document.GetFrameOfMasterDocument());
}
MediaValues* MediaValuesDynamic::Create(LocalFrame* frame) {
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index f85bd69..4695fde 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -1111,6 +1111,19 @@
return imports_controller_->LoaderFor(*this);
}
+bool Document::IsHTMLImport() const {
+ return imports_controller_ && imports_controller_->Master() != this;
+}
+
+Document& Document::MasterDocument() const {
+ if (!imports_controller_)
+ return *const_cast<Document*>(this);
+
+ Document* master = imports_controller_->Master();
+ DCHECK(master);
+ return *master;
+}
+
bool Document::HaveImportsLoaded() const {
if (!imports_controller_)
return true;
@@ -1793,6 +1806,14 @@
return frame_ ? frame_->GetPage() : nullptr;
}
+LocalFrame* Document::GetFrameOfMasterDocument() const {
+ if (frame_)
+ return frame_;
+ if (imports_controller_)
+ return imports_controller_->Master()->GetFrame();
+ return nullptr;
+}
+
Settings* Document::GetSettings() const {
return frame_ ? frame_->GetSettings() : nullptr;
}
@@ -5103,8 +5124,7 @@
// TODO(mkwst): This doesn't properly handle HTML Import documents.
// If this is an imported document, grab its master document's first-party:
- if (ImportsController() && ImportsController()->Master() &&
- ImportsController()->Master() != this)
+ if (IsHTMLImport())
return ImportsController()->Master()->SiteForCookies();
if (!GetFrame())
diff --git a/third_party/WebKit/Source/core/dom/Document.h b/third_party/WebKit/Source/core/dom/Document.h
index a1c68519..992aa95 100644
--- a/third_party/WebKit/Source/core/dom/Document.h
+++ b/third_party/WebKit/Source/core/dom/Document.h
@@ -486,6 +486,10 @@
LocalFrameView* View() const; // can be null
LocalFrame* GetFrame() const { return frame_; } // can be null
+ // Returns frame_ for current document, or if this is an HTML import, master
+ // document's frame_, if any. Can be null.
+ // TODO(kochi): Audit usage of this interface (crbug.com/746150).
+ LocalFrame* GetFrameOfMasterDocument() const;
Page* GetPage() const; // can be null
Settings* GetSettings() const; // can be null
@@ -1187,6 +1191,10 @@
}
HTMLImportLoader* ImportLoader() const;
+ bool IsHTMLImport() const;
+ // TODO(kochi): Audit usage of this interface (crbug.com/746150).
+ Document& MasterDocument() const;
+
void DidLoadAllImports();
void AdjustFloatQuadsForScrollAndAbsoluteZoom(Vector<FloatQuad>&,
diff --git a/third_party/WebKit/Source/core/dom/StyleEngine.cpp b/third_party/WebKit/Source/core/dom/StyleEngine.cpp
index 65b64eb..096dd6c 100644
--- a/third_party/WebKit/Source/core/dom/StyleEngine.cpp
+++ b/third_party/WebKit/Source/core/dom/StyleEngine.cpp
@@ -65,8 +65,7 @@
StyleEngine::StyleEngine(Document& document)
: document_(&document),
- is_master_(!document.ImportsController() ||
- document.ImportsController()->Master() == &document),
+ is_master_(!document.IsHTMLImport()),
document_style_sheet_collection_(
DocumentStyleSheetCollection::Create(document)) {
if (document.GetFrame()) {
@@ -87,7 +86,8 @@
if (IsMaster())
return document_;
HTMLImportsController* import = GetDocument().ImportsController();
- // Document::import() can return null while executing its destructor.
+ // Document::ImportsController() can return null while executing its
+ // destructor.
if (!import)
return nullptr;
return import->Master();
@@ -548,11 +548,7 @@
if (RuntimeEnabledFeatures::CSSViewportEnabled())
ViewportRulesChanged();
if (GetDocument().ImportLoader())
- GetDocument()
- .ImportsController()
- ->Master()
- ->GetStyleEngine()
- .MarkDocumentDirty();
+ GetDocument().MasterDocument().GetStyleEngine().MarkDocumentDirty();
else
GetDocument().ScheduleLayoutTreeUpdateIfNeeded();
}
@@ -1037,11 +1033,7 @@
void StyleEngine::HtmlImportAddedOrRemoved() {
if (GetDocument().ImportLoader()) {
- GetDocument()
- .ImportsController()
- ->Master()
- ->GetStyleEngine()
- .HtmlImportAddedOrRemoved();
+ GetDocument().MasterDocument().GetStyleEngine().HtmlImportAddedOrRemoved();
return;
}
diff --git a/third_party/WebKit/Source/core/html/LinkResource.cpp b/third_party/WebKit/Source/core/html/LinkResource.cpp
index 1079910..cfe86e6 100644
--- a/third_party/WebKit/Source/core/html/LinkResource.cpp
+++ b/third_party/WebKit/Source/core/html/LinkResource.cpp
@@ -33,7 +33,6 @@
#include "core/HTMLNames.h"
#include "core/dom/Document.h"
#include "core/html/HTMLLinkElement.h"
-#include "core/html/imports/HTMLImportsController.h"
namespace blink {
@@ -48,10 +47,7 @@
}
LocalFrame* LinkResource::LoadingFrame() const {
- HTMLImportsController* imports_controller = GetDocument().ImportsController();
- if (!imports_controller)
- return GetDocument().GetFrame();
- return imports_controller->Master()->GetFrame();
+ return owner_->GetDocument().MasterDocument().GetFrame();
}
Document& LinkResource::GetDocument() {
