[Mac] DownloadShelf should be notified when autoclosing.

chrome/browser/ui/cocoa/download/download_shelf_controller.mm

Index: chrome/browser/ui/cocoa/download/download_shelf_controller.mm
diff --git a/chrome/browser/ui/cocoa/download/download_shelf_controller.mm b/chrome/browser/ui/cocoa/download/download_shelf_controller.mm
index 16ea814138133fb0a5a7335bf60b4bd9ccc94fc5..6f460040a9e7da73c91d9c29617430789e7bde15 100644
--- a/chrome/browser/ui/cocoa/download/download_shelf_controller.mm
+++ b/chrome/browser/ui/cocoa/download/download_shelf_controller.mm
@@ -71,16 +71,13 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
 }  // namespace
 
 @interface DownloadShelfController(Private)
-- (void)showDownloadShelf:(BOOL)enable;
 - (void)layoutItems:(BOOL)skipFirst;
 - (void)closed;
-- (BOOL)canAutoClose;
-
+- (void)maybeAutoCloseAfterDelay;
+- (void)autoClose;
 - (void)viewFrameDidChange:(NSNotification*)notification;
-
 - (void)installTrackingArea;
 - (void)cancelAutoCloseAndRemoveTrackingArea;
-
 - (void)willEnterFullscreen;
 - (void)willLeaveFullscreen;
 - (void)updateCloseButton;
@@ -171,10 +168,14 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
   return static_cast<AnimatableView*>([self view]);
 }
 
-- (void)showDownloadsTab:(id)sender {
+- (IBAction)showDownloadsTab:(id)sender {
   chrome::ShowDownloads(bridge_->browser());
 }
 
+- (IBAction)handleClose:(id)sender {
+  bridge_->Close(DownloadShelf::USER_ACTION);
+}
+
 - (void)remove:(DownloadItemController*)download {
   // Look for the download in our controller array and remove it. This will
   // explicity release it so that it removes itself as an Observer of the
@@ -186,20 +187,17 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
   [[download view] removeFromSuperview];
 
   [downloadItemControllers_ removeObject:download];
-
   [self layoutItems];
 
-  // Check to see if we have any downloads remaining and if not, hide the shelf.
-  if (![downloadItemControllers_ count])
-    [self showDownloadShelf:NO];
+  // If there are no more downloads or if all the remaining downloads have been
+  // opened, we can close the shelf.
+  [self maybeAutoCloseAfterDelay];

[Nico, 2013/03/22 21:29:17]

maybeACAD installs a tracking area to check if the user mouses out of the
download shelf, but for remove: calls it's not a given that the cursor is in the
download shelf, right?

[asanka, 2013/03/22 21:33:24]

It's not a given. This is true for all calls of maybeAutoCloseAfterDelay since
both download opening and removal can happen without the user interacting with
the shelf.

 }
 
 - (void)downloadWasOpened:(DownloadItemController*)item_controller {
   // This should only be called on the main thead.
   DCHECK([NSThread isMainThread]);
-
-  if ([self canAutoClose])
-    [self installTrackingArea];
+  [self maybeAutoCloseAfterDelay];
 }
 
 // We need to explicitly release our download controllers here since they need
@@ -210,24 +208,34 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
   downloadItemControllers_.reset();
 }
 
-// Show or hide the bar based on the value of |enable|. Handles animating the
-// resize of the content view.
-- (void)showDownloadShelf:(BOOL)enable {
-  if ([self isVisible] == enable)
+- (void)showDownloadShelf:(BOOL)show
+             isUserAction:(BOOL)isUserAction {
+  if ([self isVisible] == show)
     return;
 
+  if (!show) {
+    [self cancelAutoCloseAndRemoveTrackingArea];
+    int numInProgress = 0;
+    for (NSUInteger i = 0; i < [downloadItemControllers_ count]; ++i) {
+      if ([[downloadItemControllers_ objectAtIndex:i]download]->IsInProgress())
+        ++numInProgress;
+    }
+    download_util::RecordShelfClose(
+        [downloadItemControllers_ count], numInProgress, !isUserAction);
+  }
+
   // Animate the shelf out, but not in.
   // TODO(rohitrao): We do not animate on the way in because Cocoa is already
   // doing a lot of work to set up the download arrow animation.  I've chosen to
   // do no animation over janky animation.  Find a way to make animating in
   // smoother.
   AnimatableView* view = [self animatableView];
-  if (enable)
+  if (show)
     [view setHeight:maxShelfHeight_];
   else
     [view animateToNewHeight:0 duration:kDownloadShelfCloseDuration];
 
-  barIsVisible_ = enable;
+  barIsVisible_ = show;
   [self updateCloseButton];
 }
 
@@ -239,31 +247,6 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
   return barIsVisible_;
 }
 
