[Mac] Anchor profile switcher to left side of avatar button in RTL

[Mac] Anchor profile switcher to left side of avatar button in RTL

BUG=647204, 648559
Committed: https://crrev.com/667b4c49949a49eb917a742fe629d7131ee03225
Cr-Commit-Position: refs/heads/master@{#432485}

[lgrey, 2016-11-10 21:52:57]

lgrey@chromium.org changed reviewers:
+ ellyjones@chromium.org, thakis@chromium.org

[lgrey, 2016-11-10 21:52:58]

PTAL :) Elly for RTL, Nico for OWNERS

[Elly Fong-Jones, 2016-11-11 13:16:11]

lgtm

[Nico, 2016-11-15 16:32:51]

lgtm, but:

https://codereview.chromium.org/2490323002/diff/1/chrome/browser/ui/cocoa/pro...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/2490323002/diff/1/chrome/browser/ui/cocoa/pro...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1372: ?
info_bubble::kAlignLeftEdgeToAnchorEdge
Hm, I wonder if the bubble's API should be "kAlignBegin/EndEdge" instead of
"Left/RightEdge" and then all the clients would pass what they want and the
bubble could have a single of these conditionals internally, instead of us
adding a conditional at each use (and then having a bug each time we forget).

[lgrey, 2016-11-15 17:41:37]

https://codereview.chromium.org/2490323002/diff/1/chrome/browser/ui/cocoa/pro...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/2490323002/diff/1/chrome/browser/ui/cocoa/pro...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1372: ?
info_bubble::kAlignLeftEdgeToAnchorEdge
On 2016/11/15 16:32:51, Nico wrote:
> Hm, I wonder if the bubble's API should be "kAlignBegin/EndEdge" instead of
> "Left/RightEdge" and then all the clients would pass what they want and the
> bubble could have a single of these conditionals internally, instead of us
> adding a conditional at each use (and then having a bug each time we forget).

Done. This makes the zoom bubble look worse under the experimental RTL flags,
but that's the least of Omnibox's problem under the flag and will be fine once I
get there.

[Nico, 2016-11-15 18:25:22]

lgtm, thanks!

https://codereview.chromium.org/2490323002/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/base_bubble_controller.mm (right):

https://codereview.chromium.org/2490323002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/base_bubble_controller.mm:440: origin.x -= isRTL ? 0 :
NSWidth([window frame]);
nit: rather than use a ternary with 0, do
  if (!isRTL)
    origin.x -= NSWidth([window frame]);

https://codereview.chromium.org/2490323002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/base_bubble_controller.mm:444: origin.x -= isRTL ?
NSWidth([window frame]) : 0;
and here
  if (isRTL)
    ...

[lgrey, 2016-11-15 19:06:35]

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

[commit-bot: I haz the power, 2016-11-15 19:07:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lgrey, 2016-11-15 19:07:34]

Thanks!

https://codereview.chromium.org/2490323002/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/base_bubble_controller.mm (right):

https://codereview.chromium.org/2490323002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/base_bubble_controller.mm:440: origin.x -= isRTL ? 0 :
NSWidth([window frame]);
On 2016/11/15 18:25:22, Nico wrote:
> nit: rather than use a ternary with 0, do
>   if (!isRTL)
>     origin.x -= NSWidth([window frame]);

Done.

https://codereview.chromium.org/2490323002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/base_bubble_controller.mm:444: origin.x -= isRTL ?
NSWidth([window frame]) : 0;
On 2016/11/15 18:25:22, Nico wrote:
> and here
>   if (isRTL)
>     ...

Done.

[commit-bot: I haz the power, 2016-11-15 19:27:41]

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

[commit-bot: I haz the power, 2016-11-15 19:27:41]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[lgrey, 2016-11-15 20:19:45]

The CQ bit was checked by lgrey@chromium.org

[lgrey, 2016-11-15 20:19:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
ellyjones@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2490323002/#ps40001
(title: "Review comments")

[commit-bot: I haz the power, 2016-11-15 20:20:44]

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, 2016-11-15 20:38:27]

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

[commit-bot: I haz the power, 2016-11-15 20:38:28]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[lgrey, 2016-11-16 14:52:19]

The CQ bit was checked by lgrey@chromium.org

[commit-bot: I haz the power, 2016-11-16 14:52:59]

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, 2016-11-16 14:59:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-16 15:03:37]

Description was changed from

==========
[Mac] Anchor profile switcher to left side of avatar button in RTL

BUG=647204,648559
==========

to

==========
[Mac] Anchor profile switcher to left side of avatar button in RTL

BUG=647204,648559

Committed: https://crrev.com/667b4c49949a49eb917a742fe629d7131ee03225
Cr-Commit-Position: refs/heads/master@{#432485}
==========

[commit-bot: I haz the power, 2016-11-16 15:03:39]

Patchset 3 (id:??) landed as
https://crrev.com/667b4c49949a49eb917a742fe629d7131ee03225
Cr-Commit-Position: refs/heads/master@{#432485}