cocoa: flip window controls and profile switcher for RTL

cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732
Committed: https://crrev.com/2ec7f4ea9e7c48cd3a34bc1c17585a02f709c22e
Cr-Commit-Position: refs/heads/master@{#417339}

[Elly Fong-Jones, 2016-09-06 15:27:02]

Description was changed from

==========
cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732
==========

to

==========
cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732
==========

[Elly Fong-Jones, 2016-09-06 15:27:02]

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

[Elly Fong-Jones, 2016-09-06 15:27:10]

avi: ptal? :)

[Avi (use Gerrit), 2016-09-06 15:38:00]

Yay for RTL! הגיע הזמן!

https://codereview.chromium.org/2313723004/diff/1/chrome/browser/ui/cocoa/bro...
File chrome/browser/ui/cocoa/browser_window_layout.mm (right):

https://codereview.chromium.org/2313723004/diff/1/chrome/browser/ui/cocoa/bro...
chrome/browser/ui/cocoa/browser_window_layout.mm:261: layout.rightIndent = width
- maxX;
Some thoughts.

I would have thought that the code would be:
---
CGFloat leftIndent = ...
CGFloat rightIndent = ...
if (base::i18n::IsRTL())
  std::swap(leftIndent, rightIndent);
---

You take the old fullscreen button size into account only in non-RTL layout. Are
we ignoring that for RTL? Are we assuming that we're not going into RTL mode on
those older systems?

[Elly Fong-Jones, 2016-09-06 17:40:36]

On 2016/09/06 15:38:00, Avi wrote:
> Yay for RTL! הגיע הזמן!
> 
>
https://codereview.chromium.org/2313723004/diff/1/chrome/browser/ui/cocoa/bro...
> File chrome/browser/ui/cocoa/browser_window_layout.mm (right):
> 
>
https://codereview.chromium.org/2313723004/diff/1/chrome/browser/ui/cocoa/bro...
> chrome/browser/ui/cocoa/browser_window_layout.mm:261: layout.rightIndent =
width
> - maxX;
> Some thoughts.
> 
> I would have thought that the code would be:
> ---
> CGFloat leftIndent = ...
> CGFloat rightIndent = ...
> if (base::i18n::IsRTL())
>   std::swap(leftIndent, rightIndent);
> ---
> 
> You take the old fullscreen button size into account only in non-RTL layout.
Are
> we ignoring that for RTL? Are we assuming that we're not going into RTL mode
on
> those older systems?

Okay, I think I addressed those.

There's another issue I still need to figure out: the position of the profile
switcher seems to be set relative to the right edge of the window, not the left
edge, so resizing it in RTL has the effect of moving the profile switcher
around. Not quite sure how to fix that yet since I'm not sure how it's
positioned.

[Avi (use Gerrit), 2016-09-06 20:40:09]

lgtm

I like it.

Have you tested this on RTL systems that have a visible fullscreen widget?

[Elly Fong-Jones, 2016-09-08 13:56:47]

On 2016/09/06 20:40:09, Avi (OOO 8-12 September) wrote:
> lgtm
> 
> I like it.
> 
> Have you tested this on RTL systems that have a visible fullscreen widget?

I added some hackery to make the tab strip do RTL as well.

I'm seriously considering shoving all this stuff behind a flag, since right now
it makes the RTL experience much less consistent. Not sure. What do you think?
Flag, or just land it?

[Avi (use Gerrit), 2016-09-08 14:45:06]

On 2016/09/08 13:56:47, Elly Jones wrote:
> On 2016/09/06 20:40:09, Avi (OOO 8-12 September) wrote:
> > lgtm
> > 
> > I like it.
> > 
> > Have you tested this on RTL systems that have a visible fullscreen widget?
> 
> I added some hackery to make the tab strip do RTL as well.
> 
> I'm seriously considering shoving all this stuff behind a flag, since right
now
> it makes the RTL experience much less consistent. Not sure. What do you think?
> Flag, or just land it?

I agree. Go lightweight, though. There are only a few other things, right?

[Elly Fong-Jones, 2016-09-08 17:25:11]

On 2016/09/08 14:45:06, Avi (OOO 8-12 September) wrote:
> On 2016/09/08 13:56:47, Elly Jones wrote:
> > On 2016/09/06 20:40:09, Avi (OOO 8-12 September) wrote:
> > > lgtm
> > > 
> > > I like it.
> > > 
> > > Have you tested this on RTL systems that have a visible fullscreen widget?
> > 
> > I added some hackery to make the tab strip do RTL as well.
> > 
> > I'm seriously considering shoving all this stuff behind a flag, since right
> now
> > it makes the RTL experience much less consistent. Not sure. What do you
think?
> > Flag, or just land it?
> 
> I agree. Go lightweight, though. There are only a few other things, right?

Yep. I added a feature for it.

[Elly Fong-Jones, 2016-09-08 17:25:18]

The CQ bit was checked by ellyjones@chromium.org

[Elly Fong-Jones, 2016-09-08 17:25:18]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org
Link to the patchset: https://codereview.chromium.org/2313723004/#ps60001
(title: "add ExperimentalMacRTL feature")

[commit-bot: I haz the power, 2016-09-08 17:25:42]

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-09-08 18:07:11]

Description was changed from

==========
cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732
==========

to

==========
cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732
==========

[commit-bot: I haz the power, 2016-09-08 18:07:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-08 18:09:02]

Description was changed from

==========
cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732
==========

to

==========
cocoa: flip window controls and profile switcher for RTL

This change:
1) Reflects the window controls horizontally for RTL
2) Reflects the profile switcher horizontally for RTL
3) Adjusts the left and right indents for the tabstrip itself to account for
   those reflections

BUG=642732

Committed: https://crrev.com/2ec7f4ea9e7c48cd3a34bc1c17585a02f709c22e
Cr-Commit-Position: refs/heads/master@{#417339}
==========

[commit-bot: I haz the power, 2016-09-08 18:09:03]

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