Resize avatar button as appropriate for opaque browser frame.

Resize avatar button as appropriate for opaque browser frame.

BUG=724943
Review-Url: https://codereview.chromium.org/2893343002
Cr-Commit-Position: refs/heads/master@{#473706}
Committed: https://chromium.googlesource.com/chromium/src/+/aa6669dd0b8fd25a346fdd4bf5f934e6a3be57e6

[Evan Stade, 2017-05-22 16:01:59]

estade@chromium.org changed reviewers:
+ pkasting@chromium.org

[Evan Stade, 2017-05-22 16:09:49]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 16:10:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 18:05:39]

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

[commit-bot: I haz the power, 2017-05-22 18:05:40]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-05-22 18:44:15]

LGTM

Doesn't also fix bug 722769, does it?

https://codereview.chromium.org/2893343002/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.cc (right):

https://codereview.chromium.org/2893343002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view.cc:76: // Need to
layout the root view here, too, as the avatar button may change
Nit: Not clear what "here, too," means in this context -- just remove?

https://codereview.chromium.org/2893343002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view.cc:80: // calculated
in LayoutProfileSwitcher(). Calling LayoutProfileSwitcher()
Nit: LayoutProfileSwitcher() is not a member of this class.  Mention
GlassBrowserFrameView:: specifically?

Though it worries me that you're needing to hoist code to where it will be
triggered for the opaque frame, but I don't think any of what this is talking
about applies to the opaque frame.  Maybe the comment should be written to
either add, or talk only about, the symptoms that affect both frames
(presumably, like the current bug you're dealing with)?

[Evan Stade, 2017-05-22 20:32:49]

https://codereview.chromium.org/2893343002/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.cc (right):

https://codereview.chromium.org/2893343002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view.cc:76: // Need to
layout the root view here, too, as the avatar button may change
On 2017/05/22 18:44:14, Peter Kasting wrote:
> Nit: Not clear what "here, too," means in this context -- just remove?

Done.

https://codereview.chromium.org/2893343002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view.cc:80: // calculated
in LayoutProfileSwitcher(). Calling LayoutProfileSwitcher()
On 2017/05/22 18:44:15, Peter Kasting wrote:
> Nit: LayoutProfileSwitcher() is not a member of this class.  Mention
> GlassBrowserFrameView:: specifically?
> 
> Though it worries me that you're needing to hoist code to where it will be
> triggered for the opaque frame, but I don't think any of what this is talking
> about applies to the opaque frame.  Maybe the comment should be written to
> either add, or talk only about, the symptoms that affect both frames
> (presumably, like the current bug you're dealing with)?

I replaced this comment with the one we used to have in
AvatarButtonManager::ButtonPreferredSizeChanged()

[Evan Stade, 2017-05-22 20:33:05]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2017-05-22 20:33:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2893343002/#ps20001
(title: "comment")

[commit-bot: I haz the power, 2017-05-22 20:33:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 21:37:10]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495485185770100,
"parent_rev": "bce55d7b55d0b8da8b65315b255e17ba36bef5c8", "commit_rev":
"aa6669dd0b8fd25a346fdd4bf5f934e6a3be57e6"}

[commit-bot: I haz the power, 2017-05-22 21:37:23]

Description was changed from

==========
Resize avatar button as appropriate for opaque browser frame.

BUG=724943
==========

to

==========
Resize avatar button as appropriate for opaque browser frame.

BUG=724943

Review-Url: https://codereview.chromium.org/2893343002
Cr-Commit-Position: refs/heads/master@{#473706}
Committed:
https://chromium.googlesource.com/chromium/src/+/aa6669dd0b8fd25a346fdd4bf5f9...
==========

[commit-bot: I haz the power, 2017-05-22 21:37:27]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/aa6669dd0b8fd25a346fdd4bf5f9...