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 b7bff829d779036ce0341b52ce9adc28eac91fa2..79b48d028ff6742d0d43ac6d32242f505587dafd 100644 --- a/third_party/blink/renderer/core/frame/frame.cc +++ b/third_party/blink/renderer/core/frame/frame.cc @@ -125,14 +125,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); @@ -156,6 +148,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 46a21ac9dfe9308bbb1b2fb44249cb1ce6e8dcf6..8b2c78985cf718e06244c73b0961a69d3a0f8ac6 100644 --- a/third_party/blink/renderer/core/frame/local_frame.cc +++ b/third_party/blink/renderer/core/frame/local_frame.cc @@ -623,10 +623,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; @@ -674,6 +670,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);