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 9eaacc65478f56de32ffa6d02c053bf5d6ea8649..bfbee4c165e545e45fbef47cc6d4556096c79e72 100644 --- a/third_party/blink/renderer/core/frame/local_frame.cc +++ b/third_party/blink/renderer/core/frame/local_frame.cc @@ -792,10 +792,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; @@ -840,6 +836,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);