Commit b04a94f1 authored by Daniel Cheng's avatar Daniel Cheng Committed by Chromium LUCI CQ

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

This reverts commit 2b2acd1c.

Reason for revert: Root cause has been determined.

Original change's description:
> 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}

TBR=dcheng@chromium.org,fergal@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1154141
Fixed: 1154583, 1154586, 1154676, 1154935
Change-Id: I53bc1544082baf6c91f99c18eb022f15c523ba5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575997Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833968}
parent 20adcbb0
......@@ -398,12 +398,6 @@ 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_;
......
......@@ -16,12 +16,12 @@ void FrameLifecycle::AdvanceTo(State state) {
case kAttached:
case kDetached:
// Normally, only allow state to move forward.
CHECK_GT(state, state_);
DCHECK_GT(state, state_);
break;
case kDetaching:
// We can go from Detaching to Detaching since the detach() method can be
// re-entered.
CHECK_GE(state, state_);
DCHECK_GE(state, state_);
break;
}
state_ = state;
......
......@@ -34,7 +34,6 @@
#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"
......@@ -2103,17 +2102,9 @@ 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;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment