Replace AppMenuDestroyed() notification with weak pointer usage.

This is simpler and less error-prone, and will allow moving the app menu
observer access point to the app menu button (in a future CL).

Bug: none
Change-Id: I17eaf8d3374ae3d8bf1b078631f93c8a6c2e88f7
Reviewed-on: https://chromium-review.googlesource.com/c/1447323
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628541}

chrome/browser/ui/views/toolbar/app_menu.cc

@@ -295,20 +295,12 @@ class InMenuButton : public LabelButton {
 };
 
 // AppMenuView is a view that can contain label buttons.
-class AppMenuView : public views::View,
-                    public views::ButtonListener,
-                    public AppMenuObserver {
+class AppMenuView : public views::View, public views::ButtonListener {
  public:
   AppMenuView(AppMenu* menu, ButtonMenuItemModel* menu_model)
-      : menu_(menu),
-        menu_model_(menu_model) {
-    menu_->AddObserver(this);
-  }
+      : menu_(menu->AsWeakPtr()), menu_model_(menu_model) {}
 
-  ~AppMenuView() override {
-    if (menu_)
-      menu_->RemoveObserver(this);
-  }
+  ~AppMenuView() override = default;
 
   // Overridden from views::View.
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
@@ -356,25 +348,21 @@ class AppMenuView : public views::View,
     return button;
   }
 
-  // Overridden from AppMenuObserver:
-  void AppMenuDestroyed() override {
-    menu_->RemoveObserver(this);
-    menu_ = nullptr;
-    menu_model_ = nullptr;
-  }
-
  protected:
-  AppMenu* menu() { return menu_; }
-  const AppMenu* menu() const { return menu_; }
-  ButtonMenuItemModel* menu_model() { return menu_model_; }
+  base::WeakPtr<AppMenu> menu() { return menu_; }
+  base::WeakPtr<const AppMenu> menu() const { return menu_; }
+  ButtonMenuItemModel* menu_model() {
+    // The menu and the items in the menu model have similar lifetimes; it's
+    // only safe to access model items if the menu is still alive.
+    DCHECK(menu_);
+    return menu_model_;
+  }
 
  private:
   // Hosting AppMenu.
-  // WARNING: this may be nullptr during shutdown.
-  AppMenu* menu_;
+  base::WeakPtr<AppMenu> menu_;
 
   // The menu model containing the increment/decrement/reset items.
-  // WARNING: this may be nullptr during shutdown.
   ButtonMenuItemModel* menu_model_;
 
   DISALLOW_COPY_AND_ASSIGN(AppMenuView);
@@ -800,8 +788,6 @@ AppMenu::~AppMenu() {
     if (model)
       model->RemoveObserver(this);
   }
-  for (AppMenuObserver& observer : observer_list_)
-    observer.AppMenuDestroyed();
 }
 
 void AppMenu::Init(ui::MenuModel* model) {

chrome/browser/ui/views/toolbar/app_menu.h

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
 #include "base/timer/elapsed_timer.h"
@@ -33,7 +34,8 @@ class MenuRunner;
 // AppMenu adapts the AppMenuModel to view's menu related classes.
 class AppMenu : public views::MenuDelegate,
                 public bookmarks::BaseBookmarkModelObserver,
-                public content::NotificationObserver {
+                public content::NotificationObserver,
+                public base::SupportsWeakPtr<AppMenu> {
  public:
   enum RunFlags {
     NO_FLAGS = 0,

chrome/browser/ui/views/toolbar/app_menu_observer.h

@@ -7,9 +7,6 @@
 
 class AppMenuObserver {
  public:
-  // Invoked when the AppMenu is about to be destroyed (from its destructor).
-  virtual void AppMenuDestroyed() {}
-
   virtual void AppMenuClosed() {}
 
   // Called after AppMenu::RunMenu().