Limit overlap between BubbleManager and BaseBubbleController.

chrome/browser/ui/cocoa/base_bubble_controller.mm

Index: chrome/browser/ui/cocoa/base_bubble_controller.mm
diff --git a/chrome/browser/ui/cocoa/base_bubble_controller.mm b/chrome/browser/ui/cocoa/base_bubble_controller.mm
index 11b921226f8e8906ff8a87a92d6cfb78930414f4..d368805f5f580d7cfc9171655f935b6dfcb42cb4 100644
--- a/chrome/browser/ui/cocoa/base_bubble_controller.mm
+++ b/chrome/browser/ui/cocoa/base_bubble_controller.mm
@@ -15,6 +15,7 @@
 #import "chrome/browser/ui/cocoa/info_bubble_view.h"
 #import "chrome/browser/ui/cocoa/info_bubble_window.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_model_observer_bridge.h"
+#include "components/bubble/bubble_controller.h"
 
 @interface BaseBubbleController (Private)
 - (void)registerForNotifications;
@@ -28,6 +29,12 @@
 - (void)parentWindowWillClose:(NSNotification*)notification;
 - (void)parentWindowWillToggleFullScreen:(NSNotification*)notification;
 - (void)closeCleanup;
+
+// Temporary methods to decide how to close the bubble controller.
+// TODO(hcarmona): remove these methods when all bubbles use the new API.
+- (void)closeBubbleWithReason:(BubbleCloseReason)reason;
+// Will be a noop in new bubble API because this is handled elsewhere.
+- (void)closeBubbleWithoutReason;

[groby-ooo-7-16, 2015/09/04 19:13:38]

Just call it closeBubble - it's not like we're actually closing without having a
reason for it :)

[hcarmona, 2015/09/09 23:33:02]

Done.

 @end
 
 @implementation BaseBubbleController
@@ -36,6 +43,7 @@
 @synthesize bubble = bubble_;
 @synthesize shouldOpenAsKeyWindow = shouldOpenAsKeyWindow_;
 @synthesize shouldCloseOnResignKey = shouldCloseOnResignKey_;
+@synthesize bubbleReference = bubbleReference_;
 
 - (id)initWithWindowNibPath:(NSString*)nibPath
                parentWindow:(NSWindow*)parentWindow
@@ -228,12 +236,12 @@
 
 - (void)parentWindowWillClose:(NSNotification*)notification {
   [self setParentWindow:nil];
-  [self close];
+  [self closeBubbleWithoutReason];
 }
 
 - (void)parentWindowWillToggleFullScreen:(NSNotification*)notification {
   [self setParentWindow:nil];
-  [self close];
+  [self closeBubbleWithoutReason];
 }
 
 - (void)closeCleanup {
@@ -252,6 +260,18 @@
   tabStripObserverBridge_.reset();
 }
 
+- (void)closeBubbleWithReason:(BubbleCloseReason)reason {
+  if ([self bubbleReference])
+    [self bubbleReference]->CloseBubble(reason);
+  else
+    [self close];
+}
+
+- (void)closeBubbleWithoutReason {

[groby-ooo-7-16, 2015/09/04 19:13:38]

That's rather... icky. This means that we won't close at all if we have a
bubbleReference. Not sure this makes sense to me?

[hcarmona, 2015/09/04 20:58:01]

This should only be called where it's redundant for bubbles managed by
the bubble manager. I _could_ close the bubbles if we have a bubble
reference too, but then we'd be doing it in two places. Shouldn't be the
end of the world because we'd be removing it from the manager, but we'd
have duplicate BubbleClose for the same reason.

[groby-ooo-7-16, 2015/09/04 21:38:03]

Can we kill that duplication?

[hcarmona, 2015/09/04 21:49:39]

That's what this is attempting to do by not calling |CloseBubble| on the
bubble reference. The BubbleManager will notify the bubble to close when
the parent closes, when fullscreen is toggled, and when a tab changes.
Those functions should go away from this class when all bubbles are
using the bubble API.

+  if (![self bubbleReference])
+    [self close];
+}
+
 - (void)windowWillClose:(NSNotification*)notification {
   [self closeCleanup];
   [[NSNotificationCenter defaultCenter] removeObserver:self];
@@ -296,7 +316,7 @@
   // Don't close when explicily disabled, or if there's an attached sheet (e.g.
   // Open File dialog).
   if ([self shouldCloseOnResignKey] && ![window attachedSheet]) {
-    [self close];
+    [self closeBubbleWithReason:BUBBLE_CLOSE_FOCUS_LOST];
     return;
   }
 
@@ -369,7 +389,7 @@
 - (IBAction)cancel:(id)sender {
   // This is not a "real" cancel as potential changes to the radio group are not
   // undone. That's ok.
-  [self close];
+  [self closeBubbleWithReason:BUBBLE_CLOSE_CANCELED];
 }
 
 // Takes the |anchor_| point and adjusts the window's origin accordingly.
@@ -429,7 +449,7 @@
                         atIndex:(NSInteger)index
                          reason:(int)reason {
   // The user switched tabs; close.
-  [self close];
+  [self closeBubbleWithoutReason];
 }
 
 @end  // BaseBubbleController