Commit 9211311c authored by arthursonzogni's avatar arthursonzogni Committed by Chromium LUCI CQ

Check RenderWidgetHostObserver stop observing before deletion.

Apply CheckedObserverAdapter::IsMarkedForRemoval() advise:
```
If |weak_ptr_| was invalidated then this attempt to iterate over the
pointer is a UAF. Tip: If it's unclear where the `delete` occurred, try
adding CHECK(!IsInObserverList()) to the ~CheckedObserver() (destructor)
override. However, note that this is not always a bug: a destroyed
observer can exist in an ObserverList so long as nothing iterates over
the ObserverList before the list itself is destroyed.
```

I have absolutely no clue what is causing bug 1153966. All the
observation are wrapped inside base::Scoped{Observation, Observer}, so
this shouldn't happen in theory. However there are proof it happens in
practice:
https://crash.corp.google.com/browse?q=ReportID%3D%27e5305cdcd95db0cb

Bug:1153966

Change-Id: I067ccdd4d0b34737ba54b31a6fb5b6b1a9d2e0c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615346
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841976}
parent a14625ef
......@@ -290,6 +290,8 @@ source_set("browser_sources") {
"render_view_host.h",
"render_widget_host.h",
"render_widget_host_iterator.h",
"render_widget_host_observer.cc",
"render_widget_host_observer.h",
"render_widget_host_view.h",
"render_widget_host_view_mac_delegate.h",
"renderer_preferences_util.cc",
......
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "content/public/browser/render_widget_host_observer.h"
#include "base/check.h"
namespace content {
RenderWidgetHostObserver::~RenderWidgetHostObserver() {
// TODO(https://crbug.com/1153966): Instrumentation. When fixed, decide if
// this CHECK should be removed or turned into a DCHECK.
CHECK(!IsInObserverList());
}
} // namespace content
......@@ -26,7 +26,7 @@ class CONTENT_EXPORT RenderWidgetHostObserver : public base::CheckedObserver {
virtual void RenderWidgetHostDestroyed(RenderWidgetHost* widget_host) {}
protected:
~RenderWidgetHostObserver() override = default;
~RenderWidgetHostObserver() override;
};
} // namespace content
......
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