Fix the misalignment on CrOS of the tab background images

chrome/browser/ui/cocoa/tabs/tab_strip_controller.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/tabs/tab_strip_controller.h"
 
 #import <QuartzCore/QuartzCore.h>
 
 #include <cmath>
 #include <limits>
@@ skipping 173 matching lines @@
     [result addRepresentation:bmpImageRep];
   }
   [NSGraphicsContext restoreGraphicsState];
 
   return result.release();
 }
 
 // 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) {
+  DCHECK_EQ([image size].width, [mask size].width);
+  DCHECK_EQ([image size].height, [mask size].height);
+
   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;
-      }
-
+      [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];
 }
 
 // Creates a modified favicon used for the recording case. The mask is used for
 // making part of the favicon transparent. (The part where the recording dot
 // later is drawn.)
 NSImage* CreateMaskedFaviconForRecording(NSImage* image,
                                          NSImage* mask,
@@ skipping 37 matching lines @@
                  fromRect:NSMakeRect(0, 0, width, height)
                 operation:NSCompositeCopy
                  fraction:1.0];
       [overlay drawAtPoint:NSZeroPoint
                   fromRect:NSMakeRect(0, 0, width, height)
                  operation:NSCompositeSourceOver
                   fraction: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)
 - (void)addSubviewToPermanentList:(NSView*)aView;
 - (void)regenerateSubviewList;
 - (NSInteger)indexForContentsView:(NSView*)view;
 - (NSImageView*)iconImageViewForContents:(content::WebContents*)contents;
 - (void)updateFaviconForContents:(content::WebContents*)contents
                          atIndex:(NSInteger)modelIndex;
 - (void)layoutTabsWithAnimation:(BOOL)animate
@@ skipping 1973 matching lines @@
   ThemeService *theme =
       static_cast<ThemeService*>([[tabStripView_ window] themeProvider]);
   if (!theme)
     return;
 
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   NSImage* mask = rb.GetNativeImageNamed(IDR_NEWTAB_BUTTON_MASK).ToNSImage();
   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* 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);
 
-  NSImage* foreground = ApplyMask(
-      theme->GetNSImageNamed(IDR_THEME_TAB_BACKGROUND, true), mask);
+  // 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];
   [[newTabButton_ cell] setImage:Overlay(foreground, hover, 1.0)
                   forButtonState:image_button_cell::kHoverState];
   [[newTabButton_ cell] setImage:Overlay(foreground, pressed, 1.0)
                     forButtonState:image_button_cell::kPressedState];
 
   // 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)
                     forButtonState:image_button_cell::kHoverStateBackground];
   } else {
     [[newTabButton_ cell] setImage:nil
                     forButtonState:image_button_cell::kDefaultStateBackground];
     [[newTabButton_ cell] setImage:nil
                     forButtonState:image_button_cell::kHoverStateBackground];
   }
 }
 
 @end
 
 NSView* GetSheetParentViewForWebContents(WebContents* web_contents) {
   // View hierarchy of the contents view:
   // NSView  -- switchView, same for all tabs
   // +- NSView  -- TabContentsController's view
   //    +- TabContentsViewCocoa
   //
   // Changing it? Do not forget to modify
   // -[TabStripController swapInTabAtIndex:] too.
   return [web_contents->GetView()->GetNativeView() superview];
 }