Mac: Fix theme image drawing when building with >=10.9 SDK.

Mac: Fix theme image drawing when building with >=10.9 SDK.

When linking with >=10.9 SDK, NSThemeFrame will have wantsLayer=YES and
wantsUpdateLayer=YES, resulting in the swizzled drawRect to be ineffective
(updateLayer is called instead of drawRect).
This patch moves the drawing of the theme image behind the tab strip area
to use a small subview instead. This subview is positioned at the top of the
window, behind its sibling views. The view will redraw when the window's
activation state changes because the theme image may be different.

NativeAppWindowCocoa is another user of the swizzled drawRect, so I'm not
removing the swizzling yet in this CL.

BUG=417872
Committed: https://crrev.com/b8bc771d55ccc9090f7e9b625a344b7377666720
Cr-Commit-Position: refs/heads/master@{#297498}

[Andre, 2014-09-26 21:50:12]

Patchset #1 (id:1) has been deleted

[Andre, 2014-09-26 23:54:26]

andresantoso@chromium.org changed reviewers:
+ erikchen@chromium.org

[Andre, 2014-09-26 23:54:27]

Erik, please review.

[erikchen, 2014-09-27 00:23:33]

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/framed_browser_window.h (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/framed_browser_window.h:22: const CGFloat
kBrowserFrameViewPaintHeight = 60.0;
My personal preference is to extern constants in header files. I couldn't find
any reference to this in chromium or google style guides.

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm:29: NSRect strokeRect
= NSInsetRect(roundedRect, -lineWidth/2, lineWidth/2);
I see that this logic is just copied from framed_browser_window, but why would
we ever want to align on the half pixel?

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_window_controller.mm (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_window_controller.mm:74:
tabStripBackgroundView_.reset([[TabStripBackgroundView alloc]
When the tab strip gets moved to another window (e.g. immersive fullscreen),
won't this view have to be moved as well?

[Andre, 2014-09-27 18:17:02]

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/framed_browser_window.h (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/framed_browser_window.h:22: const CGFloat
kBrowserFrameViewPaintHeight = 60.0;
On 2014/09/27 00:23:33, erikchen wrote:
> My personal preference is to extern constants in header files. I couldn't find
> any reference to this in chromium or google style guides.

I see the advantage of that, but in this case I think it's good to match the
style of the other constants
a few lines above.

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm:29: NSRect strokeRect
= NSInsetRect(roundedRect, -lineWidth/2, lineWidth/2);
On 2014/09/27 00:23:33, erikchen wrote:
> I see that this logic is just copied from framed_browser_window, but why would
> we ever want to align on the half pixel? 

A path's stroke is drawn right on the line instead of inside or outside the
line.
So in this case where the line is a round integer, a 1px width stroke will be
drawn 0.5px inside and 0.5 outside the path.
The vertical lines are moved 0.5px inside so that the top line gets a full
pixel. The horizontal lines are moved 0.5px outside so that it's not visible,
but to still get the rounded corners to get a border.

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_window_controller.mm (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_window_controller.mm:74:
tabStripBackgroundView_.reset([[TabStripBackgroundView alloc]
On 2014/09/27 00:23:33, erikchen wrote:
> When the tab strip gets moved to another window (e.g. immersive fullscreen),
> won't this view have to be moved as well?

I don't think this view needs to be moved.
FullscreenWindow is not a subclass of FramedBrowserWindow, so it was not
getting this custom drawing before. Instead it is using FloatingBarBackingView
to draw the background.

[Andre, 2014-09-27 18:47:00]

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm (right):

https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm:29: NSRect strokeRect
= NSInsetRect(roundedRect, -lineWidth/2, lineWidth/2);
On 2014/09/27 18:17:02, Andre wrote:
> On 2014/09/27 00:23:33, erikchen wrote:
> > I see that this logic is just copied from framed_browser_window, but why
would
> > we ever want to align on the half pixel? 
> 
> A path's stroke is drawn right on the line instead of inside or outside the
> line.
> So in this case where the line is a round integer, a 1px width stroke will be
> drawn 0.5px inside and 0.5 outside the path.
> The vertical lines are moved 0.5px inside so that the top line gets a full
> pixel. The horizontal lines are moved 0.5px outside so that it's not visible,
> but to still get the rounded corners to get a border.

Uploaded a new patch to add this as comment.

[Andre, 2014-09-28 02:11:37]

Patchset #6 (id:120001) has been deleted

[Andre, 2014-09-29 16:13:28]

On 2014/09/27 18:47:00, Andre wrote:
>
https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
> File chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm (right):
> 
>
https://codereview.chromium.org/611453004/diff/80001/chrome/browser/ui/cocoa/...
> chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm:29: NSRect
strokeRect
> = NSInsetRect(roundedRect, -lineWidth/2, lineWidth/2);
> On 2014/09/27 18:17:02, Andre wrote:
> > On 2014/09/27 00:23:33, erikchen wrote:
> > > I see that this logic is just copied from framed_browser_window, but why
> would
> > > we ever want to align on the half pixel? 
> > 
> > A path's stroke is drawn right on the line instead of inside or outside the
> > line.
> > So in this case where the line is a round integer, a 1px width stroke will
be
> > drawn 0.5px inside and 0.5 outside the path.
> > The vertical lines are moved 0.5px inside so that the top line gets a full
> > pixel. The horizontal lines are moved 0.5px outside so that it's not
visible,
> > but to still get the rounded corners to get a border.
> 
> Uploaded a new patch to add this as comment.

Patch #6 has an update to make browser_tests to pass.

[Andre, 2014-09-29 20:34:32]

andresantoso@chromium.org changed reviewers:
+ ccameron@chromium.org

[erikchen, 2014-09-29 23:16:36]

lgtm

[Andre, 2014-09-29 23:17:29]

andresantoso@chromium.org changed reviewers:
+ avi@chromium.org

[Andre, 2014-09-29 23:17:30]

Avi, please review.

[erikchen, 2014-09-29 23:23:32]

https://codereview.chromium.org/611453004/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/tabs/tab_window_controller.mm (right):

https://codereview.chromium.org/611453004/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/tabs/tab_window_controller.mm:317:
relativeTo:tabStripBackgroundView_];
This doesn't look right.
If tabStripBackgroundView_ is nil, then the contentView is brought to the top
instead.

This logic also relies on the assumption that the contentView and
tabStirpBackgroundView_ are in the same window, which doesn't look true in
browser_window_controller_private.mm:378

[Andre, 2014-09-30 03:56:25]

Patchset #7 (id:160001) has been deleted

[Andre, 2014-09-30 03:57:38]

https://codereview.chromium.org/611453004/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/tabs/tab_window_controller.mm (right):

https://codereview.chromium.org/611453004/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/tabs/tab_window_controller.mm:317:
relativeTo:tabStripBackgroundView_];
On 2014/09/29 23:23:32, erikchen wrote:
> This doesn't look right.
> If tabStripBackgroundView_ is nil, then the contentView is brought to the top
> instead.
> 
> This logic also relies on the assumption that the contentView and
> tabStirpBackgroundView_ are in the same window, which doesn't look true in
> browser_window_controller_private.mm:378

Good call.
Please see latest patch for the fix.

[Avi (use Gerrit), 2014-09-30 15:49:33]

lgtm

https://codereview.chromium.org/611453004/diff/180001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/framed_browser_window.h (right):

https://codereview.chromium.org/611453004/diff/180001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/framed_browser_window.h:22: const CGFloat
kBrowserFrameViewPaintHeight = 60.0;
Comment?

[Andre, 2014-09-30 16:47:39]

https://codereview.chromium.org/611453004/diff/180001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/framed_browser_window.h (right):

https://codereview.chromium.org/611453004/diff/180001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/framed_browser_window.h:22: const CGFloat
kBrowserFrameViewPaintHeight = 60.0;
On 2014/09/30 15:49:32, Avi wrote:
> Comment?

Done.

[Andre, 2014-09-30 19:51:32]

Erik, latest patch looks good to you?

[erikchen, 2014-09-30 19:59:12]

lgtm

[Andre, 2014-09-30 20:50:39]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2014-09-30 20:51:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/611453004/200001

[commit-bot: I haz the power, 2014-09-30 21:01:24]

Committed patchset #8 (id:200001) as dccb855d2738f7ce970aad3cd1dec5c128501a08

[commit-bot: I haz the power, 2014-09-30 21:02:13]

Patchset 8 (id:??) landed as
https://crrev.com/b8bc771d55ccc9090f7e9b625a344b7377666720
Cr-Commit-Position: refs/heads/master@{#297498}

[commit-bot: I haz the power, 2014-09-30 21:02:29]

Patchset 8 (id:??) landed as
https://crrev.com/b8bc771d55ccc9090f7e9b625a344b7377666720
Cr-Commit-Position: refs/heads/master@{#297498}