Fix the misalignment on CrOS of the tab background images

The offset of the tab strip wrt to the top of the frame is different depending on whether the browser is maximized or not on CrOS. Currently we generate IDR_THEME_TAB_BACKGROUND upon theme install by combining an HSL shifted version "theme_frame" with the "theme_tab_background" provided by the theme author. This process assumes that the offset between the frame and the tabstrip is constant which it is not on CrOS.
This CL splits up the HSL shifted "theme_frame" (now IDR_THEME_TAB_BACKGROUND) and the "theme_tab_background" (now IDR_THEME_TAB_BACKGROUND_OVERLAY) provided by the theme author into two separate resources and combines them at paint time.

BUG=176857, 166955
TEST=Manual, see bug

[pkotwicz, 2013-07-05 19:31:53]

Scott, can you please take a look at the changes in chrome/browser/ui/views
I will make similar changes to GTK and cocoa once you are happy with the views
changes.

In terms of performance, this CL increases the average runtime of
PaintInactiveTabBackgroundUsingResourceIds() from .5ms to .6ms on the pixel
Chromebook.
I know that this is due in part to creating more intermediate bitmaps.

I have considered caching the result of TabUtil::CreateTiledInactiveBackground()
but have not found a convenient way of doing so. In particular the Profile is
not easily accessible from either the tab strip or the tab.

TabUtil is not in chrome/browser/themes because I will need to implement
something similar to TabUtil for GTK and cocoa which uses the native image types
to that platform. Thankfully those implementations will be much simpler.

https://codereview.chromium.org/18486007/diff/1001/chrome/browser/ui/views/ta...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/18486007/diff/1001/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab.cc:1251:
theme_provider->HasCustomImage(IDR_THEME_FRAME) ||
Checking for IDR_THEME_FRAME here is more correct. I need to fix the behavior
when win8::IsSingleWindowMetroMode() is true. A number of themes look broken on
Win8 in metro mode (with and without this CL)

[sky, 2013-07-08 14:43:20]

How about some screen shots of the changes too?

https://codereview.chromium.org/18486007/diff/6001/chrome/browser/themes/brow...
File chrome/browser/themes/browser_theme_pack.cc (right):

https://codereview.chromium.org/18486007/diff/6001/chrome/browser/themes/brow...
chrome/browser/themes/browser_theme_pack.cc:451: // |destination_prs_id|. This
method is a no-op if there is no
nit: remove 'there is'

https://codereview.chromium.org/18486007/diff/6001/chrome/browser/ui/views/ta...
File chrome/browser/ui/views/tabs/tab_util.h (right):

https://codereview.chromium.org/18486007/diff/6001/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab_util.h:16: class TabUtil {
This name is too generic and encourages making this a dumping ground for any
random stuff. Maybe TabPaintUtils. The file should be named similarly.

https://codereview.chromium.org/18486007/diff/6001/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab_util.h:33: };
private: DISALLOW_IMPLICIT_CONSTRUCTORS, or create a namespace.

[pkotwicz, 2013-07-08 23:20:39]

Scott, can you please take another look?

[sky, 2013-07-09 00:24:57]

LGTM, but wait for Elliot to review browser_theme_pack.

https://codereview.chromium.org/18486007/diff/23001/chrome/browser/ui/views/t...
File chrome/browser/ui/views/tabs/tab_paint_util.cc (right):

https://codereview.chromium.org/18486007/diff/23001/chrome/browser/ui/views/t...
chrome/browser/ui/views/tabs/tab_paint_util.cc:59: int src_x_;
Can't this and src_y_ be const too?

https://codereview.chromium.org/18486007/diff/23001/chrome/browser/ui/views/t...
File chrome/browser/ui/views/tabs/tab_paint_util.h (right):

https://codereview.chromium.org/18486007/diff/23001/chrome/browser/ui/views/t...
chrome/browser/ui/views/tabs/tab_paint_util.h:18: class TabPaintUtil {
TabPaintUtils.

[pkotwicz, 2013-07-09 02:22:56]

erg@ you can safely wait to review this CL till I expand this CL to deal with
cocoa and GTK.

[pkotwicz, 2013-07-15 01:50:09]

erg@ can you please take a look at the changes to chrome/browser/themes and
chrome/browser/ui/gtk?

sail@ can you please take a look at the changes in chrome/browser/ui/cocoa?
This CL depends on https://codereview.chromium.org/13389002/ which I will be
committing later in the day

[pkotwicz, 2013-07-15 01:50:31]

Adding sail@ properly as a reviewer

[Elliot Glaysher, 2013-07-15 19:26:43]

lgtm

[pkotwicz, 2013-07-17 17:26:11]

sail@ seems OOO. Nico, can you please take a look?

[Nico, 2013-07-17 17:34:46]

On 2013/07/17 17:26:11, pkotwicz wrote:
> sail@ seems OOO. Nico, can you please take a look?

At everything?

[pkotwicz, 2013-07-17 17:42:46]

Sorry, I should have been more specific. Nico, can you please take a look at the
changes in chrome/browser/ui/cocoa?

[Nico, 2013-07-17 17:47:47]

Ok, will do.

re cl description: Add "Fix the misalignment on CrOS of the tab background
images" as first line to the actual description (the rietveld title won't make
it into the commit). Wrap description at 80 cols for git users.

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

