mac: Make the avatar button layer backed.

mac: Make the avatar button layer backed.

Since it's added to the NSThemeFrame directly as a subview, it also needs to be
removed/added during the fullscreen transition animation.

BUG=411770
Committed: https://crrev.com/2682f5f04b173ab1d7b8ded664adfd0dad5d2aca
Cr-Commit-Position: refs/heads/master@{#294726}

[erikchen, 2014-09-11 01:18:23]

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

[erikchen, 2014-09-11 01:18:24]

andresantoso: Please review.

[Andre, 2014-09-11 03:07:57]

lgtm

https://codereview.chromium.org/566473002/diff/1/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/566473002/diff/1/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:992: NSView*
superview = [view superview];
Can we skip this and do an early return if [superview layer] is non-nil?
So that this code automatically gets skipped in the future when the NSThemeFrame
gets a layer, for example when we start linking with >=10.9 SDK.

[erikchen, 2014-09-11 17:30:00]

https://codereview.chromium.org/566473002/diff/1/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/566473002/diff/1/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:992: NSView*
superview = [view superview];
On 2014/09/11 03:07:57, Andre wrote:
> Can we skip this and do an early return if [superview layer] is non-nil?
> So that this code automatically gets skipped in the future when the
NSThemeFrame
> gets a layer, for example when we start linking with >=10.9 SDK.

Sure.

[erikchen, 2014-09-11 17:31:10]

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

[erikchen, 2014-09-11 17:31:11]

avi: Please review.

[Avi (use Gerrit), 2014-09-11 18:26:50]

https://codereview.chromium.org/566473002/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.h (right):

https://codereview.chromium.org/566473002/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.h:141: -
(void)reAddViewToSuperview:(NSView*)view;
First, this comment is not helpful; it merely restates the name of the method.

But much more importantly, name this function after _what it does_, not how it
does it. Why would someone want to call this method? What does it accomplish?
The fact that it does so by removing and re-adding the view is an implementation
detail.

I'm seriously staring at this code and it makes no sense.

[erikchen, 2014-09-12 00:31:53]

Patchset #3 (id:40001) has been deleted

[erikchen, 2014-09-12 00:32:51]

avi: PTAL

https://codereview.chromium.org/566473002/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.h (right):

https://codereview.chromium.org/566473002/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.h:141: -
(void)reAddViewToSuperview:(NSView*)view;
On 2014/09/11 18:26:49, Avi wrote:
> First, this comment is not helpful; it merely restates the name of the method.
> 
> But much more importantly, name this function after _what it does_, not how it
> does it. Why would someone want to call this method? What does it accomplish?
> The fact that it does so by removing and re-adding the view is an
implementation
> detail.
> 
> I'm seriously staring at this code and it makes no sense.

I updated the comment and the name of the method as per your suggestions.

[Avi (use Gerrit), 2014-09-12 05:58:17]

LGTM with renaming.

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.h (right):

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.h:146: -
(void)toggleLayerOrdering:(NSView*)view;
"toggle" means to switch between two settings. This isn't toggling.

-updateLayerOrdering: perhaps? (Straight from your comment.)

And yes, I'm cringing.

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:996: // Get the
current position of the view
end the comment with a . as complete sentences are required for comments

[erikchen, 2014-09-12 17:29:55]

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.h (right):

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.h:146: -
(void)toggleLayerOrdering:(NSView*)view;
On 2014/09/12 05:58:17, Avi wrote:
> "toggle" means to switch between two settings. This isn't toggling.
> 
> -updateLayerOrdering: perhaps? (Straight from your comment.)
> 
> And yes, I'm cringing.

Done.

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/566473002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:996: // Get the
current position of the view
On 2014/09/12 05:58:17, Avi wrote:
> end the comment with a . as complete sentences are required for comments

Done.

[erikchen, 2014-09-12 17:30:07]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-09-12 17:31:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/566473002/80001

[commit-bot: I haz the power, 2014-09-12 17:45:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-12 17:45:22]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[erikchen, 2014-09-13 01:20:19]

The CQ bit was checked by erikchen@chromium.org

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

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/566473002/80001

[commit-bot: I haz the power, 2014-09-13 02:15:37]

Committed patchset #4 (id:80001) as 05029d1adced59252cc949e6e72dd0d7122088f1

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

Patchset 4 (id:??) landed as
https://crrev.com/2682f5f04b173ab1d7b8ded664adfd0dad5d2aca
Cr-Commit-Position: refs/heads/master@{#294726}