Fix disabled browser action appearance on Mac.

chrome/browser/ui/cocoa/extensions/browser_action_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/extensions/browser_action_button.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/logging.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/extensions/extension_action_icon_factory.h"
 #include "chrome/browser/ui/cocoa/extensions/extension_action_context_menu.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
 #include "grit/theme_resources.h"
 #include "skia/ext/skia_utils_mac.h"
 #import "third_party/GTM/AppKit/GTMNSAnimation+Duration.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas_skia_paint.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h"
 #include "ui/gfx/size.h"
 
 using extensions::Extension;
 
 NSString* const kBrowserActionButtonDraggingNotification =
     @"BrowserActionButtonDraggingNotification";
 NSString* const kBrowserActionButtonDragEndNotification =
     @"BrowserActionButtonDragEndNotification";
@@ skipping 91 matching lines @@
     [cell setExtensionAction:extension->browser_action()];
     [cell
         accessibilitySetOverrideValue:base::SysUTF8ToNSString(extension->name())
         forAttribute:NSAccessibilityDescriptionAttribute];
     [cell setImageID:IDR_BROWSER_ACTION
       forButtonState:image_button_cell::kDefaultState];
     [cell setImageID:IDR_BROWSER_ACTION_H
       forButtonState:image_button_cell::kHoverState];
     [cell setImageID:IDR_BROWSER_ACTION_P
       forButtonState:image_button_cell::kPressedState];
+    [cell setImageID:IDR_BROWSER_ACTION
+      forButtonState:image_button_cell::kDisabledState];
 
     [self setTitle:@""];
     [self setButtonType:NSMomentaryChangeButton];
     [self setShowsBorderOnlyWhileMouseInside:YES];
 
     [self setMenu:[[[ExtensionActionContextMenu alloc]
         initWithExtension:extension
                   browser:browser
           extensionAction:extension->browser_action()] autorelease]];
 
@@ skipping 92 matching lines @@
   if (tabId_ < 0)
     return;
 
   std::string tooltip = extension_->browser_action()->GetTitle(tabId_);
   if (tooltip.empty()) {
     [self setToolTip:nil];
   } else {
     [self setToolTip:base::SysUTF8ToNSString(tooltip)];
   }
 
-  gfx::Image image = iconFactoryBridge_->GetIcon(tabId_);
+  gfx::ImageSkia image = iconFactoryBridge_->GetIcon(tabId_).AsImageSkia();
 
-  if (!image.IsEmpty())
-    [self setImage:image.ToNSImage()];
+  bool enabled = extension_->browser_action()->GetIsVisible(tabId_);
+  if (!image.isNull()) {
+    if (!enabled)
+      image = gfx::ImageSkiaOperations::CreateTransparentImage(image, 0.4);
+    [self setImage:gfx::Image(image).ToNSImage()];

[Nico, 2012/09/21 02:02:02]

It looks like we're round-tripping to skia and back just for dimming. Can you do
this in paint (see below)?

If you decide to keep it here,can you avoid the roundtrip in the "enabled" case
at least?

[Yoyo Zhou, 2012/09/21 18:04:59]

Ah, I didn't realize this was something to avoid. Doing it below seems to work.

+  }
+
+  [self setEnabled:enabled ? YES : NO];

[Nico, 2012/09/21 02:02:02]

just "enabled" is enough. Drop the ?YES:NO.

[Yoyo Zhou, 2012/09/21 18:04:59]

Done.

 
   [[self cell] setTabId:tabId_];
 
-  bool enabled = extension_->browser_action()->GetIsVisible(tabId_);
-  [self setEnabled:enabled ? YES : NO];
-
   [self setNeedsDisplay:YES];
 }
 
 - (BOOL)isAnimating {
   return [moveAnimation_ isAnimating];
 }
 
 - (NSImage*)compositedImage {
   NSRect bounds = [self bounds];
   NSImage* image = [[[NSImage alloc] initWithSize:bounds.size] autorelease];
@@ skipping 40 matching lines @@
   gfx::ScopedNSGraphicsContextSaveGState scopedGState;
   [super drawWithFrame:cellFrame inView:controlView];
   const NSSize imageSize = self.image.size;
   const NSRect imageRect =
       NSMakeRect(std::floor((NSWidth(cellFrame) - imageSize.width) / 2.0),
                  std::floor((NSHeight(cellFrame) - imageSize.height) / 2.0),
                  imageSize.width, imageSize.height);
   [self.image drawInRect:imageRect
                 fromRect:NSZeroRect
                operation:NSCompositeSourceOver
                 fraction:1.0

[Nico, 2012/09/21 02:02:02]

Can't you just change this to "enabled ? 1.0 : 0.4" instead?

[Yoyo Zhou, 2012/09/21 18:04:59]

Done. Slightly scared by operator precendence.

           respectFlipped:YES
                    hints:nil];
 
   cellFrame.origin.y += kBrowserActionBadgeOriginYOffset;
   [self drawBadgeWithinFrame:cellFrame];
 }
 
 @end