I'm a bit surprised that a cros bugfix requires so many changes in cocoa tab
strip drawing.

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm (right):

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2281: NSImage*
ntp_bg_active = theme->GetNSImageNamed(
useCamelCapsInObjCCode

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2292: int
distance_between_frame_and_button = NSHeight([tabStripView_ bounds]) -
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.

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2298: //
BrowserWindowUtils::themePatternPhaseForAlignment
can they be shared somehow instead?

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_view.mm (left):

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_view.mm:368: NSRectFill(dirtyRect);
This was called for non-selected tabs with alpha > 0 previously, and now isn't.
Is that intentional?

Taking a step back, most of the changes in here seem to be unrelated to this CL,
maybe you can pull them into a separate CL that just refactors things?

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_view.mm (right):

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_view.mm:305: - (void)
drawFillForActiveTab:(NSRect)dirtyRect {
no space after (void)

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/themed_window.h (right):

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/themed_window.h:27: THEME_PATTERN_ALIGN_WITH_TAB_STRIP
Add comments explaining what these do and when to use which.

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

sail@, can you please take a look at the chrome/browser/ui/cocoa/ changes?
thakis@ is OOO I think. Sorry that this CL is taking forever.
Since the last patch set, I have landed
https://chromiumcodereview.appspot.com/19576007 and
https://chromiumcodereview.appspot.com/19918002 which I believe addresses most
of thakis@ concerns. (There is at the very least a whole lot less cocoa code in
this CL).

This CL also has the side effect of making
https://x20web.corp.google.com/~pkotwicz/theme_testing/index.html line up
properly on Mac.

I have a new asset for IDR_THEME_TAB_BACKGROUND for the default theme which is
shifted so that it aligns with the frame at
https://codereview.chromium.org/19676005/ (This is consistent with other
platforms)

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm (right):

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2292: int
distance_between_frame_and_button = NSHeight([tabStripView_ bounds]) -
I now compute this value in NewTabButtonThemeBackgroundYPosition()

https://codereview.chromium.org/18486007/diff/32001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2298: //
BrowserWindowUtils::themePatternPhaseForAlignment
These values are now shared thanks to https://codereview.chromium.org/19918002/

[sail, 2013-08-22 18:23:00]

LGTM, just nits. I didn't follow all the previous discussion so I just looked at
the code to make sure it didn't do anything awful.

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm (right):

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:265: NSImage*
TileHorizontally(NSImage* image, CGFloat yInset, NSSize size) {
Could use better variable names. For example:
   size -> dest_size
  yInset -> dest_y_inset or src_y_inset

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:266: return
[CreateImageWithSize(size, ^(NSSize size) {
shouldn't use the same variable name as the outer scope

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:267: CGFloat width =
size.width;
Same problem with variable names. Maybe remove all together?

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:296: CGFloat
yPositionInTabstrip =
use c style variable names? more above and below

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2333: NSImage*
unmasked_background = TileHorizontally(
camel case?

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2334:
rb.GetNativeImageNamed(IDR_THEME_TAB_BACKGROUND_INACTIVE).ToNSImage(),
Why get this from the resource bungle but IDR_THEME_TAB_BACKGROUND from
ThemeService?

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_view.mm (right):

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_view.mm:308: themeImagePositionForAlignment:
THEME_IMAGE_ALIGN_WITH_TAB_STRIP];
no space after colon

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_view.mm:350: themeImagePositionForAlignment:
THEME_IMAGE_ALIGN_WITH_FRAME];
no space after colon

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_view.mm:373: themeImagePositionForAlignment:
THEME_IMAGE_ALIGN_WITH_TAB_STRIP];
no space after colon

[pkotwicz, 2013-10-25 05:15:33]

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
File chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm (right):

https://codereview.chromium.org/18486007/diff/60001/chrome/browser/ui/cocoa/t...
chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm:2334:
rb.GetNativeImageNamed(IDR_THEME_TAB_BACKGROUND_INACTIVE).ToNSImage(),
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.