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 5d875911b47c31b141c03a89a6aec8b9dee8cea6..25971105a530ae14d3c89719bb0ad17a43409020 100644 --- a/third_party/blink/renderer/core/frame/frame.cc +++ b/third_party/blink/renderer/core/frame/frame.cc @@ -135,14 +135,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); @@ -167,6 +159,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 6a8b2cab1203d89d9c5bca38f71c8c327815de78..63c906e460c8d38ff1dbafbfefcf8b082d2f77f9 100644 --- a/third_party/blink/renderer/core/frame/local_frame.cc +++ b/third_party/blink/renderer/core/frame/local_frame.cc @@ -752,10 +752,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; @@ -809,6 +805,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);