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 12 matching lines @@
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/ui/browser.h"
 #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"
 #import "chrome/browser/ui/cocoa/new_tab_button.h"
 #import "chrome/browser/ui/cocoa/tab_contents/favicon_util_mac.h"
 #import "chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_audio_indicator_view_mac.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_projecting_image_view.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.h"
@@ skipping 141 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|. |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)
 - (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* 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];
   [[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), 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)
                     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];
 }