Add CHECKs to debug why WebLocalFrameImpl has a detached core frame.

There are Android crashes where sending an orientation change event
seems to encounter mutually exclusive facts:
- WebLocalFrameImpl::GetFrame() should only return a non-null pointer if
  the LocalFrame is attached.
- Furthermore, GetFrame() and GetFrame()->DomWindow() both return
  non-null pointers (though GetFrame()->DomWindow() returning non- null
  does *not* imply the frame is attached).
- ScreenOrientationController is looked up as a a LocalDOMWindow
  supplement, using a non-null DomWindow().
- ScreenOrientationController is an ExecutionContextLifecycleObserver,
  and uses the DomWindow() helper to look up frames to iterate through.
- DomWindow() should only return null once the associated LocalDOMWindow
  is detached
- Yet DomWindow() returns null, contradicting WebLocalFrameImpl...

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

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

@@ -91,6 +91,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/debug/crash_logging.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
@@ -1915,7 +1916,7 @@ WebLocalFrameImpl::WebLocalFrameImpl(
       input_method_controller_(*this),
       spell_check_panel_host_client_(nullptr),
       self_keep_alive_(PERSISTENT_FROM_HERE, this) {
-  DCHECK(client_);
+  CHECK(client_);
   g_frame_count++;
   client_->BindToFrame(this);
 }
@@ -2389,6 +2390,17 @@ void WebLocalFrameImpl::SendOrientationChangeEvent() {
   if (!GetFrame() || !GetFrame()->DomWindow())
     return;
 
+  if (GetFrame()) {
+    // If GetFrame() returns non-null, the frame should be attached. Make sure
+    // the other state on WebLocalFrameImpl agrees.
+    CHECK(GetFrame()->IsAttached());
+    CHECK(GetFrame()->Client());
+    CHECK(Client());
+  }
+
+  SCOPED_CRASH_KEY_NUMBER("debug-1154141", web_dom_window,
+                          reinterpret_cast<uintptr_t>(GetFrame()->DomWindow()));
+
   // Screen Orientation API
   CoreInitializer::GetInstance().NotifyOrientationChanged(*GetFrame());
 

third_party/blink/renderer/modules/screen_orientation/screen_orientation_controller.cc

@@ -6,6 +6,8 @@
 
 #include <memory>
 #include <utility>
+#include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_metric_builder.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_study_settings.h"
@@ -148,8 +150,19 @@ void ScreenOrientationController::PageVisibilityChanged() {
 
 void ScreenOrientationController::NotifyOrientationChanged() {
   // TODO(dcheng): Remove this and check in the caller.
-  if (!DomWindow())
+  if (!DomWindow()) {
+    // In theory, it should not be possible to reach this when called via
+    // WebLocalFrameImpl, as WebLocalFrameImpl's checks should be sufficient
+    // to avoid notifying orientation change events on a detached execution
+    // context. Nonetheless, ExecutionContextLifecycleObserver::DomWindow() is
+    // returning null here, which seems to indicate the execution context is
+    // detached... make sure this isn't somehow associated with a null
+    // LocalDOMWindow, even though that should be impossible...
+    SCOPED_CRASH_KEY_NUMBER("debug-1154141", supplement,
+                            reinterpret_cast<uintptr_t>(GetSupplementable()));
+    base::debug::DumpWithoutCrashing();
     return;
+  }
 
   // Keep track of the frames that need to be notified before notifying the
   // current frame as it will prevent side effects from the change event