Refactor [FramedBrowserWindow drawWindowThemeInDirtyRect:] to use themeImagePositionForAlignment:

Additional refactor necessary to fix the misaligned tab background images. (https://codereview.chromium.org/18486007/)

- Make [FramedBrowserWindow drawWindowThemeInDirtyRect:] use themeImagePositionForAlignment:
- Rename themePatternPhaseForAlignment: to themeImagePositionForAlignment:. This allows  themeImagePositionForAlignment: to be used for positioning IDR_THEME_FRAME_OVERLAY. The renaming makes it clear that there is a single correct return value for themeImagePositionForAlignment:. Phase on the other hand is periodic.
(This refactoring happens to make IDR_THEME_FRAME_OVERLAY properly line up with IDR_THEME_FRAME. See http://www.corp.google.com/~pkotwicz/theme_testing/theme_overlay.crx)
- Swap setPatternPhase with cr_setPatternPhase. That makes the theme images line up consistently with or without --use-core-animation in presentation mode
- Split [BrowserWindowUtils themeImagePositionFor:] into [BrowserWindowUtils themeImagePositionFor:] and [BrowserWindowUtils themeImagePositionInTabStripCoords:]. This will allow the new tab button to use [BrowserWindowUtils themeImagePositionInTabStripCoords:] in https://codereview.chromium.org/18486007/

BUG=176857
TEST=None
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=217608

[pkotwicz, 2013-07-21 23:10:29]

Nico, can you please take a look?

The main reason for this change is to get [BrowserWindowUtils
themeImagePositionFor:THEME_IMAGE_ALIGN_WITH_FRAME] to return a correct value
when in presentation mode. (Sorry that I missed that on the first go around)
Separately, I do not really like the behavior of theme images in presentation
mode, but will wait till a UX designer bugs me to change that.

I have also udpated https://codereview.chromium.org/18486007/ to use this CL so
there should hopefully not be any more intermediate CLs

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/fr...
File chrome/browser/ui/cocoa/framed_browser_window.mm (right):

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/fr...
chrome/browser/ui/cocoa/framed_browser_window.mm:451: NSView* frameView =
[[[view window] contentView] superview];
I assume that we probably want to move the logic below to
themeImagePositionForAlignment: However I do not have a retina mac to figure out
if we have a problems with the toolbar image etc.

[Nico, 2013-07-22 04:18:01]

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/fr...
File chrome/browser/ui/cocoa/framed_browser_window.mm (right):

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/fr...
chrome/browser/ui/cocoa/framed_browser_window.mm:451: NSView* frameView =
[[[view window] contentView] superview];
On 2013/07/21 23:10:29, pkotwicz wrote:
> I assume that we probably want to move the logic below to
> themeImagePositionForAlignment: However I do not have a retina mac to figure
out
> if we have a problems with the toolbar image etc.

You can run retina mode on regular macs:
http://developer.apple.com/library/mac/#documentation/GraphicsAnimation/Conce...

(open the "To enable high-resolution display modes" section)

[pkotwicz, 2013-07-22 21:23:30]

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/fr...
File chrome/browser/ui/cocoa/framed_browser_window.mm (right):

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/fr...
chrome/browser/ui/cocoa/framed_browser_window.mm:451: NSView* frameView =
[[[view window] contentView] superview];
I did not notice anything wiggling with the toolbar images in retina (2x) mode.
So we do not need to generalize.
The code below seems to have been added to support non integer scale factors.
https://codereview.chromium.org/879003

I have not checked fractional scale factors. That seems to be for another day.
http://devimages.apple.com/maintenance

[pkotwicz, 2013-07-24 01:35:19]

Ping!

[Nico, 2013-07-24 03:26:52]

Sorry, missed this today :-/ Ping tomorrow 11am PST if I haven't looked by
then.


On Tue, Jul 23, 2013 at 6:35 PM, <pkotwicz@chromium.org> wrote:

> Ping!
>
>
https://codereview.chromium.**org/19918002/<https://codereview.chromium.org/1...
>

[Nico, 2013-07-24 20:35:10]

CL description: have a first short overview line before listing all the
individual changes.

Looking at the code now…

[pkotwicz, 2013-07-25 01:34:48]

Did you forget to publish the comments?

[Nico, 2013-07-30 00:13:51]

I apologize for the atrocious turnaround time.

Code lgtm as far as I can see.

Please check in hidpi, and with CoreAnimation enabled on about:flags, and check
with --use-core-animation (theme offsets were wrong in that mode before. I think
this change shouldn't break things, but who knows).

+rohitrao, rsesek fti  refullscreen, +sail, ccameron, avi fti re theming

[Nico, 2013-07-30 00:14:05]

On 2013/07/30 00:13:51, Nico wrote:
> I apologize for the atrocious turnaround time.
> 
> Code lgtm as far as I can see.
> 
> Please check in hidpi, and with CoreAnimation enabled on about:flags, and
check
> with --use-core-animation (theme offsets were wrong in that mode before. I
think
> this change shouldn't break things, but who knows).
> 
> +rohitrao, rsesek fti  refullscreen, +sail, ccameron, avi fti re theming

(CL description still needs the tweak I asked for last time.)

[Robert Sesek, 2013-07-30 14:25:24]

Mac FS stuff looks generally okay. Some nits.

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/br...
File chrome/browser/ui/cocoa/browser_window_utils.h (right):

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/br...
chrome/browser/ui/cocoa/browser_window_utils.h:45: + (NSPoint)
themeImagePositionFor:(NSView*)windowView
nit: no space after )

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/br...
chrome/browser/ui/cocoa/browser_window_utils.h:53: + (NSPoint)
themeImagePositionInTabStripCoords:(NSView*)tabStripView
nit: same

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/br...
File chrome/browser/ui/cocoa/browser_window_utils.mm (right):

https://codereview.chromium.org/19918002/diff/4001/chrome/browser/ui/cocoa/br...
chrome/browser/ui/cocoa/browser_window_utils.mm:181: + (NSPoint)
themeImagePositionInTabStripCoords:(NSView*)tabStripView
nit: no space after )

[commit-bot: I haz the power, 2013-08-04 15:07:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19918002/20001

[commit-bot: I haz the power, 2013-08-04 15:19:12]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-08-14 19:26:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19918002/48001

[commit-bot: I haz the power, 2013-08-14 19:30:47]

Change committed as 217608