Fix darkly-drawn Show All button in Downloads Shelf.

chrome/browser/ui/cocoa/download/download_show_all_button.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/download/download_show_all_button.h"
 
 #include "base/logging.h"
 #import "chrome/browser/ui/cocoa/download/download_show_all_cell.h"
 #import "chrome/browser/ui/cocoa/view_id_util.h"
 #include "grit/theme_resources.h"
 #import "ui/base/cocoa/nsview_additions.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
 
 @implementation DownloadShowAllButton
 
 - (void)awakeFromNib {
   DCHECK([[self cell] isKindOfClass:[DownloadShowAllCell class]]);
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   NSImage* favicon = rb.GetNativeImageNamed(IDR_DOWNLOADS_FAVICON).ToNSImage();
   [self setImage:favicon];
 }
 
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+
+  [super dealloc];
+}
+
 // GTM's layout tweaker calls sizeToFit to receive the desired width of views.
 // By default, buttons will be only 14px high, but the Show All button needs to
 // be higher.
 - (void)sizeToFit {
   NSRect oldRect = [self frame];
   [super sizeToFit];
   NSRect newRect = [self frame];
 
   // Keep old height.
   newRect.origin.y = oldRect.origin.y;
   newRect.size.height = oldRect.size.height;
 
   [self setFrame:newRect];
 }
 
 - (BOOL)isOpaque {
   // Make this control opaque so that sub-pixel anti-aliasing works when
   // CoreAnimation is enabled.
   return YES;
 }
 
+- (void)downloadShelfFrameDidChange:(NSNotification*)aNotification {
+  [[NSNotificationCenter defaultCenter]
+      removeObserver:self
+                name:NSViewFrameDidChangeNotification
+              object:[self ancestorWithViewID:VIEW_ID_DOWNLOAD_SHELF]];
+
+  [self setNeedsDisplay:YES];
+}
+
 - (void)drawRect:(NSRect)rect {
   NSView* downloadShelfView = [self ancestorWithViewID:VIEW_ID_DOWNLOAD_SHELF];
-  [self cr_drawUsingAncestor:downloadShelfView inRect:rect];
+  if ([downloadShelfView bounds].size.height < [self bounds].size.height) {
+    // Ordinarily the show all button uses cr_drawUsingAncestor:inRect: to use
+    // the download shelf to draw its background. However when the download
+    // shelf has zero height, the shelf's drawing methods don't work as expected
+    // because they constrain their drawing to the shelf's bounds rect. This
+    // situation occurs sometimes when the shelf is about to become visible,
+    // and the result is a very dark show all button.
+    //
+    // To work around this problem, we'll draw a color that roughly matches
+    // the shelf's background gradient, and then wait for the shelf to change
+    // its frame, at which point we'll schedule a redraw (which will work
+    // correctly because the shelf will have a non-zero height).
+    [[NSColor colorWithCalibratedWhite:0.8 alpha:1.0] set];

[asanka, 2015/05/08 17:49:39]

Is there a way to determine this at runtime without hardcoding?

[shrike, 2015/05/08 18:27:48]

Not cleanly. The code that we need lives in [BackgroundGradientView
drawBackground:], but to copy/paste that into DownloadShowAllButton requires
exposing a private method from BackgroundGradientView (backgroundImageColor).

Calling [BackgroundGradientView drawBackground:] directly can't be done because
it relies on a static varliable set up in cr_drawUsingAncestor:inRect:.

In my testing the hard-coded background color version comes onscreen for a small
fraction of a second before being replaced by the real one.

It might be possible to say

- (BOOL)isOpaque {
  return [downloadShelfView bounds].size.height > 0.0;
}

and skip trying to draw the background in drawRect: (but still wait for the
shelf to grow in height and request a redraw).

[asanka, 2015/05/12 00:20:51]

The only thing I'm worried about is whether this would result in a visible flash
if there's a big difference in contrast between the theme background and the
color we are choosing. It would be great if we can find a solution that avoids
this possibility.

+    NSRectFill(rect);
+    NSNotificationCenter* defaultCenter = [NSNotificationCenter defaultCenter];
+    [defaultCenter addObserver:self
+                      selector:@selector(downloadShelfFrameDidChange:)
+                          name:NSViewFrameDidChangeNotification
+                        object:downloadShelfView];
+  } else {

[asanka, 2015/05/08 17:48:36]

Do we need the observer if there was a successful paint?

[shrike, 2015/05/08 18:27:48]

No, we should not. Are you asking because you see an issue? There should be no
observer at this point.

[asanka, 2015/05/12 00:20:51]

Right. Misread.

+    [self cr_drawUsingAncestor:downloadShelfView inRect:rect];
+  }
   [super drawRect:rect];
 }
 
 // ThemedWindowDrawing implementation.
 
 - (void)windowDidChangeTheme {
   [self setNeedsDisplay:YES];
 }
 
 - (void)windowDidChangeActive {
   [self setNeedsDisplay:YES];
 }
 
 @end