Centralize hack to avoid double-detaching an already detached main frame

Bug: 1154141
Change-Id: I8996510a69ca1fe1b6d4f1affca9a431961ac1cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577762Reviewed-by: Fergal Daly <fergal@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834549}

third_party/blink/renderer/core/frame/local_frame.cc

@@ -2111,15 +2111,6 @@ bool LocalFrame::IsProvisional() const {
   // this state can no longer be accurately calculated.
   CHECK(!IsDetached());
 
-  // TODO(https://crbug.com/838348): Sadly, there are situations where Blink may
-  // attempt to detach a main frame twice due to a bug. That rewinds
-  // FrameLifecycle from kDetached to kDetaching, but GetPage() will already be
-  // null. Early returning false in that case is "safe enough", as the frame has
-  // already been detached, so any detach work gated on IsProvisional() has
-  // already been done.
-  if (!GetPage())
-    return false;
-
   if (IsMainFrame()) {
     return GetPage()->MainFrame() != this;
   }

third_party/blink/renderer/core/page/page.cc

@@ -955,7 +955,15 @@ void Page::WillCloseAnimationHost(LocalFrameView* view) {
 void Page::WillBeDestroyed() {
   Frame* main_frame = main_frame_;
 
-  main_frame->Detach(FrameDetachType::kRemove);
+  // TODO(https://crbug.com/838348): Sadly, there are situations where Blink may
+  // attempt to detach a main frame twice due to a bug. That rewinds
+  // FrameLifecycle from kDetached to kDetaching, but GetPage() will already be
+  // null. Since Detach() has already happened, just skip the actual Detach()
+  // call to try to limit the side effects of this bug on the rest of frame
+  // detach.
+  if (main_frame->GetPage()) {
+    main_frame->Detach(FrameDetachType::kRemove);
+  }
 
   DCHECK(AllPages().Contains(this));
   AllPages().erase(this);