Commit 0187ccfa authored by Dmitry Gozman's avatar Dmitry Gozman Committed by Commit Bot

Revert "Null-check LocalFrame::Client() before accessing in LocalWindowProxy"

This reverts commit 250e22d9.

Reason for revert: Not needed anymore, since another fix was landed.

Original change's description:
> Null-check LocalFrame::Client() before accessing in LocalWindowProxy
> 
> When the frame has already been detached, Client() will be nullptr.
> Since we can call LocalWindowProxy methods even when detached,
> e.g. through ToV8ContextEvenIfDetached(), we should null-check the client.
> 
> This is a band-aid fix, because we actually should not initialize context
> on a detached frame and change ToV8ContextEvenIfDetached to never force
> context. However, the proper solution has many risks and needs additional
> investigation.
> 
> Bug: 805882
> Change-Id: Idcd6bbc0e6eec9b2de53acfb646b30bd9636d797
> Reviewed-on: https://chromium-review.googlesource.com/949603
> Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#543266}

TBR=dgozman@chromium.org,dcheng@chromium.org,peria@chromium.org,yukishiino@chromium.org,haraken@chromium.org

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

Bug: 805882
Change-Id: Ia9bf73134c13c3f0bb2c29e69f9d88dbed336ce6
Reviewed-on: https://chromium-review.googlesource.com/973468Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544782}
parent 8d6df7ba
...@@ -77,9 +77,6 @@ void LocalWindowProxy::DisposeContext(Lifecycle next_status, ...@@ -77,9 +77,6 @@ void LocalWindowProxy::DisposeContext(Lifecycle next_status,
// The embedder could run arbitrary code in response to the // The embedder could run arbitrary code in response to the
// willReleaseScriptContext callback, so all disposing should happen after // willReleaseScriptContext callback, so all disposing should happen after
// it returns. // it returns.
// TODO(yukishiino): Apparently, we can create context for a detached frame
// (see comment in CreateContext), but then we do not dispose it. We should
// make sure that create/dispose operations are balanced.
GetFrame()->Client()->WillReleaseScriptContext(context, world_->GetWorldId()); GetFrame()->Client()->WillReleaseScriptContext(context, world_->GetWorldId());
MainThreadDebugger::Instance()->ContextWillBeDestroyed(script_state_.get()); MainThreadDebugger::Instance()->ContextWillBeDestroyed(script_state_.get());
...@@ -173,12 +170,7 @@ void LocalWindowProxy::Initialize() { ...@@ -173,12 +170,7 @@ void LocalWindowProxy::Initialize() {
GetFrame()->IsMainFrame()); GetFrame()->IsMainFrame());
MainThreadDebugger::Instance()->ContextCreated(script_state_.get(), MainThreadDebugger::Instance()->ContextCreated(script_state_.get(),
GetFrame(), origin); GetFrame(), origin);
// TODO(yukishiino): Remove this client check, we should not create context GetFrame()->Client()->DidCreateScriptContext(context, world_->GetWorldId());
// on a frame without client.
if (GetFrame()->Client()) {
GetFrame()->Client()->DidCreateScriptContext(context,
world_->GetWorldId());
}
} }
InstallConditionalFeatures(); InstallConditionalFeatures();
...@@ -197,9 +189,7 @@ void LocalWindowProxy::CreateContext() { ...@@ -197,9 +189,7 @@ void LocalWindowProxy::CreateContext() {
Vector<const char*> extension_names; Vector<const char*> extension_names;
// Dynamically tell v8 about our extensions now. // Dynamically tell v8 about our extensions now.
// TODO(yukishiino): Remove this client check, we should not create context if (GetFrame()->Client()->AllowScriptExtensions()) {
// on a frame without client.
if (GetFrame()->Client() && GetFrame()->Client()->AllowScriptExtensions()) {
const V8Extensions& extensions = ScriptController::RegisteredExtensions(); const V8Extensions& extensions = ScriptController::RegisteredExtensions();
extension_names.ReserveInitialCapacity(extensions.size()); extension_names.ReserveInitialCapacity(extensions.size());
for (const auto* extension : extensions) for (const auto* extension : extensions)
......
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