User orientation lock

User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation will be used.
   - If the active appilcation's orientation is specified, the user locked orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next steps:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.
  - Add portrait/landscape lock icon and make UI consistent with Android rotation lock UI.

BUG=683456
TEST=manual, unitt tests

Review-Url: https://codereview.chromium.org/2757493002
Cr-Commit-Position: refs/heads/master@{#459291}
Committed: https://chromium.googlesource.com/chromium/src/+/a5176fd790a6ccc800274a4135dbdc7476deac88

[oshima, 2017-03-20 23:36:01]

Description was changed from

==========
lock

BUG=
==========

to

==========
ScreenOrientation v2

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the appilcation's orientation is any, the user locked orientation will
be used.
   - If the appilcation's orientation is specified, the user locked orientation
will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

[oshima, 2017-03-20 23:36:01]

oshima@chromium.org changed reviewers:
+ jonross@chromium.org

[oshima, 2017-03-20 23:42:51]

Description was changed from

==========
ScreenOrientation v2

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the appilcation's orientation is any, the user locked orientation will
be used.
   - If the appilcation's orientation is specified, the user locked orientation
will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

to

==========
ScreenOrientation v2

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

[oshima, 2017-03-20 23:43:12]

Patchset #1 (id:1) has been deleted

[oshima, 2017-03-20 23:43:15]

Patchset #1 (id:20001) has been deleted

[oshima, 2017-03-20 23:43:20]

Patchset #1 (id:40001) has been deleted

[oshima, 2017-03-20 23:43:22]

Patchset #2 (id:80001) has been deleted

[oshima, 2017-03-20 23:43:41]

Patchset #2 (id:100001) has been deleted

[oshima, 2017-03-20 23:43:44]

Patchset #1 (id:60001) has been deleted

[oshima, 2017-03-20 23:44:38]

Patchset #1 (id:120001) has been deleted

[oshima, 2017-03-20 23:51:10]

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

[oshima, 2017-03-20 23:51:18]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2017-03-20 23:52:40]

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

[commit-bot: I haz the power, 2017-03-20 23:53:06]

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-03-21 00:19:07]

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

[commit-bot: I haz the power, 2017-03-21 00:19:08]

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

[oshima, 2017-03-21 02:30:37]

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

[commit-bot: I haz the power, 2017-03-21 02:31:12]

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-03-21 02:36:22]

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

[commit-bot: I haz the power, 2017-03-21 02:36:23]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2017-03-21 02:41:55]

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

[oshima, 2017-03-21 02:42:06]

Patchset #3 (id:180001) has been deleted

[oshima, 2017-03-21 02:42:10]

Patchset #2 (id:160001) has been deleted

[oshima, 2017-03-21 02:42:14]

Patchset #1 (id:140001) has been deleted

[oshima, 2017-03-21 02:42:40]

Description was changed from

==========
ScreenOrientation v2

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

to

==========
ScreenOrientation v2

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

[commit-bot: I haz the power, 2017-03-21 02:42:44]

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-03-21 02:48:09]

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

[commit-bot: I haz the power, 2017-03-21 02:48:10]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2017-03-21 04:00:11]

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

[commit-bot: I haz the power, 2017-03-21 04:00:27]

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-03-21 05:02:13]

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

[commit-bot: I haz the power, 2017-03-21 05:02:14]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[oshima, 2017-03-21 07:10:09]

Patchset #1 (id:200001) has been deleted

