Commit d669a85a authored by Hans Wennborg's avatar Hans Wennborg Committed by Chromium LUCI CQ

Revert "Move CanExecuteScripts() to V8ScriptRunner::CompileAndRunScript()"

This reverts commit 3d519f44.

Reason for revert:
This caused a 15% build time increase (see second bug).

Original change's description:
> Move CanExecuteScripts() to V8ScriptRunner::CompileAndRunScript()
>
> To unify how CanExecuteScripts() is checked across
> classic script evaluation code paths.
>
> This CL doesn't change the behavior.
>
> Bug: 1111134
> Change-Id: If95a32043c52767eda05f16f9630edee26c5e75f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2544938
> Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#832649}

TBR=yukishiino@chromium.org,hiroshige@chromium.org,dom@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1111134, 1155843
Change-Id: I7531522623086a7e9903364288a557341208d84b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576236Reviewed-by: default avatarHans Wennborg <hans@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834014}
parent 84080ed1
...@@ -153,8 +153,7 @@ void ScheduledAction::Execute(ExecutionContext* context) { ...@@ -153,8 +153,7 @@ void ScheduledAction::Execute(ExecutionContext* context) {
script_state_->GetContext(), script_state_->GetContext(),
ScriptSourceCode(code_, ScriptSourceCode(code_,
ScriptSourceLocationType::kEvalForScheduledAction), ScriptSourceLocationType::kEvalForScheduledAction),
KURL(), SanitizeScriptErrors::kDoNotSanitize, ScriptFetchOptions(), KURL(), SanitizeScriptErrors::kDoNotSanitize);
ScriptController::kDoNotExecuteScriptWhenScriptsDisabled);
} else { } else {
WorkerGlobalScope* worker = To<WorkerGlobalScope>(context); WorkerGlobalScope* worker = To<WorkerGlobalScope>(context);
DCHECK(worker->GetThread()->IsCurrentThread()); DCHECK(worker->GetThread()->IsCurrentThread());
......
...@@ -87,17 +87,16 @@ v8::Local<v8::Value> ScriptController::ExecuteScriptAndReturnValue( ...@@ -87,17 +87,16 @@ v8::Local<v8::Value> ScriptController::ExecuteScriptAndReturnValue(
const ScriptSourceCode& source, const ScriptSourceCode& source,
const KURL& base_url, const KURL& base_url,
SanitizeScriptErrors sanitize_script_errors, SanitizeScriptErrors sanitize_script_errors,
const ScriptFetchOptions& fetch_options, const ScriptFetchOptions& fetch_options) {
ScriptController::ExecuteScriptPolicy policy) { ScriptEvaluationResult result = V8ScriptRunner::CompileAndRunScript(
ScriptEvaluationResult result = V8ScriptRunner::CompileAndRunScript( GetIsolate(), ScriptState::From(context), window_.Get(), source,
GetIsolate(), ScriptState::From(context), window_.Get(), source, base_url, base_url, sanitize_script_errors, fetch_options,
sanitize_script_errors, fetch_options, policy, V8ScriptRunner::RethrowErrorsOption::DoNotRethrow());
V8ScriptRunner::RethrowErrorsOption::DoNotRethrow());
if (result.GetResultType() == ScriptEvaluationResult::ResultType::kSuccess) if (result.GetResultType() == ScriptEvaluationResult::ResultType::kSuccess)
return result.GetSuccessValue(); return result.GetSuccessValue();
return v8::Local<v8::Value>(); return v8::Local<v8::Value>();
} }
TextPosition ScriptController::EventHandlerPosition() const { TextPosition ScriptController::EventHandlerPosition() const {
...@@ -275,6 +274,10 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld( ...@@ -275,6 +274,10 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld(
SanitizeScriptErrors sanitize_script_errors, SanitizeScriptErrors sanitize_script_errors,
const ScriptFetchOptions& fetch_options, const ScriptFetchOptions& fetch_options,
ExecuteScriptPolicy policy) { ExecuteScriptPolicy policy) {
if (!CanExecuteScript(policy)) {
return v8::Local<v8::Value>();
}
// |script_state->GetContext()| should be initialized already due to the // |script_state->GetContext()| should be initialized already due to the
// WindowProxy() call inside ToScriptStateForMainWorld(). // WindowProxy() call inside ToScriptStateForMainWorld().
ScriptState* script_state = ToScriptStateForMainWorld(window_->GetFrame()); ScriptState* script_state = ToScriptStateForMainWorld(window_->GetFrame());
...@@ -285,7 +288,7 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld( ...@@ -285,7 +288,7 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld(
return ExecuteScriptAndReturnValue(script_state->GetContext(), source_code, return ExecuteScriptAndReturnValue(script_state->GetContext(), source_code,
base_url, sanitize_script_errors, base_url, sanitize_script_errors,
fetch_options, policy); fetch_options);
} }
v8::Local<v8::Value> ScriptController::EvaluateMethodInMainWorld( v8::Local<v8::Value> ScriptController::EvaluateMethodInMainWorld(
...@@ -352,10 +355,8 @@ v8::Local<v8::Value> ScriptController::ExecuteScriptInIsolatedWorld( ...@@ -352,10 +355,8 @@ v8::Local<v8::Value> ScriptController::ExecuteScriptInIsolatedWorld(
// WindowProxy() inside ToScriptState() above. Add a helper which makes that // WindowProxy() inside ToScriptState() above. Add a helper which makes that
// obvious? // obvious?
return ExecuteScriptAndReturnValue( return ExecuteScriptAndReturnValue(script_state->GetContext(), source,
script_state->GetContext(), source, base_url, sanitize_script_errors, base_url, sanitize_script_errors);
ScriptFetchOptions(),
ScriptController::kExecuteScriptWhenScriptsDisabled);
} }
scoped_refptr<DOMWrapperWorld> scoped_refptr<DOMWrapperWorld>
......
...@@ -61,8 +61,6 @@ class SecurityOrigin; ...@@ -61,8 +61,6 @@ class SecurityOrigin;
class CORE_EXPORT ScriptController final class CORE_EXPORT ScriptController final
: public GarbageCollected<ScriptController> { : public GarbageCollected<ScriptController> {
public: public:
// TODO(crbug.com/1111134): Move this enum to V8ScriptRunner and remove
// #include "script_controller.h" from v8_script_runner.h.
enum ExecuteScriptPolicy { enum ExecuteScriptPolicy {
kExecuteScriptWhenScriptsDisabled, kExecuteScriptWhenScriptsDisabled,
kDoNotExecuteScriptWhenScriptsDisabled kDoNotExecuteScriptWhenScriptsDisabled
...@@ -84,8 +82,7 @@ class CORE_EXPORT ScriptController final ...@@ -84,8 +82,7 @@ class CORE_EXPORT ScriptController final
const ScriptSourceCode&, const ScriptSourceCode&,
const KURL& base_url, const KURL& base_url,
SanitizeScriptErrors, SanitizeScriptErrors,
const ScriptFetchOptions&, const ScriptFetchOptions& = ScriptFetchOptions());
ScriptController::ExecuteScriptPolicy);
v8::Local<v8::Value> EvaluateMethodInMainWorld( v8::Local<v8::Value> EvaluateMethodInMainWorld(
v8::Local<v8::Function> function, v8::Local<v8::Function> function,
......
...@@ -375,24 +375,13 @@ ScriptEvaluationResult V8ScriptRunner::CompileAndRunScript( ...@@ -375,24 +375,13 @@ ScriptEvaluationResult V8ScriptRunner::CompileAndRunScript(
const KURL& base_url, const KURL& base_url,
SanitizeScriptErrors sanitize_script_errors, SanitizeScriptErrors sanitize_script_errors,
const ScriptFetchOptions& fetch_options, const ScriptFetchOptions& fetch_options,
ScriptController::ExecuteScriptPolicy policy,
RethrowErrorsOption rethrow_errors) { RethrowErrorsOption rethrow_errors) {
DCHECK_EQ(isolate, script_state->GetIsolate()); DCHECK_EQ(isolate, script_state->GetIsolate());
if (policy == ScriptController::kDoNotExecuteScriptWhenScriptsDisabled && v8::Context::Scope scope(script_state->GetContext());
!execution_context->CanExecuteScripts(kAboutToExecuteScript)) {
return ScriptEvaluationResult::FromClassicNotRun();
}
LocalDOMWindow* window = DynamicTo<LocalDOMWindow>(execution_context); LocalDOMWindow* window = DynamicTo<LocalDOMWindow>(execution_context);
LocalFrame* frame = window ? window->GetFrame() : nullptr; LocalFrame* frame = window ? window->GetFrame() : nullptr;
if (window && window->document()->IsInitialEmptyDocument()) {
window->GetFrame()->Loader().DidAccessInitialDocument();
}
v8::Context::Scope scope(script_state->GetContext());
TRACE_EVENT1("devtools.timeline", "EvaluateScript", "data", TRACE_EVENT1("devtools.timeline", "EvaluateScript", "data",
inspector_evaluate_script_event::Data( inspector_evaluate_script_event::Data(
frame, source.Url().GetString(), source.StartPosition())); frame, source.Url().GetString(), source.StartPosition()));
......
...@@ -27,7 +27,6 @@ ...@@ -27,7 +27,6 @@
#define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_SCRIPT_RUNNER_H_ #define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_SCRIPT_RUNNER_H_
#include "third_party/blink/renderer/bindings/core/v8/sanitize_script_errors.h" #include "third_party/blink/renderer/bindings/core/v8/sanitize_script_errors.h"
#include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
#include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
#include "third_party/blink/renderer/platform/wtf/forward.h" #include "third_party/blink/renderer/platform/wtf/forward.h"
...@@ -127,7 +126,6 @@ class CORE_EXPORT V8ScriptRunner final { ...@@ -127,7 +126,6 @@ class CORE_EXPORT V8ScriptRunner final {
const KURL&, const KURL&,
SanitizeScriptErrors, SanitizeScriptErrors,
const ScriptFetchOptions&, const ScriptFetchOptions&,
ScriptController::ExecuteScriptPolicy,
RethrowErrorsOption); RethrowErrorsOption);
static v8::MaybeLocal<v8::Value> CompileAndRunInternalScript( static v8::MaybeLocal<v8::Value> CompileAndRunInternalScript(
v8::Isolate*, v8::Isolate*,
......
...@@ -317,6 +317,9 @@ ScriptEvaluationResult WorkerOrWorkletScriptController::EvaluateAndReturnValue( ...@@ -317,6 +317,9 @@ ScriptEvaluationResult WorkerOrWorkletScriptController::EvaluateAndReturnValue(
const ScriptSourceCode& source_code, const ScriptSourceCode& source_code,
SanitizeScriptErrors sanitize_script_errors, SanitizeScriptErrors sanitize_script_errors,
V8ScriptRunner::RethrowErrorsOption rethrow_errors) { V8ScriptRunner::RethrowErrorsOption rethrow_errors) {
if (IsExecutionForbidden())
return ScriptEvaluationResult::FromClassicNotRun();
DCHECK(IsContextInitialized()); DCHECK(IsContextInitialized());
DCHECK(is_ready_to_evaluate_); DCHECK(is_ready_to_evaluate_);
...@@ -329,9 +332,7 @@ ScriptEvaluationResult WorkerOrWorkletScriptController::EvaluateAndReturnValue( ...@@ -329,9 +332,7 @@ ScriptEvaluationResult WorkerOrWorkletScriptController::EvaluateAndReturnValue(
// TODO(crbug/1114989): Plumb ScriptFetchOptions from ClassicScript. // TODO(crbug/1114989): Plumb ScriptFetchOptions from ClassicScript.
ScriptEvaluationResult result = V8ScriptRunner::CompileAndRunScript( ScriptEvaluationResult result = V8ScriptRunner::CompileAndRunScript(
isolate_, script_state_, global_scope_, source_code, base_url, isolate_, script_state_, global_scope_, source_code, base_url,
sanitize_script_errors, ScriptFetchOptions(), sanitize_script_errors, ScriptFetchOptions(), std::move(rethrow_errors));
ScriptController::kDoNotExecuteScriptWhenScriptsDisabled,
std::move(rethrow_errors));
if (result.GetResultType() == ScriptEvaluationResult::ResultType::kAborted) if (result.GetResultType() == ScriptEvaluationResult::ResultType::kAborted)
ForbidExecution(); ForbidExecution();
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "third_party/blink/renderer/core/editing/editor.h" #include "third_party/blink/renderer/core/editing/editor.h"
#include "third_party/blink/renderer/core/editing/testing/editing_test_base.h" #include "third_party/blink/renderer/core/editing/testing/editing_test_base.h"
#include "third_party/blink/renderer/core/editing/visible_position.h" #include "third_party/blink/renderer/core/editing/visible_position.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h" #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
namespace blink { namespace blink {
......
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