Commit b74c8557 authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

Forbid accessing ScriptLoader from PendingScript

This CL makes ScriptElementBase::Loader() protected, and thus
after this CL PendingScript no longer has access to ScriptLoader,
making separation between PendingScript and ScriptLoader more strict.

XML/HTMLParserScriptRunner previously get ScriptLoader via
ScriptElementBase, but after this CL they get ScriptLoader
directly via ScriptLoaderFromElement().

No behavior changes.

Bug: 842349
Change-Id: I5104c0817669e871a68b8d8286d037e3e23622cc
Reviewed-on: https://chromium-review.googlesource.com/1054855
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561379}
parent e849d4cc
...@@ -50,18 +50,18 @@ namespace { ...@@ -50,18 +50,18 @@ namespace {
// TODO(bmcquade): move this to a shared location if we find ourselves wanting // TODO(bmcquade): move this to a shared location if we find ourselves wanting
// to trace similar data elsewhere in the codebase. // to trace similar data elsewhere in the codebase.
std::unique_ptr<TracedValue> GetTraceArgsForScriptElement( std::unique_ptr<TracedValue> GetTraceArgsForScriptElement(
ScriptElementBase* element, Document& document,
const TextPosition& text_position, const TextPosition& text_position,
const KURL& url) { const KURL& url) {
std::unique_ptr<TracedValue> value = TracedValue::Create(); std::unique_ptr<TracedValue> value = TracedValue::Create();
if (!url.IsNull()) if (!url.IsNull())
value->SetString("url", url.GetString()); value->SetString("url", url.GetString());
if (element->GetDocument().GetFrame()) { if (document.GetFrame()) {
value->SetString( value->SetString(
"frame", "frame",
String::Format("0x%" PRIx64, String::Format("0x%" PRIx64,
static_cast<uint64_t>(reinterpret_cast<intptr_t>( static_cast<uint64_t>(
element->GetDocument().GetFrame())))); reinterpret_cast<intptr_t>(document.GetFrame()))));
} }
if (text_position.line_.ZeroBasedInt() > 0 || if (text_position.line_.ZeroBasedInt() > 0 ||
text_position.column_.ZeroBasedInt() > 0) { text_position.column_.ZeroBasedInt() > 0) {
...@@ -74,9 +74,9 @@ std::unique_ptr<TracedValue> GetTraceArgsForScriptElement( ...@@ -74,9 +74,9 @@ std::unique_ptr<TracedValue> GetTraceArgsForScriptElement(
std::unique_ptr<TracedValue> GetTraceArgsForScriptElement( std::unique_ptr<TracedValue> GetTraceArgsForScriptElement(
const PendingScript* pending_script) { const PendingScript* pending_script) {
DCHECK(pending_script); DCHECK(pending_script);
return GetTraceArgsForScriptElement(pending_script->GetElement(), return GetTraceArgsForScriptElement(
pending_script->StartingPosition(), pending_script->GetElement()->GetDocument(),
pending_script->UrlForTracing()); pending_script->StartingPosition(), pending_script->UrlForTracing());
} }
void DoExecuteScript(PendingScript* pending_script, const KURL& document_url) { void DoExecuteScript(PendingScript* pending_script, const KURL& document_url) {
...@@ -221,7 +221,7 @@ void HTMLParserScriptRunner::ExecutePendingScriptAndDispatchEvent( ...@@ -221,7 +221,7 @@ void HTMLParserScriptRunner::ExecutePendingScriptAndDispatchEvent(
parser_blocking_script_ = nullptr; parser_blocking_script_ = nullptr;
} }
if (pending_script->GetElement()->Loader()) { {
// <spec label="scriptEndTag" step="B.7">Increment the parser's script // <spec label="scriptEndTag" step="B.7">Increment the parser's script
// nesting level by one (it should be zero before this step, so this sets it // nesting level by one (it should be zero before this step, so this sets it
// to one).</spec> // to one).</spec>
...@@ -477,15 +477,11 @@ void HTMLParserScriptRunner::ProcessScriptElementInternal( ...@@ -477,15 +477,11 @@ void HTMLParserScriptRunner::ProcessScriptElementInternal(
DCHECK(document_); DCHECK(document_);
DCHECK(!HasParserBlockingScript()); DCHECK(!HasParserBlockingScript());
{ {
ScriptElementBase* element = ScriptLoader* script_loader = ScriptLoaderFromElement(script);
ScriptElementBase::FromElementIfPossible(script);
DCHECK(element);
ScriptLoader* script_loader = element->Loader();
DCHECK(script_loader);
// FIXME: Align trace event name and function name. // FIXME: Align trace event name and function name.
TRACE_EVENT1("blink", "HTMLParserScriptRunner::execute", "data", TRACE_EVENT1("blink", "HTMLParserScriptRunner::execute", "data",
GetTraceArgsForScriptElement(element, script_start_position, GetTraceArgsForScriptElement(*document_, script_start_position,
NullURL())); NullURL()));
DCHECK(script_loader->IsParserInserted()); DCHECK(script_loader->IsParserInserted());
......
...@@ -9,12 +9,14 @@ ...@@ -9,12 +9,14 @@
namespace blink { namespace blink {
ScriptElementBase* ScriptElementBase::FromElementIfPossible(Element* element) { ScriptLoader* ScriptLoaderFromElement(Element* element) {
ScriptLoader* script_loader = nullptr;
if (auto* html_script = ToHTMLScriptElementOrNull(*element)) if (auto* html_script = ToHTMLScriptElementOrNull(*element))
return html_script; script_loader = html_script->Loader();
if (auto* svg_script = ToSVGScriptElementOrNull(*element)) else if (auto* svg_script = ToSVGScriptElementOrNull(*element))
return svg_script; script_loader = svg_script->Loader();
return nullptr; DCHECK(script_loader);
return script_loader;
} }
ScriptLoader* ScriptElementBase::InitializeScriptLoader(bool parser_inserted, ScriptLoader* ScriptElementBase::InitializeScriptLoader(bool parser_inserted,
......
...@@ -35,10 +35,10 @@ class Element; ...@@ -35,10 +35,10 @@ class Element;
class HTMLScriptElementOrSVGScriptElement; class HTMLScriptElementOrSVGScriptElement;
class ScriptLoader; class ScriptLoader;
ScriptLoader* ScriptLoaderFromElement(Element*);
class CORE_EXPORT ScriptElementBase : public GarbageCollectedMixin { class CORE_EXPORT ScriptElementBase : public GarbageCollectedMixin {
public: public:
static ScriptElementBase* FromElementIfPossible(Element*);
virtual bool AsyncAttributeValue() const = 0; virtual bool AsyncAttributeValue() const = 0;
virtual String CharsetAttributeValue() const = 0; virtual String CharsetAttributeValue() const = 0;
virtual String CrossOriginAttributeValue() const = 0; virtual String CrossOriginAttributeValue() const = 0;
...@@ -66,16 +66,14 @@ class CORE_EXPORT ScriptElementBase : public GarbageCollectedMixin { ...@@ -66,16 +66,14 @@ class CORE_EXPORT ScriptElementBase : public GarbageCollectedMixin {
virtual void SetScriptElementForBinding( virtual void SetScriptElementForBinding(
HTMLScriptElementOrSVGScriptElement&) = 0; HTMLScriptElementOrSVGScriptElement&) = 0;
virtual ScriptLoader* Loader() const = 0; virtual void DispatchLoadEvent() = 0;
virtual void DispatchErrorEvent() = 0;
protected: protected:
ScriptLoader* InitializeScriptLoader(bool parser_inserted, ScriptLoader* InitializeScriptLoader(bool parser_inserted,
bool already_started); bool already_started);
friend class ScriptLoader; virtual ScriptLoader* Loader() const = 0;
friend class PendingScript;
virtual void DispatchLoadEvent() = 0;
virtual void DispatchErrorEvent() = 0;
}; };
} // namespace blink } // namespace blink
......
...@@ -762,7 +762,6 @@ void ScriptLoader::PendingScriptFinished(PendingScript* pending_script) { ...@@ -762,7 +762,6 @@ void ScriptLoader::PendingScriptFinished(PendingScript* pending_script) {
return; return;
} }
DCHECK_EQ(pending_script->GetElement()->Loader(), this);
context_document->GetScriptRunner()->NotifyScriptReady(pending_script); context_document->GetScriptRunner()->NotifyScriptReady(pending_script);
pending_script_->StopWatchingForLoad(); pending_script_->StopWatchingForLoad();
pending_script_ = nullptr; pending_script_ = nullptr;
......
...@@ -67,12 +67,7 @@ void XMLParserScriptRunner::ProcessScriptElement( ...@@ -67,12 +67,7 @@ void XMLParserScriptRunner::ProcessScriptElement(
DCHECK(element); DCHECK(element);
DCHECK(!parser_blocking_script_); DCHECK(!parser_blocking_script_);
ScriptElementBase* script_element_base = ScriptLoader* script_loader = ScriptLoaderFromElement(element);
ScriptElementBase::FromElementIfPossible(element);
CHECK(script_element_base);
ScriptLoader* script_loader = script_element_base->Loader();
DCHECK(script_loader);
// [Parsing] When the element's end tag is subsequently parsed, the user agent // [Parsing] When the element's end tag is subsequently parsed, the user agent
// must perform a microtask checkpoint, and then prepare the script element. // must perform a microtask checkpoint, and then prepare the script element.
......
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