From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 20 Sep 2018 17:45:03 -0700 Subject: blink_local_frame.patch According to electron/electron#3699, it is unreliable to use |unload| event for process.exit('exit'), so we have to do that in willReleaseScriptContext. Refs changes in: * https://codereview.chromium.org/1657583002 * https://chromium-review.googlesource.com/c/chromium/src/+/2596880 * https://chromium-review.googlesource.com/c/chromium/src/+/2597169 This patch reverts the changes to fix associated crashes in Electron. diff --git a/third_party/blink/renderer/core/frame/frame.cc b/third_party/blink/renderer/core/frame/frame.cc index 8bf6b4bc077cc41da5e0e6b13302bc343537c68f..01bddc0bcb7476408023c4cfc042a0883c16430c 100644 --- a/third_party/blink/renderer/core/frame/frame.cc +++ b/third_party/blink/renderer/core/frame/frame.cc @@ -120,14 +120,6 @@ bool Frame::Detach(FrameDetachType type) { DCHECK(!IsDetached()); - // TODO(dcheng): FocusController::FrameDetached() *should* fire JS events, - // hence the above check for `client_` being null. However, when this was - // previously placed before the `FrameDetached()` call, nothing crashes, which - // is suspicious. Investigate if we really don't need to fire JS events--and - // if we don't, move `forbid_scripts` up to be instantiated sooner and - // simplify this code. - ScriptForbiddenScope forbid_scripts; - if (type == FrameDetachType::kRemove) { if (provisional_frame_) { provisional_frame_->Detach(FrameDetachType::kRemove); @@ -150,6 +142,14 @@ bool Frame::Detach(FrameDetachType type) { GetWindowProxyManager()->ClearForSwap(); } + // TODO(dcheng): FocusController::FrameDetached() *should* fire JS events, + // hence the above check for `client_` being null. However, when this was + // previously placed before the `FrameDetached()` call, nothing crashes, which + // is suspicious. Investigate if we really don't need to fire JS events--and + // if we don't, move `forbid_scripts` up to be instantiated sooner and + // simplify this code. + ScriptForbiddenScope forbid_scripts; + // After this, we must no longer talk to the client since this clears // its owning reference back to our owning LocalFrame. client_->Detached(type); diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc index cf505929c9f6eac724a22e5abcd362ed70aed710..b49500d62fb29f179ae802366108c84e2f891836 100644 --- a/third_party/blink/renderer/core/frame/local_frame.cc +++ b/third_party/blink/renderer/core/frame/local_frame.cc @@ -553,10 +553,6 @@ bool LocalFrame::DetachImpl(FrameDetachType type) { } DCHECK(!view_ || !view_->IsAttached()); - // This is the earliest that scripting can be disabled: - // - FrameLoader::Detach() can fire XHR abort events - // - Document::Shutdown() can dispose plugins which can run script. - ScriptForbiddenScope forbid_script; if (!Client()) return false; @@ -603,6 +599,11 @@ bool LocalFrame::DetachImpl(FrameDetachType type) { DCHECK(!view_->IsAttached()); Client()->WillBeDetached(); + // This is the earliest that scripting can be disabled: + // - FrameLoader::Detach() can fire XHR abort events + // - Document::Shutdown() can dispose plugins which can run script. + ScriptForbiddenScope forbid_script; + // TODO(crbug.com/729196): Trace why LocalFrameView::DetachFromLayout crashes. CHECK(!view_->IsAttached()); SetView(nullptr);