Commit a6dcbb82 authored by Xiaoqian Dai's avatar Xiaoqian Dai Committed by Commit Bot

Add CHECKs to help investigate crbug.com/937381.

From the crash stack, |Widget::widget_delegate_| has been deleted but
not set to nullptr, thus caused the nullptr dereference crash. However,
we did not find the equivalent number of crashes in
WidgetDelegate::~WidgetDelegate().

This CL does:

1) Change DCHECK to CHECK in Widget::~Widget() when NATIVE_WIDGET_OWNS_WIDGET
to see if we can get another crash stack to help investigate the bug.
If crashes happen here, means Widget is destroyed while its native widget is
still alive, thus Widget::OnNativeWidgetDestroyed() is not called, thus
|widget_delegate_| is not set to nullptr.

2) Besides that, it may be also possible that a window is destroyed while
building up the mru window. We added a scoped window observer to observe
window in IsWindowConsideredActivatable() and see if there is any window is
destroyed out from the function.

Bug: 937381
Change-Id: I01172cc36fcb19078a5d416d7c87fb505f6304d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1615384
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661386}
parent b8ce5f07
...@@ -25,8 +25,32 @@ namespace ash { ...@@ -25,8 +25,32 @@ namespace ash {
namespace { namespace {
bool IsWindowConsideredActivatable(const aura::Window* window) { // A class that observes a window that should not be destroyed inside a certain
// scope. This class is added to investigate crbug.com/937381 to see if it's
// possible that a window is destroyed while building up the mru window list.
// TODO(crbug.com/937381): Remove this class once we figure out the reason.
class ScopedWindowClosingObserver : public aura::WindowObserver {
public:
explicit ScopedWindowClosingObserver(aura::Window* window) : window_(window) {
window_->AddObserver(this);
}
~ScopedWindowClosingObserver() override {
window_->RemoveObserver(this);
window_ = nullptr;
}
// aura::WindowObserver:
void OnWindowDestroyed(aura::Window* window) override { CHECK(false); }
private:
aura::Window* window_;
DISALLOW_COPY_AND_ASSIGN(ScopedWindowClosingObserver);
};
bool IsWindowConsideredActivatable(aura::Window* window) {
DCHECK(window); DCHECK(window);
ScopedWindowClosingObserver observer(window);
AshFocusRules* focus_rules = Shell::Get()->focus_rules(); AshFocusRules* focus_rules = Shell::Get()->focus_rules();
// Only toplevel windows can be activated. // Only toplevel windows can be activated.
......
...@@ -187,7 +187,8 @@ Widget::~Widget() { ...@@ -187,7 +187,8 @@ Widget::~Widget() {
if (ownership_ == InitParams::WIDGET_OWNS_NATIVE_WIDGET) { if (ownership_ == InitParams::WIDGET_OWNS_NATIVE_WIDGET) {
delete native_widget_; delete native_widget_;
} else { } else {
DCHECK(native_widget_destroyed_) // TODO(crbug.com/937381): Revert to DCHECK once we figure out the reason.
CHECK(native_widget_destroyed_)
<< "Destroying a widget with a live native widget. " << "Destroying a widget with a live native widget. "
<< "Widget probably should use WIDGET_OWNS_NATIVE_WIDGET ownership."; << "Widget probably should use WIDGET_OWNS_NATIVE_WIDGET ownership.";
} }
......
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