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 da12f2f47f97628f1adeabc8900ffd16132afd7e..61d373f78520a063c7f86bde6869af9d26f32d11 100644 --- a/third_party/blink/renderer/core/frame/frame.cc +++ b/third_party/blink/renderer/core/frame/frame.cc @@ -122,14 +122,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); @@ -153,6 +145,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 cf424b45c60ee6cd8fcb5bced73332ffd4b60c2d..17d59201aa0e4f077536ec29a3c59bd4e1d831a7 100644 --- a/third_party/blink/renderer/core/frame/local_frame.cc +++ b/third_party/blink/renderer/core/frame/local_frame.cc @@ -542,10 +542,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; @@ -591,6 +587,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);