Fix the misalignment on CrOS of the tab background images

chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

Index: chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
diff --git a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
index c99c63fae6c0f9031e5aba28356c75398d822312..e60c272641a0d7ae30c1e474de8c5b0094275b5d 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -30,6 +30,7 @@
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#import "chrome/browser/ui/cocoa/browser_window_utils.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h"
 #include "chrome/browser/ui/cocoa/drag_util.h"
 #import "chrome/browser/ui/cocoa/image_button_cell.h"
@@ -191,33 +192,16 @@ NSImage* CreateImageWithSize(NSSize size,
 // Takes a normal bitmap and a mask image and returns an image the size of the
 // mask that has pixels from |image| but alpha information from |mask|.
 NSImage* ApplyMask(NSImage* image, NSImage* mask) {
-  return [CreateImageWithSize([mask size], ^(NSSize size) {
-      // Skip a few pixels from the top of the tab background gradient, because
-      // the new tab button is not drawn at the very top of the browser window.
-      const int kYOffset = 10;
-      CGFloat width = size.width;
-      CGFloat height = size.height;
-
-      // In some themes, the tab background image is narrower than the
-      // new tab button, so tile the background image.
-      CGFloat x = 0;
-      // The floor() is to make sure images with odd widths don't draw to the
-      // same pixel twice on retina displays. (Using NSDrawThreePartImage()
-      // caused a startup perf regression, so that cannot be used.)
-      CGFloat tileWidth = floor(std::min(width, [image size].width));
-      while (x < width) {
-        [image drawAtPoint:NSMakePoint(x, 0)
-                  fromRect:NSMakeRect(0,
-                                      [image size].height - height - kYOffset,
-                                      tileWidth,
-                                      height)
-                 operation:NSCompositeCopy
-                  fraction:1.0];
-        x += tileWidth;
-      }
+  DCHECK_EQ([image size].width, [mask size].width);
+  DCHECK_EQ([image size].height, [mask size].height);
 
+  return [CreateImageWithSize([mask size], ^(NSSize size) {
+      [image drawAtPoint:NSZeroPoint
+                fromRect:NSMakeRect(0, 0, size.width, size.height)
+               operation:NSCompositeCopy
+                fraction:1.0];
       [mask drawAtPoint:NSZeroPoint
-               fromRect:NSMakeRect(0, 0, width, height)
+               fromRect:NSMakeRect(0, 0, size.width, size.height)
               operation:NSCompositeDestinationIn
                fraction:1.0];
   }) autorelease];
@@ -275,6 +259,49 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   }) autorelease];
 }
 
+// Tiles |image| horizontally into a new NSImage of |size|. |yInset| refers
+// to the position from the bottom of the returned image at which the top of
+// |image| should be painted.
+NSImage* TileHorizontally(NSImage* image, CGFloat yInset, NSSize size) {

[sail, 2013/08/22 18:23:01]

Could use better variable names. For example:
   size -> dest_size
  yInset -> dest_y_inset or src_y_inset

+  return [CreateImageWithSize(size, ^(NSSize size) {

[sail, 2013/08/22 18:23:01]

shouldn't use the same variable name as the outer scope

+      CGFloat width = size.width;

[sail, 2013/08/22 18:23:01]

Same problem with variable names. Maybe remove all together?

+      CGFloat height = size.height;
+
+      CGFloat x = 0;
+      // The floor() is to make sure images with odd widths and height don't
+      // draw to the same pixel twice on retina displays. (Using
+      // NSDrawThreePartImage() caused a startup perf regression, so that
+      // cannot be used.)
+      CGFloat tileWidth = floor(std::min(width, [image size].width));
+      while (x < width) {
+        [image drawAtPoint:NSMakePoint(x, 0)
+                  fromRect:NSMakeRect(0,
+                                      [image size].height - yInset,
+                                      tileWidth,
+                                      height)
+                 operation:NSCompositeCopy
+                  fraction:1.0];
+        x += tileWidth;
+      }
+  }) autorelease];
+}
+
+// Computes the y position that the top of a theme image with |alignment|
+// should be painted at. The returned y position is in the coordinates of an
+// NSImage of |dst_height| which is centered on the new tab button. This method
+// is a helper for [TabStripController setNewTabImages].
+int NewTabButtonThemeBackgroundYPosition(TabStripView* tabstrip,
+                                         ThemeImageAlignment alignment,
+                                         CGFloat dstHeight) {
+  CGFloat yPositionInTabstrip =

[sail, 2013/08/22 18:23:01]

use c style variable names? more above and below

+      [BrowserWindowUtils themeImagePositionInTabStripCoords:tabstrip
+                                                   alignment:alignment].y;
+  // The image is centered in the new tab button.
+  NSView* newTabButton = [tabstrip getNewTabButton];
+  return yPositionInTabstrip -
+      roundf((NSHeight([newTabButton bounds]) - dstHeight) / 2.0);
+}
+
 }  // namespace
 
 @interface TabStripController (Private)