-- (void)show:(id)sender {
-  [self showDownloadShelf:YES];
-}
-
-- (void)hide:(id)sender {
-  [self cancelAutoCloseAndRemoveTrackingArea];
-
-  // If |sender| isn't nil, then we're being closed from the UI by the user and
-  // we need to tell our shelf implementation to close. Otherwise, we're being
-  // closed programmatically by our shelf implementation.
-  bool auto_closed = (sender == nil);
-
-  int numInProgress = 0;
-  for (NSUInteger i = 0; i < [downloadItemControllers_ count]; ++i) {
-    if ([[downloadItemControllers_ objectAtIndex:i]download]->IsInProgress())
-      ++numInProgress;
-  }
-  download_util::RecordShelfClose(
-      [downloadItemControllers_ count], numInProgress, auto_closed);
-  if (auto_closed)
-    [self showDownloadShelf:NO];
-  else
-    bridge_->Close();
-}
-
 - (void)animationDidEnd:(NSAnimation*)animation {
   if (![self isVisible])
     [self closed];
@@ -383,24 +366,41 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
 - (void)mouseExited:(NSEvent*)event {
   // Cancel any previous hide requests, just to be safe.
   [NSObject cancelPreviousPerformRequestsWithTarget:self
-                                           selector:@selector(hide:)
-                                             object:self];
+                                           selector:@selector(autoClose)
+                                             object:nil];
 
   // Schedule an autoclose after a delay.  If the mouse is moved back into the
   // view, or if an item is added to the shelf, the timer will be canceled.
-  [self performSelector:@selector(hide:)
-             withObject:self
+  [self performSelector:@selector(autoClose)
+             withObject:nil
              afterDelay:kAutoCloseDelaySeconds];
 }
 
-- (BOOL)canAutoClose {
+- (void)maybeAutoCloseAfterDelay {
+  // We can close the shelf automatically if all the downloads on the shelf have
+  // been opened.
   for (NSUInteger i = 0; i < [downloadItemControllers_ count]; ++i) {
     DownloadItemController* itemController =
         [downloadItemControllers_ objectAtIndex:i];
     if (![itemController download]->GetOpened())
-      return NO;
+      return;
   }
-  return YES;
+
+  if ([self isVisible] && [downloadItemControllers_ count] > 0) {
+    // If the shelf is visible and has download items remaining on it, close the
+    // shelf after the user moves the mouse out of the download shelf.

[Nico, 2013/03/22 21:42:47]

Might want to mention that this is somewhat optimistically done as the cursor
isn't necessarily over the shelf. (Should we detect that and not install a
tracking area in that case?)

[asanka, 2013/03/29 15:56:20]

I added a comment describing the current behavior.

Detecting the case and closing immediately is done in
https://codereview.chromium.org/13318002/

+    [self installTrackingArea];
+  } else {
+    // We notify the DownloadShelf of our intention to close even if the shelf
+    // is currently hidden. If the shelf was temporarily hidden (e.g. because
+    // the browser window entered fullscreen mode), then this prevents the shelf
+    // from being shown again when the browser exits fullscreen mode.
+    [self autoClose];
+  }
+}
+
+- (void)autoClose {
+  bridge_->Close(DownloadShelf::AUTOMATIC);
 }
 
 - (void)installTrackingArea {
@@ -420,8 +420,8 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 };
 
 - (void)cancelAutoCloseAndRemoveTrackingArea {
   [NSObject cancelPreviousPerformRequestsWithTarget:self
-                                           selector:@selector(hide:)
-                                             object:self];
+                                           selector:@selector(autoClose)
+                                             object:nil];
 
   if (trackingArea_.get()) {
     [[self view] removeTrackingArea:trackingArea_];