Fix various Widget observers

Widget observers need to make sure they unregister themselves before
they are destroyed.

Bug: 1055483
Change-Id: I53c78555a9cd9d28fe7edab0630bca17f90e47a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2137319Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757412}

chrome/browser/ui/views/feature_promos/global_media_controls_promo_controller.cc

@@ -31,6 +31,9 @@ GlobalMediaControlsPromoController::GlobalMediaControlsPromoController(
   DCHECK(profile_);
 }
 
+GlobalMediaControlsPromoController::~GlobalMediaControlsPromoController() =
+    default;
+
 void GlobalMediaControlsPromoController::ShowPromo() {
   // This shouldn't be called more than once. Check that state is fresh.
   DCHECK(!show_promo_called_);
@@ -59,7 +62,7 @@ void GlobalMediaControlsPromoController::ShowPromo() {
       string_specifier, base::nullopt, base::nullopt, base::nullopt,
       std::move(feature_promo_bubble_timeout));
   promo_bubble_->set_close_on_deactivate(false);
-  promo_bubble_->GetWidget()->AddObserver(this);
+  observer_.Add(promo_bubble_->GetWidget());
 }
 
 void GlobalMediaControlsPromoController::OnMediaDialogOpened() {
@@ -79,6 +82,8 @@ void GlobalMediaControlsPromoController::OnWidgetDestroying(
   DCHECK(promo_bubble_);
   promo_bubble_ = nullptr;
 
+  observer_.Remove(widget);
+
   FinishPromo();
 }
 

chrome/browser/ui/views/feature_promos/global_media_controls_promo_controller.h

@@ -5,8 +5,10 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_FEATURE_PROMOS_GLOBAL_MEDIA_CONTROLS_PROMO_CONTROLLER_H_
 #define CHROME_BROWSER_UI_VIEWS_FEATURE_PROMOS_GLOBAL_MEDIA_CONTROLS_PROMO_CONTROLLER_H_
 
+#include "base/scoped_observer.h"
 #include "chrome/browser/ui/global_media_controls/media_toolbar_button_observer.h"
 #include "chrome/browser/ui/views/feature_promos/feature_promo_bubble_view.h"
+#include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 
 class MediaToolbarButtonView;
@@ -21,7 +23,7 @@ class GlobalMediaControlsPromoController : public views::WidgetObserver,
  public:
   GlobalMediaControlsPromoController(MediaToolbarButtonView* owner,
                                      Profile* profile);
-  ~GlobalMediaControlsPromoController() override = default;
+  ~GlobalMediaControlsPromoController() override;
 
   // Shows the IPH promo. Should only be called once.
   void ShowPromo();
@@ -51,6 +53,8 @@ class GlobalMediaControlsPromoController : public views::WidgetObserver,
   Profile* const profile_;
   FeaturePromoBubbleView* promo_bubble_ = nullptr;
 
+  ScopedObserver<views::Widget, views::WidgetObserver> observer_{this};
+
   // Whether we are showing the promo.
   bool is_showing_ = false;
 

chrome/browser/ui/views/feature_promos/reopen_tab_promo_controller.cc

@@ -37,6 +37,8 @@ ReopenTabPromoController::ReopenTabPromoController(BrowserView* browser_view)
       browser_view_(browser_view) {
 }
 
+ReopenTabPromoController::~ReopenTabPromoController() = default;
+
 void ReopenTabPromoController::ShowPromo() {
   // This shouldn't be called more than once. Check that state is fresh.
   DCHECK(!show_promo_called_);
@@ -74,7 +76,7 @@ void ReopenTabPromoController::ShowPromo() {
                 IDS_REOPEN_TAB_PROMO, base::nullopt,
                 IDS_REOPEN_TAB_PROMO_SCREENREADER, accelerator);
   promo_bubble_->set_close_on_deactivate(false);
-  promo_bubble_->GetWidget()->AddObserver(this);
+  observer_.Add(promo_bubble_->GetWidget());
 }
 
 void ReopenTabPromoController::OnTabReopened(int command_id) {
@@ -104,6 +106,8 @@ void ReopenTabPromoController::OnWidgetDestroying(views::Widget* widget) {
   // timed out without the user following our IPH. End it.
   if (promo_step_ == StepAtDismissal::kBubbleShown)
     PromoEnded();
+
+  observer_.Remove(widget);
 }
 
 void ReopenTabPromoController::AppMenuShown() {

chrome/browser/ui/views/feature_promos/reopen_tab_promo_controller.h

@@ -5,7 +5,9 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_FEATURE_PROMOS_REOPEN_TAB_PROMO_CONTROLLER_H_
 #define CHROME_BROWSER_UI_VIEWS_FEATURE_PROMOS_REOPEN_TAB_PROMO_CONTROLLER_H_
 
+#include "base/scoped_observer.h"
 #include "chrome/browser/ui/views/frame/app_menu_button_observer.h"
+#include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 
 class BrowserView;
@@ -19,7 +21,7 @@ class ReopenTabPromoController : public AppMenuButtonObserver,
                                  public views::WidgetObserver {
  public:
   explicit ReopenTabPromoController(BrowserView* browser_view);
-  ~ReopenTabPromoController() override = default;
+  ~ReopenTabPromoController() override;
 
   // Shows the IPH promo. Should only be called once.
   void ShowPromo();
@@ -67,6 +69,8 @@ class ReopenTabPromoController : public AppMenuButtonObserver,
   BrowserView* const browser_view_;
   FeaturePromoBubbleView* promo_bubble_ = nullptr;
 
+  ScopedObserver<views::Widget, views::WidgetObserver> observer_{this};
+
   // The promo stage that has been reached, logged to a histogram when the promo
   // flow ends.
   StepAtDismissal promo_step_ = StepAtDismissal::kBubbleShown;

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

@@ -151,9 +151,10 @@ OmniboxPopupContentsView::OmniboxPopupContentsView(
 }
 
 OmniboxPopupContentsView::~OmniboxPopupContentsView() {
-  // We don't need to do anything with |popup_| here.  The OS either has already
-  // closed the window, in which case it's been deleted, or it will soon, in
-  // which case there's nothing we need to do.
+  // We don't need to close or delete |popup_| here. The OS either has already
+  // closed the window, in which case it's been deleted, or it will soon.
+  if (popup_)
+    popup_->RemoveObserver(this);
 }
 
 void OmniboxPopupContentsView::OpenMatch(

components/exo/shell_surface_base.cc

@@ -694,6 +694,8 @@ bool ShellSurfaceBase::OnCloseRequested(
 
 void ShellSurfaceBase::WindowClosing() {
   SetEnabled(false);
+  if (widget_)
+    widget_->RemoveObserver(this);
   widget_ = nullptr;
 }
 

ui/message_center/views/message_popup_view.cc

@@ -113,7 +113,7 @@ void MessagePopupView::Show() {
   views::Widget* widget = new views::Widget();
   popup_collection_->ConfigureWidgetInitParamsForContainer(widget, &params);
   widget->set_focus_on_creation(false);
-  widget->AddObserver(this);
+  observer_.Add(widget);
 
 #if defined(OS_WIN)
   // We want to ensure that this toast always goes to the native desktop,
@@ -204,6 +204,10 @@ void MessagePopupView::OnWidgetActivationChanged(views::Widget* widget,
   popup_collection_->Update();
 }
 
+void MessagePopupView::OnWidgetDestroyed(views::Widget* widget) {
+  observer_.Remove(widget);
+}
+
 bool MessagePopupView::IsWidgetValid() const {
   return GetWidget() && !GetWidget()->IsClosed();
 }

ui/message_center/views/message_popup_view.h

@@ -5,7 +5,9 @@
 #ifndef UI_MESSAGE_CENTER_VIEWS_MESSAGE_POPUP_VIEW_H_
 #define UI_MESSAGE_CENTER_VIEWS_MESSAGE_POPUP_VIEW_H_
 
+#include "base/scoped_observer.h"
 #include "ui/message_center/message_center_export.h"
+#include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/widget/widget_observer.h"
 
@@ -59,6 +61,7 @@ class MESSAGE_CENTER_EXPORT MessagePopupView : public views::WidgetDelegateView,
 
   // views::WidgetObserver:
   void OnWidgetActivationChanged(views::Widget* widget, bool active) override;
+  void OnWidgetDestroyed(views::Widget* widget) override;
 
   bool is_hovered() const { return is_hovered_; }
   bool is_active() const { return is_active_; }
@@ -83,6 +86,8 @@ class MESSAGE_CENTER_EXPORT MessagePopupView : public views::WidgetDelegateView,
   bool is_hovered_ = false;
   bool is_active_ = false;
 
+  ScopedObserver<views::Widget, views::WidgetObserver> observer_{this};
+
   DISALLOW_COPY_AND_ASSIGN(MessagePopupView);
 };
 

ui/views/corewm/tooltip_aura.cc

@@ -296,6 +296,8 @@ bool TooltipAura::IsVisible() {
 
 void TooltipAura::OnWidgetDestroying(views::Widget* widget) {
   DCHECK_EQ(widget_, widget);
+  if (widget_)
+    widget_->RemoveObserver(this);
   widget_ = nullptr;
   tooltip_window_ = nullptr;
 }