[Media controls] Fix fullscreen orientation lock vs manifest lock

MediaControlsOrientationLockDelegate was releasing its fullscreen screen
orientation lock when the device had been rotated to the match the
orientation of the video (and other conditions were met).

This was intended to have no immediate effect, and simply allow the user
to exit fullscreen by rotating the device away.

However it turns out that unlocking actually locks to the "default"
orientation, and e.g. for an add-to-homescreen webapp that has set its
manifest to portrait orientation, the "default" orientation would be
portrait. So unlocking when in landscape would immediately rotate to
portrait and exit fullscreen!

This patch fixes that - instead of unlocking when the device orientation
matches the video orientation, it locks to the "any" orientation which
enables accelerometer-based screen orientation auto-rotation (overriding
manifest orientations). A full unlock to "default" orientation happens
later when fullscreen is exited.

(To understand this, it's helpful to know that Chrome doesn't have a
stack of orientation locks, it's simply last write wins, with the
exception that ScreenOrientationProvider restores the manifest
orientation when unlocking == locking to "default").

Bug: 740205
Change-Id: I590b33fa45a1ab6cc44ae3b49f983ae57d41a682
Reviewed-on: https://chromium-review.googlesource.com/577849Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: John Mellor <johnme@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488229}

third_party/WebKit/Source/modules/media_controls/MediaControlsOrientationLockDelegate.cpp

@@ -90,7 +90,7 @@ class DummyScreenOrientationCallback : public WebLockOrientationCallback {
 
 }  // anonymous namespace
 
-constexpr TimeDelta MediaControlsOrientationLockDelegate::kUnlockDelay;
+constexpr TimeDelta MediaControlsOrientationLockDelegate::kLockToAnyDelay;
 
 MediaControlsOrientationLockDelegate::MediaControlsOrientationLockDelegate(
     HTMLVideoElement& video)
@@ -163,6 +163,17 @@ void MediaControlsOrientationLockDelegate::MaybeLockOrientation() {
   MaybeListenToDeviceOrientation();
 }
 
