Commit eb371599 authored by arthursonzogni's avatar arthursonzogni Committed by Chromium LUCI CQ

Stop observing after destroying RenderWidgetHost.

This is about release blocker: https://crbug.com/1153966

We think the bug is about iterating over an observer that has already
been deleted. After:
https://chromium-review.googlesource.com/c/chromium/src/+/2594772
all the observers are using ScopedObserver / base::ScopedObservation.
This ensures they aren't deleted before unregistering themself.

There are 5 RenderWidgetHostObserver:
1) FileSelectHelper
2) SessionRestoreStatsCollector
3) RenderWidgetHostVisibilityTracker
4) HungPagesTableModel
5) PageHandler.

Bug happens when deleting an unresponsive tab. Maybe HungPagesTableModel
is the culprit here?

This patch makes every observer to stop observing as soon as the
observee sends RenderWidgetHostDestroyed(). This also clears any
references toward the RenderWidgetHost.

I don't really have high hopes this will fix the bug. However this
doesn't hurt to be more cautious.

Bug: 1153966
Change-Id: I210e414e281665f56b148ac4248770448c55a0be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595443
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838273}
parent d5726c8c
...@@ -176,11 +176,11 @@ void SessionRestoreStatsCollector::RenderWidgetHostVisibilityChanged( ...@@ -176,11 +176,11 @@ void SessionRestoreStatsCollector::RenderWidgetHostVisibilityChanged(
void SessionRestoreStatsCollector::RenderWidgetHostDestroyed( void SessionRestoreStatsCollector::RenderWidgetHostDestroyed(
content::RenderWidgetHost* widget_host) { content::RenderWidgetHost* widget_host) {
observer_.Remove(widget_host);
auto* tab_state = GetTabState(widget_host); auto* tab_state = GetTabState(widget_host);
if (tab_state) { if (tab_state)
observer_.Remove(tab_state->observed_host);
tab_state->observed_host = nullptr; tab_state->observed_host = nullptr;
}
} }
void SessionRestoreStatsCollector::RemoveTab(NavigationController* tab) { void SessionRestoreStatsCollector::RemoveTab(NavigationController* tab) {
......
...@@ -158,6 +158,10 @@ void HungPagesTableModel::RenderProcessExited( ...@@ -158,6 +158,10 @@ void HungPagesTableModel::RenderProcessExited(
void HungPagesTableModel::RenderWidgetHostDestroyed( void HungPagesTableModel::RenderWidgetHostDestroyed(
content::RenderWidgetHost* widget_host) { content::RenderWidgetHost* widget_host) {
DCHECK(widget_observation_.IsObservingSource(render_widget_host_));
widget_observation_.Reset();
render_widget_host_ = nullptr;
// Notify the delegate. // Notify the delegate.
delegate_->TabDestroyed(); delegate_->TabDestroyed();
// WARNING: we've likely been deleted. // WARNING: we've likely been deleted.
......
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