Fix regression: Download status doesn't appear anymore in the download-shelf

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

 // Copyright (c) 2011 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/download/download_item_cell.h"
 
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_manager.h"
@@ skipping 91 matching lines @@
 @end
 
 @implementation DownloadItemCell
 
 @synthesize secondaryTitle = secondaryTitle_;
 @synthesize secondaryFont = secondaryFont_;
 
 - (void)setInitialState {
   isStatusTextVisible_ = NO;
   titleY_ = kPrimaryTextPosTop;
-  statusAlpha_ = 1.0;
+  statusAlpha_ = 0.0;
 
   [self setFont:[NSFont systemFontOfSize:
       [NSFont systemFontSizeForControlSize:NSSmallControlSize]]];
   [self setSecondaryFont:[NSFont systemFontOfSize:
       [NSFont systemFontSizeForControlSize:NSSmallControlSize]]];
 
   [self updateTrackingAreas:self];
   [[NSNotificationCenter defaultCenter]
       addObserver:self
          selector:@selector(updateTrackingAreas:)
@@ skipping 14 matching lines @@
   if ((self = [super initTextCell:string])) {
     [self setInitialState];
   }
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   if ([completionAnimation_ isAnimating])
     [completionAnimation_ stopAnimation];
-  if ([showStatusAnimation_ isAnimating])
-    [showStatusAnimation_ stopAnimation];
-  if ([hideStatusAnimation_ isAnimating])
-    [hideStatusAnimation_ stopAnimation];
+  if ([toggleStatusVisibilityAnimation_ isAnimating])
+    [toggleStatusVisibilityAnimation_ stopAnimation];
   if (trackingAreaButton_) {
     [[self controlView] removeTrackingArea:trackingAreaButton_];
     trackingAreaButton_.reset();
   }
   if (trackingAreaDropdown_) {
     [[self controlView] removeTrackingArea:trackingAreaDropdown_];
     trackingAreaDropdown_.reset();
   }
   [secondaryTitle_ release];
   [secondaryFont_ release];
   [super dealloc];
 }
 
 - (void)setStateFromDownload:(BaseDownloadItemModel*)downloadModel {
   // Set the name of the download.
   downloadPath_ = downloadModel->download()->GetFileNameToReportUser();
 
   string16 statusText = downloadModel->GetStatusText();
   if (statusText.empty()) {
     // Remove the status text label.
     [self hideSecondaryTitle];
-    isStatusTextVisible_ = NO;
   } else {
     // Set status text.
     NSString* statusString = base::SysUTF16ToNSString(statusText);
     [self setSecondaryTitle:statusString];
     [self showSecondaryTitle];
-    isStatusTextVisible_ = YES;
   }
 
   switch (downloadModel->download()->state()) {
     case DownloadItem::COMPLETE:
       // Small downloads may start in a complete state due to asynchronous
       // notifications. In this case, we'll get a second complete notification
       // via the observers, so we ignore it and avoid creating a second complete
       // animation.
       if (completionAnimation_.get())
         break;
@@ skipping 403 matching lines @@
 }
 
 - (NSRect)imageRectForBounds:(NSRect)cellFrame {
   return NSMakeRect(cellFrame.origin.x + kImagePaddingLeft,
                     cellFrame.origin.y + kImagePaddingTop,
                     kImageWidth,
                     kImageHeight);
 }
 
 - (void)showSecondaryTitle {
-  if (!isStatusTextVisible_) {
+  if (isStatusTextVisible_)
+    return;
+  isStatusTextVisible_ = YES;
+  if (toggleStatusVisibilityAnimation_ &&
+      [toggleStatusVisibilityAnimation_ isAnimating]) {
+    // If the animation is hiding the status text, cancel the animation and
+    // show the status text immediately.
+    [toggleStatusVisibilityAnimation_ stopAnimation];
+    [self animation:toggleStatusVisibilityAnimation_ progressed:1.0];
+    toggleStatusVisibilityAnimation_.reset();
+  } else {
     // No core animation -- text in CA layers is not subpixel antialiased :-/

[Nico, 2011/07/04 16:39:02]

Can you s/No/Don't use/?

-    showStatusAnimation_.reset([[DownloadItemCellAnimation alloc]
-        initWithDownloadItemCell:self
-                        duration:kShowStatusDuration
-                  animationCurve:NSAnimationEaseIn]);
-    [showStatusAnimation_.get() setDelegate:self];
-    [showStatusAnimation_.get() startAnimation];
-  } else {
-    // If the status line continues to be visible, don't show an animation
-    [self animation:nil progressed:0.0];
+    toggleStatusVisibilityAnimation_.reset([[DownloadItemCellAnimation alloc]
+                                    initWithDownloadItemCell:self
+                                    duration:kShowStatusDuration
+                                    animationCurve:NSAnimationEaseIn]);

[Nico, 2011/07/04 16:39:02]

ObjC style is to align the colons (:) vertically – see the left side of this
diff.

+    [toggleStatusVisibilityAnimation_.get() setDelegate:self];
+    [toggleStatusVisibilityAnimation_.get() startAnimation];
+    NSLog(@"start");

[Nico, 2011/07/04 16:39:02]

Remove

   }
 }
 
 - (void)hideSecondaryTitle {

[Nico, 2011/07/04 16:39:02]

From what I can tell, this function is identical to the previous one except for
the first 3 lines. Can you factor the common part into a new function?

-  if (isStatusTextVisible_) {
-    // No core animation -- text in CA layers is not subpixel antialiased :-/
-    hideStatusAnimation_.reset([[DownloadItemCellAnimation alloc]
-        initWithDownloadItemCell:self
-                        duration:kHideStatusDuration
-                  animationCurve:NSAnimationEaseIn]);
-    [hideStatusAnimation_.get() setDelegate:self];
-    [hideStatusAnimation_.get() startAnimation];
-  } else {
+  if (!isStatusTextVisible_)
+    return;
+  isStatusTextVisible_ = NO;
+  if (toggleStatusVisibilityAnimation_ &&
+      [toggleStatusVisibilityAnimation_ isAnimating]) {
     // If the download is done so quickly that the status line is never visible,
     // don't show an animation
-    [self animation:nil progressed:1.0];
+    [toggleStatusVisibilityAnimation_ stopAnimation];
+    [self animation:toggleStatusVisibilityAnimation_ progressed:1.0];
+    toggleStatusVisibilityAnimation_.reset();
+  } else {
+    // No core animation -- text in CA layers is not subpixel antialiased :-/

[Nico, 2011/07/04 16:39:02]

here too

+    toggleStatusVisibilityAnimation_.reset([[DownloadItemCellAnimation alloc]
+                                initWithDownloadItemCell:self
+                                duration:kHideStatusDuration
+                                animationCurve:NSAnimationEaseIn]);

[Nico, 2011/07/04 16:39:02]

align : too

+    [toggleStatusVisibilityAnimation_.get() setDelegate:self];
+    [toggleStatusVisibilityAnimation_.get() startAnimation];
   }
 }
 
 - (void)animation:(NSAnimation*)animation
-      progressed:(NSAnimationProgress)progress {
-  if (animation == showStatusAnimation_) {
-    titleY_ = (1 - progress)*kPrimaryTextOnlyPosTop +
-      kPrimaryTextPosTop;
-    statusAlpha_ = progress;
-    [[self controlView] setNeedsDisplay:YES];
-  } else if (animation == hideStatusAnimation_ || animation == nil) {
-    titleY_ = progress*kPrimaryTextOnlyPosTop +
-        (1 - progress)*kPrimaryTextPosTop;
-    statusAlpha_ = 1 - progress;
+   progressed:(NSAnimationProgress)progress {
+     NSLog(@"progress");

[Nico, 2011/07/04 16:39:02]

remove

+  if (animation == toggleStatusVisibilityAnimation_) {
+    if (isStatusTextVisible_) {
+      titleY_ = (1 - progress)*kPrimaryTextOnlyPosTop + kPrimaryTextPosTop;
+      statusAlpha_ = progress;
+    } else {
+      titleY_ = progress*kPrimaryTextOnlyPosTop +
+          (1 - progress)*kPrimaryTextPosTop;
+      statusAlpha_ = 1 - progress;
+    }
     [[self controlView] setNeedsDisplay:YES];
   } else if (animation == completionAnimation_) {
     [[self controlView] setNeedsDisplay:YES];
   }
 }
 
 - (void)animationDidEnd:(NSAnimation *)animation {
-  if (animation == showStatusAnimation_)
-    showStatusAnimation_.reset();
-  else if (animation == hideStatusAnimation_)
-    hideStatusAnimation_.reset();
+  if (animation == toggleStatusVisibilityAnimation_)
+    toggleStatusVisibilityAnimation_.reset();
   else if (animation == completionAnimation_)
     completionAnimation_.reset();
 }
 
+- (BOOL)isStatusTextVisible {
+  return isStatusTextVisible_;
+}
+
+- (CGFloat)statusTextAlpha {
+  return statusAlpha_;
+}
+
+- (void)skipVisibilityAnimation {
+  [toggleStatusVisibilityAnimation_ setCurrentProgress:1.0];
+}
+
 @end
 
 @implementation DownloadItemCellAnimation
 
 - (id)initWithDownloadItemCell:(DownloadItemCell*)cell
                       duration:(NSTimeInterval)duration
                 animationCurve:(NSAnimationCurve)animationCurve {
   if ((self = [super gtm_initWithDuration:duration
                                 eventMask:NSLeftMouseDownMask
                            animationCurve:animationCurve])) {
     cell_ = cell;
     [self setAnimationBlockingMode:NSAnimationNonblocking];
   }
   return self;
 }
 
 - (void)setCurrentProgress:(NSAnimationProgress)progress {
   [super setCurrentProgress:progress];
   [cell_ animation:self progressed:progress];
 }
 
 @end