New issue
Advanced search Search tips

Issue 735078 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

cleanup orientation lock code

Project Member Reported by osh...@chromium.org, Jun 20 2017

Issue description

When I added arc++ behavior support, I added on top of html5 based impl, then it's
evolved to mixed of two. I now have better understanding of
overall code and logic, we should cleanup and consolidate duplicated logic
into ash.

 
Components: UI>Shell>TouchView
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6b5111b94bf58436bed9adbf6249662b7784275

commit c6b5111b94bf58436bed9adbf6249662b7784275
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Mar 19 21:05:29 2018

Introduce ash::OrientationTypeLock

Remove dependency to blink in ash/
Define ash own orientation lock type so that
exo can set the orientation w/o depending on blink.

Bug:  735078 
Test: None
Change-Id: Ie11e1716cf6e657dd6899d05a2f1921cace17980
Reviewed-on: https://chromium-review.googlesource.com/967721
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544157}
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/content/display/screen_orientation_controller_chromeos_unittest.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/content/screen_orientation_delegate_chromeos.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/display/DEPS
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/display/display_manager_unittest.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/display/screen_orientation_controller_chromeos.h
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/display/screen_orientation_controller_test_api.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/display/screen_orientation_controller_test_api.h
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/metrics/pointer_metrics_recorder.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/overview/overview_window_drag_controller.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/overview/window_selector_unittest.cc
[delete] https://crrev.com/6a1e03470f144852659f994b7b6ed8b4b940f85c/ash/wm/splitview/DEPS
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/splitview/split_view_controller.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/splitview/split_view_controller.h
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/splitview/split_view_controller_unittest.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/splitview/split_view_divider.cc
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/ash/wm/splitview/split_view_divider.h
[modify] https://crrev.com/c6b5111b94bf58436bed9adbf6249662b7784275/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/850d9723dfa60cfaf49b91dfa24ffa63dafed797

commit 850d9723dfa60cfaf49b91dfa24ffa63dafed797
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Wed Mar 21 21:24:20 2018

Fold the "current" orientation logic to ash::ScreenOrientationController

This is preparation to move the orientation request protocol
to exo remote shell.

* ash's orientation logic was originally deisnged
for w3c spec, which doesn't have "current" orientation, so it was implemented in
ArcAppWindowLauncherController. I'm moving this to ash so that exo can
implement it without re-implementing it. It'll also make it easy to
test.

* This also moves the logic "apply orientation only in tablet mode" logic
 to ash. I believe this is correct even for html as
ScreenOrientationDelegateChromeos::ScreenOrientationProviderSupported()
returns true only in tablet mode.

* Updated ScreenOrientationControllerTest to switch to tablet mode to
 match the expectation above.

BUG= 735078 ,  823634 
TEST=Updated unit tests. See comment above.

Change-Id: Id26ca7e02fd8dcb5396ab05ce61a52c635f6eb54
Reviewed-on: https://chromium-review.googlesource.com/967684
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544850}
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/ash/content/display/screen_orientation_controller_chromeos_unittest.cc
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/ash/content/screen_orientation_delegate_chromeos.cc
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/ash/display/display_manager_unittest.cc
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/ash/display/screen_orientation_controller_chromeos.h
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h
[modify] https://crrev.com/850d9723dfa60cfaf49b91dfa24ffa63dafed797/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc48908438f814a8751736e43ce84504122e938f

commit dc48908438f814a8751736e43ce84504122e938f
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Wed Mar 21 22:33:41 2018

Follow up CL  for crrev.com/c/967684

TBR=afakhry@chromium.org
BUG= 735078 ,  823634 
TEST=None

Change-Id: I6351b02d21fd3d64ca0963204f22fdd97910dc4b
Reviewed-on: https://chromium-review.googlesource.com/974293
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544879}
[modify] https://crrev.com/dc48908438f814a8751736e43ce84504122e938f/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/dc48908438f814a8751736e43ce84504122e938f/ash/display/screen_orientation_controller_chromeos.h

Comment 5 by osh...@chromium.org, Mar 21 2018

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddfde3555c5151d461923bdd92f67b051b6b87c4

commit ddfde3555c5151d461923bdd92f67b051b6b87c4
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Mar 22 09:12:13 2018

Remove _chromeos from screen_orientation_controller_chromeos.{h|cc}

Done by mass-rename.sh

BUG= 735078 
TEST=None

Change-Id: I2d111218065a991817534f4cc4a3f3d974f2c8c1
Reviewed-on: https://chromium-review.googlesource.com/967049
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545019}
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/BUILD.gn
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/content/display/screen_orientation_controller_chromeos_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/content/screen_orientation_delegate_chromeos.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/display/display_manager_unittest.cc
[rename] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/display/screen_orientation_controller.cc
[rename] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/display/screen_orientation_controller.h
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/display/screen_orientation_controller_test_api.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/metrics/pointer_metrics_recorder.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/rotator/screen_rotation_animator_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/shell.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/system/rotation/rotation_lock_feature_pod_controller.h
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/system/rotation/tray_rotation_lock.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/system/rotation/tray_rotation_lock.h
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/system/rotation/tray_rotation_lock_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/system/screen_layout_observer.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/wm/splitview/split_view_controller.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/wm/splitview/split_view_controller.h
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/wm/splitview/split_view_controller_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/wm/splitview/split_view_divider.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/ash/wm/tablet_mode/tablet_mode_controller_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/chrome/browser/chromeos/display/display_prefs_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/chrome/browser/extensions/display_info_provider_chromeos.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/chrome/browser/extensions/display_info_provider_chromeos_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/ddfde3555c5151d461923bdd92f67b051b6b87c4/components/arc/rotation_lock/arc_rotation_lock_bridge.h

Sign in to add a comment