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

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

 // Copyright (c) 2012 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.
 
 #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
 
 #include <algorithm>
 
 #include "base/mac/bundle_locations.h"
 #include "base/mac/foundation_util.h"
@@ skipping 188 matching lines @@
 
 }  // namespace ToolbarControllerInternal
 
 @implementation ToolbarController
 
 @synthesize browser = browser_;
 
 - (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
                               bundle:base::mac::FrameworkBundle()])) {
     commands_ = commands;
     profile_ = profile;
     browser_ = browser;
-    resizeDelegate_ = resizeDelegate;
     hasToolbar_ = YES;
     hasLocationBar_ = YES;
 
     // Register for notifications about state changes for the toolbar buttons
     commandObserver_.reset(
         new ToolbarControllerInternal::CommandObserverBridge(self));
 
     commands->AddCommandObserver(IDC_BACK, commandObserver_.get());
     commands->AddCommandObserver(IDC_FORWARD, commandObserver_.get());
     commands->AddCommandObserver(IDC_RELOAD, commandObserver_.get());
     commands->AddCommandObserver(IDC_HOME, commandObserver_.get());
     commands->AddCommandObserver(IDC_BOOKMARK_PAGE, commandObserver_.get());
     // NOTE: Don't remove the command observers. ToolbarController is
     // autoreleased at about the same time as the CommandUpdater (owned by the
     // Browser), so |commands_| may not be valid any more.
   }
   return self;
 }
 
 - (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.
 - (void)awakeFromNib {
   [[backButton_ cell] setImageID:IDR_BACK
                   forButtonState:image_button_cell::kDefaultState];
   [[backButton_ cell] setImageID:IDR_BACK_H
                   forButtonState:image_button_cell::kHoverState];
   [[backButton_ cell] setImageID:IDR_BACK_P
                   forButtonState:image_button_cell::kPressedState];
@@ skipping 107 matching lines @@
   // ViewIDs of |toolbarView|, |reloadButton_|, |locationBar_| and
   // |browserActionsContainerView_| are handled by themselves.
   view_id_util::SetID(backButton_, VIEW_ID_BACK_BUTTON);
   view_id_util::SetID(forwardButton_, VIEW_ID_FORWARD_BUTTON);
   view_id_util::SetID(homeButton_, VIEW_ID_HOME_BUTTON);
   view_id_util::SetID(wrenchButton_, VIEW_ID_APP_MENU);
 
   [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);
   [[backButton_ cell]
       accessibilitySetOverrideValue:description
                        forAttribute:NSAccessibilityDescriptionAttribute];
   description = l10n_util::GetNSStringWithFixup(IDS_ACCNAME_FORWARD);
   [[forwardButton_ cell]
       accessibilitySetOverrideValue:description
                        forAttribute:NSAccessibilityDescriptionAttribute];
@@ skipping 501 matching lines @@
 - (void)hideDropURLsIndicatorInView:(NSView*)view {
   // Do nothing.
 }
 
 // (URLDropTargetController protocol)
 - (BOOL)isUnsupportedDropData:(id<NSDraggingInfo>)info {
   return drag_util::IsUnsupportedDropData(profile_, info);
 }
 
 @end