+void MediaControlsOrientationLockDelegate::ChangeLockToAnyOrientation() {
+  // Must already be locked.
+  DCHECK_EQ(state_, State::kMaybeLockedFullscreen);
+  DCHECK_NE(locked_orientation_, kWebScreenOrientationLockDefault);
+
+  locked_orientation_ = kWebScreenOrientationLockAny;
+  ScreenOrientationController::From(*GetDocument().GetFrame())
+      ->lock(locked_orientation_,
+             WTF::WrapUnique(new DummyScreenOrientationCallback));
+}
+
 void MediaControlsOrientationLockDelegate::MaybeUnlockOrientation() {
   DCHECK(state_ != State::kPendingFullscreen);
 
@@ -180,7 +191,7 @@ void MediaControlsOrientationLockDelegate::MaybeUnlockOrientation() {
   ScreenOrientationController::From(*GetDocument().GetFrame())->unlock();
   locked_orientation_ = kWebScreenOrientationLockDefault /* unlocked */;
 
-  unlock_task_.Cancel();
+  lock_to_any_task_.Cancel();
 }
 
 void MediaControlsOrientationLockDelegate::MaybeListenToDeviceOrientation() {
@@ -274,7 +285,8 @@ void MediaControlsOrientationLockDelegate::handleEvent(
   }
 
   if (event->type() == EventTypeNames::deviceorientation) {
-    MaybeUnlockIfDeviceOrientationMatchesVideo(ToDeviceOrientationEvent(event));
+    MaybeLockToAnyIfDeviceOrientationMatchesVideo(
+        ToDeviceOrientationEvent(event));
 
     return;
   }
@@ -401,7 +413,8 @@ MediaControlsOrientationLockDelegate::ComputeDeviceOrientation(
 }
 
 void MediaControlsOrientationLockDelegate::
-    MaybeUnlockIfDeviceOrientationMatchesVideo(DeviceOrientationEvent* event) {
+    MaybeLockToAnyIfDeviceOrientationMatchesVideo(
+        DeviceOrientationEvent* event) {
   DCHECK_EQ(state_, State::kMaybeLockedFullscreen);
   DCHECK(locked_orientation_ == kWebScreenOrientationLockPortrait ||
          locked_orientation_ == kWebScreenOrientationLockLandscape);
@@ -418,13 +431,13 @@ void MediaControlsOrientationLockDelegate::
     return;
 
   // Job done: the user rotated their device to match the orientation of the
-  // video that we locked to, so now we can stop listening and unlock.
+  // video that we locked to, so now we can stop listening.
   if (LocalDOMWindow* dom_window = GetDocument().domWindow()) {
     dom_window->removeEventListener(EventTypeNames::deviceorientation, this,
                                     false);
   }
-  // Delay before unlocking, as a workaround for the case where the device is
-  // initially portrait-primary, then fullscreen orientation lock locks it to
+  // Delay before changing lock, as a workaround for the case where the device
+  // is initially portrait-primary, then fullscreen orientation lock locks it to
   // landscape and the screen orientation changes to landscape-primary, but the
   // user actually rotates the device to landscape-secondary. In that case, if
   // this delegate unlocks the orientation before Android has detected the
@@ -432,14 +445,27 @@ void MediaControlsOrientationLockDelegate::
   // Android would change the screen orientation back to portrait-primary. This
   // is avoided by delaying unlocking long enough to ensure that Android has
   // detected the orientation change.
-  unlock_task_ =
+  lock_to_any_task_ =
       TaskRunnerHelper::Get(TaskType::kMediaElementEvent, &GetDocument())
           ->PostDelayedCancellableTask(
               BLINK_FROM_HERE,
-              WTF::Bind(
-                  &MediaControlsOrientationLockDelegate::MaybeUnlockOrientation,
-                  WrapPersistent(this)),
-              kUnlockDelay);
+              // Conceptually, this callback will unlock the screen orientation,
+              // so that the user can now rotate their device to the opposite
+              // orientation in order to exit fullscreen. But unlocking
+              // corresponds to kWebScreenOrientationLockDefault, which is
+              // sometimes a specific orientation. For example in a webapp added
+              // to homescreen that has set its orientation to portrait using
+              // the manifest, unlocking actually locks to portrait, which would
+              // immediately exit fullscreen if we're watching a landscape video
+              // in landscape orientation! So instead, this locks to
+              // kWebScreenOrientationLockAny which will auto-rotate according
+              // to the accelerometer, and only exit fullscreen once the user
+              // actually rotates their device. We only fully unlock to
+              // kWebScreenOrientationLockDefault once fullscreen is exited.
+              WTF::Bind(&MediaControlsOrientationLockDelegate::
+                            ChangeLockToAnyOrientation,
+                        WrapPersistent(this)),
+              kLockToAnyDelay);
 }
 
 DEFINE_TRACE(MediaControlsOrientationLockDelegate) {

third_party/WebKit/Source/modules/media_controls/MediaControlsOrientationLockDelegate.h

@@ -103,6 +103,12 @@ class MediaControlsOrientationLockDelegate final : public EventListener {
   // otherwise.
   void MaybeLockOrientation();
 
+  // Changes a previously locked screen orientation to instead be locked to
+  // the "any" orientation that allows accelerometer-based rotation. This is
+  // not the same as unlocking (which returns to the "default" orientation,
+  // which may in fact be more restrictive).
+  void ChangeLockToAnyOrientation();
+
   // Unlocks the screen orientation if the screen orientation was previously
   // locked.
   void MaybeUnlockOrientation();
@@ -113,12 +119,12 @@ class MediaControlsOrientationLockDelegate final : public EventListener {
   MODULES_EXPORT DeviceOrientationType
   ComputeDeviceOrientation(DeviceOrientationData*) const;
 
-  void MaybeUnlockIfDeviceOrientationMatchesVideo(DeviceOrientationEvent*);
+  void MaybeLockToAnyIfDeviceOrientationMatchesVideo(DeviceOrientationEvent*);
 
-  // Delay before unlocking - see `MaybeUnlockIfDeviceOrientationMatchesVideo`.
+  // Delay before `MaybeLockToAnyIfDeviceOrientationMatchesVideo` changes lock.
   // Emprically, 200ms is too short, but 250ms avoids glitches. 500ms gives us
   // a 2x margin in case the device is running slow, without being noticeable.
-  MODULES_EXPORT static constexpr TimeDelta kUnlockDelay =
+  MODULES_EXPORT static constexpr TimeDelta kLockToAnyDelay =
       TimeDelta::FromMilliseconds(500);
 
   // Current state of the object. See comment at the top of the file for a
@@ -129,7 +135,7 @@ class MediaControlsOrientationLockDelegate final : public EventListener {
   WebScreenOrientationLockType locked_orientation_ =
       kWebScreenOrientationLockDefault /* unlocked */;
 
-  TaskHandle unlock_task_;
+  TaskHandle lock_to_any_task_;
 
   device::mojom::blink::ScreenOrientationListenerPtr monitor_;
 

third_party/WebKit/Source/modules/media_controls/MediaControlsOrientationLockDelegateTest.cpp

@@ -131,7 +131,7 @@ class MediaControlsOrientationLockDelegateTest : public ::testing::Test {
       MediaControlsOrientationLockDelegate::DeviceOrientationType;
 
   static constexpr TimeDelta GetUnlockDelay() {
-    return MediaControlsOrientationLockDelegate::kUnlockDelay;
+    return MediaControlsOrientationLockDelegate::kLockToAnyDelay;
   }
 
   void SetUp() override {
@@ -795,9 +795,10 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   RotateDeviceTo(90 /* landscape primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
 
-  // MediaControlsOrientationLockDelegate should unlock orientation.
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  // MediaControlsOrientationLockDelegate should lock to "any" orientation.
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
 }
 
 TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
@@ -856,9 +857,10 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   RotateDeviceTo(90 /* landscape primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
 
-  // MediaControlsOrientationLockDelegate should unlock orientation.
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  // MediaControlsOrientationLockDelegate should lock to "any" orientation.
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
   EXPECT_TRUE(Video().IsFullscreen());
 }
 
@@ -960,9 +962,10 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   RotateDeviceTo(90 /* landscape primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
 
-  // MediaControlsOrientationLockDelegate should unlock orientation.
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  // MediaControlsOrientationLockDelegate should lock to "any" orientation.
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
 }
 
 TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
@@ -974,13 +977,14 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   InitVideo(640, 480);
   SetIsAutoRotateEnabledByUser(true);
 
-  // Initially fullscreen, unlocked orientation.
+  // Initially fullscreen, locked to "any" orientation.
   SimulateEnterFullscreen();
   RotateDeviceTo(90 /* landscape primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
   ASSERT_TRUE(Video().IsFullscreen());
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
 
   // Simulate user rotating their device to portrait triggering a screen
   // orientation change.
@@ -991,7 +995,7 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   // MediaControlsRotateToFullscreenDelegate should exit fullscreen.
   EXPECT_FALSE(Video().IsFullscreen());
 
-  // MediaControlsOrientationLockDelegate should remain unlocked.
+  // MediaControlsOrientationLockDelegate should unlock screen orientation.
   CheckStatePendingFullscreen();
   EXPECT_FALSE(DelegateWillUnlockFullscreen());
 }
@@ -1005,19 +1009,20 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   InitVideo(640, 480);
   SetIsAutoRotateEnabledByUser(true);
 
-  // Initially fullscreen, unlocked orientation.
+  // Initially fullscreen, locked to "any" orientation.
   SimulateEnterFullscreen();
   RotateDeviceTo(90 /* landscape primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
   ASSERT_TRUE(Video().IsFullscreen());
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
 
   // Simulate user clicking on media controls exit fullscreen button.
   SimulateExitFullscreen();
   EXPECT_FALSE(Video().IsFullscreen());
 
-  // MediaControlsOrientationLockDelegate should remain unlocked.
+  // MediaControlsOrientationLockDelegate should unlock screen orientation.
   CheckStatePendingFullscreen();
   EXPECT_FALSE(DelegateWillUnlockFullscreen());
 }
@@ -1258,9 +1263,10 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   RotateDeviceTo(0 /* portrait primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
 
-  // MediaControlsOrientationLockDelegate should unlock orientation.
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  // MediaControlsOrientationLockDelegate should lock to "any" orientation.
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
   EXPECT_TRUE(Video().IsFullscreen());
 
   // Simulate user rotating their device to landscape triggering a screen
@@ -1271,7 +1277,7 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   // MediaControlsRotateToFullscreenDelegate should exit fullscreen.
   EXPECT_FALSE(Video().IsFullscreen());
 
-  // MediaControlsOrientationLockDelegate should remain unlocked.
+  // MediaControlsOrientationLockDelegate should unlock screen orientation.
   CheckStatePendingFullscreen();
   EXPECT_FALSE(DelegateWillUnlockFullscreen());
 }
@@ -1309,9 +1315,10 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   RotateDeviceTo(0 /* landscape primary */);
   testing::RunDelayedTasks(GetUnlockDelay());
 
-  // MediaControlsOrientationLockDelegate should unlock orientation.
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  // MediaControlsOrientationLockDelegate should lock to "any" orientation.
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
   EXPECT_TRUE(Video().IsFullscreen());
 
   // Simulate user rotating their device to portrait triggering a screen
@@ -1322,7 +1329,7 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   // MediaControlsRotateToFullscreenDelegate should exit fullscreen.
   EXPECT_FALSE(Video().IsFullscreen());
 
-  // MediaControlsOrientationLockDelegate should remain unlocked.
+  // MediaControlsOrientationLockDelegate should unlock screen orientation.
   CheckStatePendingFullscreen();
   EXPECT_FALSE(DelegateWillUnlockFullscreen());
 }
@@ -1397,9 +1404,10 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   // Wait for the rest of the unlock delay.
   testing::RunDelayedTasks(GetUnlockDelay() - kMinUnlockDelay);
 
-  // MediaControlsOrientationLockDelegate should now have unlocked.
-  CheckStatePendingFullscreen();
-  EXPECT_FALSE(DelegateWillUnlockFullscreen());
+  // MediaControlsOrientationLockDelegate should've locked to "any" orientation.
+  CheckStateMaybeLockedFullscreen();
+  EXPECT_EQ(kWebScreenOrientationLockAny, DelegateOrientationLock());
+  EXPECT_TRUE(DelegateWillUnlockFullscreen());
 }
 
 }  // namespace blink