CHECKs to debug why LocalFrame is in an inconsistent detached state.

A number of Android crashes have a non-detached LocalFrame but GetPage()
returns null. This CL adds a few CHECKs in IsProvisional(), which is one
place where this crash has been observed. It also adds a few CHECKs in
the FrameLifecycle state machine to make sure state isn't somehow being
rewound.

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

third_party/blink/renderer/core/frame/frame.h

@@ -398,6 +398,12 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
   bool ConsumeTransientUserActivationInFrameTree();
   void ClearUserActivationInFrameTree();
 
+  // Note: this is a temporary debug helper. Note that the despite the name,
+  // this is not the same thing as LocalFrame's FrameLifecycleState.
+  FrameLifecycle::State GetFrameLifecycle() const {
+    return lifecycle_.GetState();
+  }
+
   mutable FrameTree tree_node_;
 
   Member<Page> page_;

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

@@ -16,12 +16,12 @@ void FrameLifecycle::AdvanceTo(State state) {
     case kAttached:
     case kDetached:
       // Normally, only allow state to move forward.
-      DCHECK_GT(state, state_);
+      CHECK_GT(state, state_);
       break;
     case kDetaching:
       // We can go from Detaching to Detaching since the detach() method can be
       // re-entered.
-      DCHECK_GE(state, state_);
+      CHECK_GE(state, state_);
       break;
   }
   state_ = state;

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

@@ -34,6 +34,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/debug/crash_logging.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/unguessable_token.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
@@ -2102,9 +2103,17 @@ void LocalFrame::ForceSynchronousDocumentInstall(
 }
 
 bool LocalFrame::IsProvisional() const {
+  SCOPED_CRASH_KEY_NUMBER("inconsistent-frame-state", lifecycle,
+                          static_cast<int>(GetFrameLifecycle()));
   // Calling this after the frame is marked as completely detached is a bug, as
   // this state can no longer be accurately calculated.
   CHECK(!IsDetached());
+  // TODO(https://crbug.com/1154141): This are added for temporary debugging.
+  // If a LocalFrame is attached, it should have both a LocalFrameClient and a
+  // Page associated with it. Yet there are some crash reports that seem to
+  // indicate that GetPage() is somehow null... confirm this.
+  CHECK(Client());
+  CHECK(GetPage());
 
   if (IsMainFrame()) {
     return GetPage()->MainFrame() != this;