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 7bbea7884ebf282543688603aa75eb567b33833b..367c7a053e725ee3f6f3a4d29b3bb98e6897fcf8 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -191,33 +191,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 +258,33 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   }) autorelease];
 }
 
+// Tiles |image| horizontally into a new NSImage of |size|. |y_inset| refers
+// to the inset from the bottom of the returned image at which |image| should
+// be painted.
+NSImage* TileHorizontally(NSImage* image, CGFloat y_inset, NSSize size) {
+  return [CreateImageWithSize(size, ^(NSSize size) {
+      CGFloat width = size.width;
+      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 - y_inset,
+                                      tileWidth,
+                                      height)
+                 operation:NSCompositeCopy
+                  fraction:1.0];
+        x += tileWidth;
+      }
+  }) autorelease];
+}
+
 }  // namespace
 
 @interface TabStripController (Private)
@@ -2268,9 +2278,41 @@ 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* foreground = ApplyMask(
-      theme->GetNSImageNamed(IDR_THEME_TAB_BACKGROUND, true), mask);
+  NSImage* ntp_bg_active = theme->GetNSImageNamed(

[Nico, 2013/07/17 18:03:04]

useCamelCapsInObjCCode

+      IDR_THEME_TAB_BACKGROUND, true);
+  NSImage* ntp_bg_active_overlay = theme->GetNSImageNamed(
+      IDR_THEME_TAB_BACKGROUND_OVERLAY, false);
+
+  // Compute the y insets so that |ntp_bg_active| lines up vertically with the
+  // frame and that |ntp_bg_active_overlay| lines up vertically with the the
+  // tab. The tab strip has not been layed out yet so
+  // themePatternPhaseForAlignment cannot be used.
+  int distance_between_tab_strip_and_button = [[self class] defaultTabHeight] -
+      NSMaxY([newTabButton_ bounds]);
+  int distance_between_frame_and_button = NSHeight([tabStripView_ bounds]) -

[Nico, 2013/07/17 18:03:04]

Can you move most of this stuff into a different method? For good readability,
ll lines in a function should have about the same level of abstraction, and this
is much lower-level than the rest of this function.

[pkotwicz, 2013/08/15 07:09:59]

I now compute this value in NewTabButtonThemeBackgroundYPosition()

+      NSMaxY([newTabButton_ bounds]);
+  // ImageButtonCell draws the image centered vertically.
+  int y_offset_in_button = roundf(
+      (NSHeight([newTabButton_ bounds]) - [mask size].height) / 2);
+  // The constants below match those used in
+  // BrowserWindowUtils::themePatternPhaseForAlignment

[Nico, 2013/07/17 18:03:04]

can they be shared somehow instead?

[pkotwicz, 2013/08/15 07:09:59]

These values are now shared thanks to https://codereview.chromium.org/19918002/

+  int y_inset_align_with_tab_strip = distance_between_tab_strip_and_button +
+      y_offset_in_button + [mask size].height + 2;
+  int y_inset_align_with_frame = distance_between_frame_and_button +
+      y_offset_in_button + [mask size].height + 3;
+
+  // 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
+  // are narrower than the new tab button for some themes.
+  NSImage* unmasked_foreground = TileHorizontally(ntp_bg_active,
+      y_inset_align_with_frame, [mask size]);
+  if (ntp_bg_active_overlay) {
+    NSImage* tiled_overlay = TileHorizontally(ntp_bg_active_overlay,
+        y_inset_align_with_tab_strip, [mask size]);
+    unmasked_foreground = Overlay(unmasked_foreground, tiled_overlay, 1.0);
+  }
+  NSImage* foreground = ApplyMask(unmasked_foreground, mask);
 
   [[newTabButton_ cell] setImage:Overlay(foreground, normal, 1.0)
                   forButtonState:image_button_cell::kDefaultState];
@@ -2282,8 +2324,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, true), mask);
+    NSImage* unmasked_background = TileHorizontally(
+        rb.GetNativeImageNamed(IDR_THEME_TAB_BACKGROUND_INACTIVE).ToNSImage(),
+        y_inset_align_with_tab_strip,
+        [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)