[oshima, 2017-03-21 07:11:53]

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.h (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.h:80: void
ToggleUserRotationLock();
I'm using toggle instead of SetUserRotationLock to avoid dealing with the
condition whether or not the true->true should update the orientation. Let me
know if you prefer SetXxx.

[oshima, 2017-03-21 13:16:14]

Description was changed from

==========
ScreenOrientation v2

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

to

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

[jonross, 2017-03-21 14:56:44]

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.cc (left):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:142:
Shell::GetInstance()->display_manager()->RegisterDisplayRotationProperties(
Were you planning to land this in a follow up change?

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.cc (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:244:
SetRotationLockedInternal(true);
So currently we have been treating the settings app as a source of a user
rotation lock.

This change would bypass the updated user rotation lock logic. So they would not
be able to use the tray to toggle state.

Based on your TODO above you feel we should just disable this feature completely
in touchview? That would make sense for the internal display.

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:462: for (auto const& pair
: locking_windows_) {
The ARC++ app windows are being registered with locking_windows_?

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.h (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.h:80: void
ToggleUserRotationLock();
On 2017/03/21 07:11:53, oshima wrote:
> I'm using toggle instead of SetUserRotationLock to avoid dealing with the
> condition whether or not the true->true should update the orientation. Let me
> know if you prefer SetXxx.

Using Toggle does make the unit testing harder to follow.

For the true->true transition, do we expect that to ever occur? TrayRotationLock
currently just flips the state.

https://codereview.chromium.org/2757493002/diff/220001/ash/system/chromeos/ro...
File ash/system/chromeos/rotation/tray_rotation_lock.cc (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/system/chromeos/ro...
ash/system/chromeos/rotation/tray_rotation_lock.cc:106: void
RotationLockDefaultView::Update() {
So this might become confusing for users.

As an ARC++ app coming to the foreground and rotating the display would still
show the user rotation lock in the tray.

A precedent from Android is that the rotation lock icon shows the locked
orientation (Portrait vs Landscape)

So when an app launches that contradicts this the user can still see the
orientation they requested.

We might consider adjust for this as a followup. Though we'd need new assets.

[oshima, 2017-03-21 15:08:31]

Description was changed from

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next step:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.

BUG=683456
TEST=manual, unitt tests
==========

to

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next steps:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.
  - Add portrait/landscape lock icon to be consistent with Android rotation lock
UI.

BUG=683456
TEST=manual, unitt tests
==========

[oshima, 2017-03-21 15:10:57]

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.cc (left):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:142:
Shell::GetInstance()->display_manager()->RegisterDisplayRotationProperties(
On 2017/03/21 14:56:43, jonross wrote:
> Were you planning to land this in a follow up change?

Yes. I mentioned it in the CL description.
(note that the code was simply moved down below, so this still exists, but it
needs to be updated.)

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.cc (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:244:
SetRotationLockedInternal(true);
On 2017/03/21 14:56:43, jonross wrote:
> So currently we have been treating the settings app as a source of a user
> rotation lock.
> This change would bypass the updated user rotation lock logic. So they would
not
> be able to use the tray to toggle state.
> 
> Based on your TODO above you feel we should just disable this feature
completely
> in touchview? That would make sense for the internal display.

I'll make two changes:
- Disable the rotation change in settings as it's confusing (you shouldn't be
able to change the orientation
  when the app requested fixed orientation.
- We still can't prevent an app from calling chrome.system.display, so this will
be treated as
 a user requested rotation lock.

This will be addressed in the follow up CL.

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:462: for (auto const& pair
: locking_windows_) {
On 2017/03/21 14:56:43, jonross wrote:
> The ARC++ app windows are being registered with locking_windows_?

Yes, ARC++ apps always set the orientation.

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.h (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.h:80: void
ToggleUserRotationLock();
On 2017/03/21 14:56:43, jonross wrote:
> On 2017/03/21 07:11:53, oshima wrote:
> > I'm using toggle instead of SetUserRotationLock to avoid dealing with the
> > condition whether or not the true->true should update the orientation. Let
me
> > know if you prefer SetXxx.
> 
> Using Toggle does make the unit testing harder to follow.
> 
> For the true->true transition, do we expect that to ever occur?
TrayRotationLock
> currently just flips the state.

No it will never happen as UI is toggle. Passing bool makes the semantics
a bit vague. Should we ignore true->true (which is sufficient for production),
or should we update it to the current orientation.

It's not hard to implement, but if we do, then question is if we need to test.

https://codereview.chromium.org/2757493002/diff/220001/ash/system/chromeos/ro...
File ash/system/chromeos/rotation/tray_rotation_lock.cc (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/system/chromeos/ro...
ash/system/chromeos/rotation/tray_rotation_lock.cc:106: void
RotationLockDefaultView::Update() {
On 2017/03/21 14:56:44, jonross wrote:
> So this might become confusing for users.
> 
> As an ARC++ app coming to the foreground and rotating the display would still
> show the user rotation lock in the tray.
> 
> A precedent from Android is that the rotation lock icon shows the locked
> orientation (Portrait vs Landscape)
> 
> So when an app launches that contradicts this the user can still see the
> orientation they requested.
> 
> We might consider adjust for this as a followup. Though we'd need new assets.

Basically, we're making this consistent with Androd auto-rotate icon behavior.
Portrait/landscape icon will be added in a separate CL.

[oshima, 2017-03-21 16:15:58]

Description was changed from

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next steps:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.
  - Add portrait/landscape lock icon to be consistent with Android rotation lock
UI.

BUG=683456
TEST=manual, unitt tests
==========

to

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next steps:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.
  - Add portrait/landscape lock icon and make UI consistent with Android
rotation lock UI.

BUG=683456
TEST=manual, unitt tests
==========

[oshima, 2017-03-22 23:17:35]

ping?

[jonross, 2017-03-22 23:51:46]

On 2017/03/22 23:17:35, oshima wrote:
> ping?

Sorry I somehow missed the replies you sent yesterday.

The plans for followups all SGTM.

This change LGTM.

[jonross, 2017-03-22 23:51:56]

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.cc (left):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:142:
Shell::GetInstance()->display_manager()->RegisterDisplayRotationProperties(
On 2017/03/21 15:10:57, oshima wrote:
> On 2017/03/21 14:56:43, jonross wrote:
> > Were you planning to land this in a follow up change?
> 
> Yes. I mentioned it in the CL description.
> (note that the code was simply moved down below, so this still exists, but it
> needs to be updated.)

SGTM

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.cc (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:244:
SetRotationLockedInternal(true);
On 2017/03/21 15:10:57, oshima wrote:
> On 2017/03/21 14:56:43, jonross wrote:
> > So currently we have been treating the settings app as a source of a user
> > rotation lock.
> > This change would bypass the updated user rotation lock logic. So they would
> not
> > be able to use the tray to toggle state.
> > 
> > Based on your TODO above you feel we should just disable this feature
> completely
> > in touchview? That would make sense for the internal display.
> 
> I'll make two changes:
> - Disable the rotation change in settings as it's confusing (you shouldn't be
> able to change the orientation
>   when the app requested fixed orientation.
> - We still can't prevent an app from calling chrome.system.display, so this
will
> be treated as
>  a user requested rotation lock.
> 
> This will be addressed in the follow up CL.
> 
> 

SGTM

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.cc:462: for (auto const& pair
: locking_windows_) {
On 2017/03/21 15:10:57, oshima wrote:
> On 2017/03/21 14:56:43, jonross wrote:
> > The ARC++ app windows are being registered with locking_windows_?
> 
> Yes, ARC++ apps always set the orientation.

Good to know, I wasn't 100% which path the took. SGTM

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
File ash/display/screen_orientation_controller_chromeos.h (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/display/screen_ori...
ash/display/screen_orientation_controller_chromeos.h:80: void
ToggleUserRotationLock();
On 2017/03/21 15:10:57, oshima wrote:
> On 2017/03/21 14:56:43, jonross wrote:
> > On 2017/03/21 07:11:53, oshima wrote:
> > > I'm using toggle instead of SetUserRotationLock to avoid dealing with the
> > > condition whether or not the true->true should update the orientation. Let
> me
> > > know if you prefer SetXxx.
> > 
> > Using Toggle does make the unit testing harder to follow.
> > 
> > For the true->true transition, do we expect that to ever occur?
> TrayRotationLock
> > currently just flips the state.
> 
> No it will never happen as UI is toggle. Passing bool makes the semantics
> a bit vague. Should we ignore true->true (which is sufficient for production),
> or should we update it to the current orientation.
> 
> It's not hard to implement, but if we do, then question is if we need to test.
> 
> 
> 

Well true->true might also end up being a confusing concept for a user if we
hooked that up. So I would support ignoring it.

I'm fine with the toggle semantics.

https://codereview.chromium.org/2757493002/diff/220001/ash/system/chromeos/ro...
File ash/system/chromeos/rotation/tray_rotation_lock.cc (right):

https://codereview.chromium.org/2757493002/diff/220001/ash/system/chromeos/ro...
ash/system/chromeos/rotation/tray_rotation_lock.cc:106: void
RotationLockDefaultView::Update() {
On 2017/03/21 15:10:57, oshima wrote:
> On 2017/03/21 14:56:44, jonross wrote:
> > So this might become confusing for users.
> > 
> > As an ARC++ app coming to the foreground and rotating the display would
still
> > show the user rotation lock in the tray.
> > 
> > A precedent from Android is that the rotation lock icon shows the locked
> > orientation (Portrait vs Landscape)
> > 
> > So when an app launches that contradicts this the user can still see the
> > orientation they requested.
> > 
> > We might consider adjust for this as a followup. Though we'd need new
assets.
> 
> Basically, we're making this consistent with Androd auto-rotate icon behavior.
> Portrait/landscape icon will be added in a separate CL.

SGTM

[oshima, 2017-03-23 00:25:51]

oshima@chromium.org changed reviewers:
+ benwells@chromium.org, mlamouri@chromium.org

[oshima, 2017-03-23 00:25:51]

benwells@ -> chrome/browser/extensions
mlamouri@chromium.org -> ash/test/DEPS

[oshima, 2017-03-23 00:31:56]

Patchset #2 (id:240001) has been deleted

[benwells, 2017-03-23 04:31:05]

c/b/e lgtm

[mlamouri (slow - plz ping), 2017-03-23 19:56:18]

On 2017/03/23 at 04:31:05, benwells wrote:
> c/b/e lgtm

ash/test/deps lgtm

[oshima, 2017-03-23 20:04:35]

The CQ bit was checked by oshima@chromium.org

[oshima, 2017-03-23 20:04:37]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2017-03-23 20:07:20]

The CQ bit was checked by oshima@chromium.org

[oshima, 2017-03-23 20:07:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
jonross@chromium.org, benwells@chromium.org, mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2757493002/#ps260001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-23 20:07:48]

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-03-23 20:23:50]

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

[commit-bot: I haz the power, 2017-03-23 20:23:52]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2017-03-23 21:13:16]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2017-03-23 21:13:57]

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

[oshima, 2017-03-23 21:14:37]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2017-03-23 21:22:17]

The CQ bit was checked by oshima@chromium.org

[oshima, 2017-03-23 21:22:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
jonross@chromium.org, benwells@chromium.org, mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2757493002/#ps280001
(title: "rebase again")

[commit-bot: I haz the power, 2017-03-23 21:23:17]

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-03-24 00:09:12]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1490304137516570,
"parent_rev": "6b815d45f10ca1b8fb208647f2a0576bdf31e006", "commit_rev":
"a5176fd790a6ccc800274a4135dbdc7476deac88"}

[commit-bot: I haz the power, 2017-03-24 00:10:44]

Description was changed from

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next steps:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.
  - Add portrait/landscape lock icon and make UI consistent with Android
rotation lock UI.

BUG=683456
TEST=manual, unitt tests
==========

to

==========
User orientation lock 

* Added user locked orientation.
  Once it's locked, the orientation will be used as a preferred orientation.
   - If the active appilcation's orientation is any, the user locked orientation
will be used.
   - If the active appilcation's orientation is specified, the user locked
orientation will be ignored
     and uses the application's orientation.
* Exclude windows that does not have AppType because they do not control
orientation into account.
  For example, task manager, app list shouldn't affect orientation.
* added ScreenOrientationControllerTestApi

Next steps:
  - landscape / portrait app should lock to secondary/primary.
  - remember and restore the locked state.
  - handle rotation change from chrome.system.display api.
  - Add portrait/landscape lock icon and make UI consistent with Android
rotation lock UI.

BUG=683456
TEST=manual, unitt tests

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

[commit-bot: I haz the power, 2017-03-24 00:10:45]

Committed patchset #3 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/a5176fd790a6ccc800274a4135db...