@@ -2268,9 +2295,30 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   NSImage* normal = rb.GetNativeImageNamed(IDR_NEWTAB_BUTTON).ToNSImage();
   NSImage* hover = rb.GetNativeImageNamed(IDR_NEWTAB_BUTTON_H).ToNSImage();
   NSImage* pressed = rb.GetNativeImageNamed(IDR_NEWTAB_BUTTON_P).ToNSImage();
+  NSImage* bgActive = theme->GetNSImageNamed(IDR_THEME_TAB_BACKGROUND);
+  NSImage* bgActiveOverlay = nil;
+  if (theme->HasCustomImage(IDR_THEME_TAB_BACKGROUND_OVERLAY)) {
+    bgActiveOverlay = theme->GetNSImageNamed(
+        IDR_THEME_TAB_BACKGROUND_OVERLAY);
+  }
 
-  NSImage* foreground = ApplyMask(
-      theme->GetNSImageNamed(IDR_THEME_TAB_BACKGROUND), mask);
+  int yPositionAlignWithFrame = NewTabButtonThemeBackgroundYPosition(
+      tabStripView_, THEME_IMAGE_ALIGN_WITH_FRAME, [mask size].height);
+  int yPositionAlignWithTabStrip = NewTabButtonThemeBackgroundYPosition(
+      tabStripView_, THEME_IMAGE_ALIGN_WITH_TAB_STRIP, [mask size].height);
+
+  // Create the foreground image by combining the tinted frame image
+  // (IDR_THEME_TAB_BACKGROUND) and the custom theme provided overlay
+  // (IDR_THEME_TAB_BACKGROUND_OVERLAY). Tile the images because the images
+  // can be narrower than the new tab button for some themes.
+  NSImage* unmaskedForeground = TileHorizontally(bgActive,
+      yPositionAlignWithFrame, [mask size]);
+  if (bgActiveOverlay) {
+    NSImage* tiledOverlay = TileHorizontally(bgActiveOverlay,
+        yPositionAlignWithTabStrip, [mask size]);
+    unmaskedForeground = Overlay(unmaskedForeground, tiledOverlay, 1.0);
+  }
+  NSImage* foreground = ApplyMask(unmaskedForeground, mask);
 
   [[newTabButton_ cell] setImage:Overlay(foreground, normal, 1.0)
                   forButtonState:image_button_cell::kDefaultState];
@@ -2282,8 +2330,11 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   // IDR_THEME_TAB_BACKGROUND_INACTIVE is only used with the default theme.
   if (theme->UsingDefaultTheme()) {
     const CGFloat alpha = tabs::kImageNoFocusAlpha;
-    NSImage* background = ApplyMask(
-        theme->GetNSImageNamed(IDR_THEME_TAB_BACKGROUND_INACTIVE), mask);
+    NSImage* unmasked_background = TileHorizontally(

[sail, 2013/08/22 18:23:01]

camel case?

+        rb.GetNativeImageNamed(IDR_THEME_TAB_BACKGROUND_INACTIVE).ToNSImage(),

[sail, 2013/08/22 18:23:01]

Why get this from the resource bungle but IDR_THEME_TAB_BACKGROUND from
ThemeService?

[pkotwicz, 2013/10/25 05:15:34]

IDR_THEME_TAB_BACKGROUND_INACTIVE is not a themeable image. I should change the
resource name so that it does not have _THEME_ to make this more obvious.

+        yPositionAlignWithTabStrip,
+        [mask size]);
+    NSImage* background = ApplyMask(unmasked_background, mask);
     [[newTabButton_ cell] setImage:Overlay(background, normal, alpha)
                     forButtonState:image_button_cell::kDefaultStateBackground];
     [[newTabButton_ cell] setImage:Overlay(background, hover, alpha)