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

Remove return value of ExecuteScriptBlock()

This makes PrepareScript() return true (previously returning false)
for a classic inline script blocked due to CSP (*).

This causes |script_element_ = nullptr| only
in XMLDocumentParser::EndElementNs() and thus no behavior changes,
because this return value is only used in EndElementNs(),
and ReadyToBeParserExecuted() and WillBeParserExecuted() are false
in such (*) cases.

Bug: 686281
Change-Id: If9bb3f9dc89ba46efd6c24a24fc8727a0174c403
Reviewed-on: https://chromium-review.googlesource.com/564200Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491495}
parent 274d243b
...@@ -683,8 +683,9 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position, ...@@ -683,8 +683,9 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
? element_document.Url() ? element_document.Url()
: KURL(); : KURL();
return ExecuteScriptBlock(ClassicPendingScript::Create(element_, position), ExecuteScriptBlock(ClassicPendingScript::Create(element_, position),
script_url); script_url);
return true;
} }
bool ScriptLoader::FetchClassicScript( bool ScriptLoader::FetchClassicScript(
...@@ -892,7 +893,7 @@ void ScriptLoader::Execute() { ...@@ -892,7 +893,7 @@ void ScriptLoader::Execute() {
} }
// https://html.spec.whatwg.org/#execute-the-script-block // https://html.spec.whatwg.org/#execute-the-script-block
bool ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script, void ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script,
const KURL& document_url) { const KURL& document_url) {
DCHECK(pending_script); DCHECK(pending_script);
DCHECK_EQ(pending_script->IsExternal(), is_external_script_); DCHECK_EQ(pending_script->IsExternal(), is_external_script_);
...@@ -909,17 +910,17 @@ bool ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script, ...@@ -909,17 +910,17 @@ bool ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script,
Document* context_document = element_document->ContextDocument(); Document* context_document = element_document->ContextDocument();
if (original_document_ != context_document && if (original_document_ != context_document &&
script->GetScriptType() == ScriptType::kModule) script->GetScriptType() == ScriptType::kModule)
return false; return;
// 2. "If the script's script is null, fire an event named error at the // 2. "If the script's script is null, fire an event named error at the
// element, and abort these steps." // element, and abort these steps."
if (error_occurred) { if (error_occurred) {
DispatchErrorEvent(); DispatchErrorEvent();
return false; return;
} }
if (was_canceled) if (was_canceled)
return false; return;
double script_exec_start_time = MonotonicallyIncreasingTime(); double script_exec_start_time = MonotonicallyIncreasingTime();
...@@ -943,20 +944,17 @@ bool ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script, ...@@ -943,20 +944,17 @@ bool ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script,
// load at the script element." // load at the script element."
if (is_external) if (is_external)
DispatchLoadEvent(); DispatchLoadEvent();
return true; break;
case ExecuteScriptResult::kShouldFireErrorEvent: case ExecuteScriptResult::kShouldFireErrorEvent:
// Consider as if "the script's script is null" retrospectively, // Consider as if "the script's script is null" retrospectively,
// due to CSP check failures etc., which are considered as load failure. // due to CSP check failures etc., which are considered as load failure.
DispatchErrorEvent(); DispatchErrorEvent();
return false; break;
case ExecuteScriptResult::kShouldFireNone: case ExecuteScriptResult::kShouldFireNone:
return true; break;
} }
NOTREACHED();
return false;
} }
void ScriptLoader::PendingScriptFinished(PendingScript* pending_script) { void ScriptLoader::PendingScriptFinished(PendingScript* pending_script) {
......
...@@ -84,12 +84,7 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>, ...@@ -84,12 +84,7 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
// https://html.spec.whatwg.org/#execute-the-script-block // https://html.spec.whatwg.org/#execute-the-script-block
// The single entry point of script execution. // The single entry point of script execution.
// PendingScript::Dispose() is called in ExecuteScriptBlock(). // PendingScript::Dispose() is called in ExecuteScriptBlock().
// void ExecuteScriptBlock(PendingScript*, const KURL&);
// TODO(hiroshige): Replace ExecuteScript() calls with ExecuteScriptBlock().
//
// TODO(hiroshige): Currently this returns bool (true if success) only to
// preserve existing code structure around PrepareScript(). Clean up this.
bool ExecuteScriptBlock(PendingScript*, const KURL&);
// Creates a PendingScript for external script whose fetch is started in // Creates a PendingScript for external script whose fetch is started in
// FetchClassicScript()/FetchModuleScriptTree(). // FetchClassicScript()/FetchModuleScriptTree().
......
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