[Mac] Inform reference counted objects that hold a weak Browser* when the Browser is being destroye…

chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm

Index: chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm
diff --git a/chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm b/chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm
index 1a1c45512c99171b9a24d884945b9ed838aaf90e..6f42b326894431426a7d06d4fd206af9d56cb91f 100644
--- a/chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm
+++ b/chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm
@@ -206,7 +206,6 @@ class NotificationBridge : public WrenchMenuBadgeController::Delegate {
 - (id)initWithCommands:(CommandUpdater*)commands
                profile:(Profile*)profile
                browser:(Browser*)browser
-        resizeDelegate:(id<ViewResizer>)resizeDelegate
           nibFileNamed:(NSString*)nibName {
   DCHECK(commands && profile && [nibName length]);
   if ((self = [super initWithNibName:nibName
@@ -214,7 +213,6 @@ class NotificationBridge : public WrenchMenuBadgeController::Delegate {
     commands_ = commands;
     profile_ = profile;
     browser_ = browser;
-    resizeDelegate_ = resizeDelegate;
     hasToolbar_ = YES;
     hasLocationBar_ = YES;
 
@@ -236,41 +234,15 @@ class NotificationBridge : public WrenchMenuBadgeController::Delegate {
 
 - (id)initWithCommands:(CommandUpdater*)commands
                profile:(Profile*)profile
-               browser:(Browser*)browser
-        resizeDelegate:(id<ViewResizer>)resizeDelegate {
+               browser:(Browser*)browser {
   if ((self = [self initWithCommands:commands
                              profile:profile
                              browser:browser
-                      resizeDelegate:resizeDelegate
                         nibFileNamed:@"Toolbar"])) {
   }
   return self;
 }
 
-
-- (void)dealloc {
-  browserActionsContainerDelegate_.reset();
-
-  // Unset ViewIDs of toolbar elements.
-  // ViewIDs of |toolbarView|, |reloadButton_|, |locationBar_| and
-  // |browserActionsContainerView_| are handled by themselves.
-  view_id_util::UnsetID(backButton_);
-  view_id_util::UnsetID(forwardButton_);
-  view_id_util::UnsetID(homeButton_);
-  view_id_util::UnsetID(wrenchButton_);
-
-  // Make sure any code in the base class which assumes [self view] is
-  // the "parent" view continues to work.
-  hasToolbar_ = YES;
-  hasLocationBar_ = YES;
-
-  [[NSNotificationCenter defaultCenter] removeObserver:self];
-
-  if (trackingArea_.get())
-    [[self view] removeTrackingArea:trackingArea_.get()];
-  [super dealloc];
-}
-
 // Called after the view is done loading and the outlets have been hooked up.
 // Now we can hook up bridges that rely on UI objects such as the location
 // bar and button state.
@@ -398,6 +370,37 @@ class NotificationBridge : public WrenchMenuBadgeController::Delegate {
   [self addAccessibilityDescriptions];
 }
 
+- (void)browserWillBeDestroyed {

[erikchen, 2015/07/08 01:18:03]

This method makes the assumption that this is guaranteed to be called before
teardown. This makes the class harder to unit test, and also adds implicit
behavior restrictions on consumers of the class.

How about:
1) Add an idempotent method -cleanUp.
2) Call -cleanUp from browserWillBeDestroyed and -dealloc.

Ditto for all other instances of this method.

[jackhou1, 2015/07/08 07:02:12]

Yeah good point. For most of these it's fine to just call
-browserWillBeDestroyed in -dealloc. I think we can add -cleanUp for just those
classes that need it.

+  // This method indicates imminent destruction. Destroy owned objects that hold
+  // a weak Browser*, or pass this call onto reference counted objects.
+  locationBarView_.reset();
+  [backMenuController_ browserWillBeDestroyed];
+  [forwardMenuController_ browserWillBeDestroyed];
+  [browserActionsController_ browserWillBeDestroyed];
+  [wrenchMenuController_ browserWillBeDestroyed];
+  browserActionsContainerDelegate_.reset();
+
+  // Unset ViewIDs of toolbar elements.
+  // ViewIDs of |toolbarView|, |reloadButton_|, |locationBar_| and
+  // |browserActionsContainerView_| are handled by themselves.
+  view_id_util::UnsetID(backButton_);
+  view_id_util::UnsetID(forwardButton_);
+  view_id_util::UnsetID(homeButton_);
+  view_id_util::UnsetID(wrenchButton_);
+
+  // Make sure any code in the base class which assumes [self view] is
+  // the "parent" view continues to work.
+  hasToolbar_ = YES;
+  hasLocationBar_ = YES;
+
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+
+  if (trackingArea_.get())
+    [[self view] removeTrackingArea:trackingArea_.get()];
+
+  browser_ = nullptr;
+}
+
 - (void)addAccessibilityDescriptions {
   // Set accessibility descriptions. http://openradar.appspot.com/7496255
   NSString* description = l10n_util::GetNSStringWithFixup(IDS_ACCNAME_BACK);