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

Use ToScriptState in ScriptController

This CL merges the logic for getting `v8::Context` in
ScriptController into ToScriptState*() methods, in order to
Unify code paths around script evaluation and decouple them
from ScriptController.
This is preparation for
https://chromium-review.googlesource.com/c/chromium/src/+/2413430
that will move out script evaluation methods out of ScriptController.

Since `ScriptController::window_proxy_manager_` is equal to
its LocalDOMWindow's Frame's `window_proxy_manager_`,
this CL keeps the existing behavior, except that
ToScriptState*() methods have more checks that result in
null ScriptState and thus skipping script evaluation.

This CL removes MainWorldProxy() methods that are no longer
used after this CL.

Bug: 1111134
Change-Id: I49247650ec984d04f3dd44115d197044f021c1f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2543189Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829941}
parent fbb70c72
...@@ -278,15 +278,20 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld( ...@@ -278,15 +278,20 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld(
return v8::Local<v8::Value>(); return v8::Local<v8::Value>();
} }
// |context| should be initialized already due to the // |script_state->GetContext()| should be initialized already due to the
// MainWorldProxy() call. // WindowProxy() call inside ToScriptStateForMainWorld().
v8::Local<v8::Context> context = ScriptState* script_state = ToScriptStateForMainWorld(window_->GetFrame());
window_proxy_manager_->MainWorldProxy()->ContextIfInitialized(); if (!script_state) {
v8::Context::Scope scope(context); return v8::Local<v8::Value>();
}
DCHECK_EQ(script_state->GetIsolate(), GetIsolate());
v8::Context::Scope scope(script_state->GetContext());
v8::EscapableHandleScope handle_scope(GetIsolate()); v8::EscapableHandleScope handle_scope(GetIsolate());
v8::Local<v8::Value> object = ExecuteScriptAndReturnValue( v8::Local<v8::Value> object = ExecuteScriptAndReturnValue(
context, source_code, base_url, sanitize_script_errors, fetch_options); script_state->GetContext(), source_code, base_url, sanitize_script_errors,
fetch_options);
if (object.IsEmpty()) if (object.IsEmpty())
return v8::Local<v8::Value>(); return v8::Local<v8::Value>();
...@@ -304,18 +309,21 @@ v8::Local<v8::Value> ScriptController::EvaluateMethodInMainWorld( ...@@ -304,18 +309,21 @@ v8::Local<v8::Value> ScriptController::EvaluateMethodInMainWorld(
return v8::Local<v8::Value>(); return v8::Local<v8::Value>();
} }
// |context| should be initialized already due to the // |script_state->GetContext()| should be initialized already due to the
// MainWorldProxy() call. // WindowProxy() call inside ToScriptStateForMainWorld().
v8::Local<v8::Context> context = ScriptState* script_state = ToScriptStateForMainWorld(window_->GetFrame());
window_proxy_manager_->MainWorldProxy()->ContextIfInitialized(); if (!script_state) {
v8::Context::Scope scope(context); return v8::Local<v8::Value>();
}
DCHECK_EQ(script_state->GetIsolate(), GetIsolate());
v8::Context::Scope scope(script_state->GetContext());
v8::EscapableHandleScope handle_scope(GetIsolate()); v8::EscapableHandleScope handle_scope(GetIsolate());
v8::TryCatch try_catch(GetIsolate()); v8::TryCatch try_catch(GetIsolate());
try_catch.SetVerbose(true); try_catch.SetVerbose(true);
ExecutionContext* executionContext = ExecutionContext* executionContext = ExecutionContext::From(script_state);
ExecutionContext::From(ScriptState::From(context));
v8::MaybeLocal<v8::Value> resultObj = V8ScriptRunner::CallFunction( v8::MaybeLocal<v8::Value> resultObj = V8ScriptRunner::CallFunction(
function, executionContext, receiver, argc, function, executionContext, receiver, argc,
...@@ -345,17 +353,20 @@ v8::Local<v8::Value> ScriptController::ExecuteScriptInIsolatedWorld( ...@@ -345,17 +353,20 @@ v8::Local<v8::Value> ScriptController::ExecuteScriptInIsolatedWorld(
SanitizeScriptErrors sanitize_script_errors) { SanitizeScriptErrors sanitize_script_errors) {
DCHECK_GT(world_id, 0); DCHECK_GT(world_id, 0);
scoped_refptr<DOMWrapperWorld> world = ScriptState* script_state = ToScriptState(
DOMWrapperWorld::EnsureIsolatedWorld(GetIsolate(), world_id); window_, *DOMWrapperWorld::EnsureIsolatedWorld(GetIsolate(), world_id));
LocalWindowProxy* isolated_world_window_proxy = WindowProxy(*world); if (!script_state) {
return v8::Local<v8::Value>();
}
// TODO(dcheng): Context must always be initialized here, due to the call to // TODO(dcheng): Context must always be initialized here, due to the call to
// windowProxy() on the previous line. Add a helper which makes that obvious? // WindowProxy() inside ToScriptState() above. Add a helper which makes that
v8::Local<v8::Context> context = // obvious?
isolated_world_window_proxy->ContextIfInitialized();
v8::Context::Scope scope(context); v8::Context::Scope scope(script_state->GetContext());
v8::Local<v8::Value> evaluation_result = ExecuteScriptAndReturnValue( v8::Local<v8::Value> evaluation_result = ExecuteScriptAndReturnValue(
context, source, base_url, sanitize_script_errors); script_state->GetContext(), source, base_url, sanitize_script_errors);
if (!evaluation_result.IsEmpty()) if (!evaluation_result.IsEmpty())
return evaluation_result; return evaluation_result;
return v8::Local<v8::Value>::New(GetIsolate(), v8::Undefined(GetIsolate())); return v8::Local<v8::Value>::New(GetIsolate(), v8::Undefined(GetIsolate()));
......
...@@ -39,11 +39,6 @@ class WindowProxyManager : public GarbageCollected<WindowProxyManager> { ...@@ -39,11 +39,6 @@ class WindowProxyManager : public GarbageCollected<WindowProxyManager> {
void CORE_EXPORT ReleaseGlobalProxies(GlobalProxyVector&); void CORE_EXPORT ReleaseGlobalProxies(GlobalProxyVector&);
void CORE_EXPORT SetGlobalProxies(const GlobalProxyVector&); void CORE_EXPORT SetGlobalProxies(const GlobalProxyVector&);
WindowProxy* MainWorldProxy() {
window_proxy_->InitializeIfNeeded();
return window_proxy_;
}
WindowProxy* GetWindowProxy(DOMWrapperWorld& world) { WindowProxy* GetWindowProxy(DOMWrapperWorld& world) {
WindowProxy* window_proxy = WindowProxyMaybeUninitialized(world); WindowProxy* window_proxy = WindowProxyMaybeUninitialized(world);
window_proxy->InitializeIfNeeded(); window_proxy->InitializeIfNeeded();
...@@ -76,9 +71,6 @@ class WindowProxyManagerImplHelper : public WindowProxyManager { ...@@ -76,9 +71,6 @@ class WindowProxyManagerImplHelper : public WindowProxyManager {
using Base = WindowProxyManager; using Base = WindowProxyManager;
public: public:
ProxyType* MainWorldProxy() {
return static_cast<ProxyType*>(Base::MainWorldProxy());
}
ProxyType* WindowProxy(DOMWrapperWorld& world) { ProxyType* WindowProxy(DOMWrapperWorld& world) {
return static_cast<ProxyType*>(Base::GetWindowProxy(world)); return static_cast<ProxyType*>(Base::GetWindowProxy(world));
} }
......
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