Skip to content

Commit

Permalink
[Media Controls] Disable rotate-to-fullscren if zero Device Orientation
Browse files Browse the repository at this point in the history
Rotate-to-fullscreen was previously disabled on devices that don't
support the Device Orientation API, in
https://chromium-review.googlesource.com/645981
(48a2ccc). But it turns out there are
devices that claim to support Device Orientation but then give
device orientation values that are always fixed to zero!

This patch disables rotate-to-fullscreen on those devices too, for
the same reason that after entering fullscreen it wouldn't be possible
to rotate-to-exit-fullscreen since the
MediaControlsOrientationLockDelegate won't be able to read the Device
Orientation, which would give an inconsistent UX.

Bug: 760737
Change-Id: I79ce15bde124d5663028a3f2538f4cfe522e0517
Reviewed-on: https://chromium-review.googlesource.com/667379
Commit-Queue: John Mellor <[email protected]>
Commit-Queue: Mounir Lamouri <[email protected]>
Reviewed-by: Mounir Lamouri <[email protected]>
Cr-Commit-Position: refs/heads/master@{#502017}
  • Loading branch information
johnmellor authored and Commit Bot committed Sep 14, 2017
1 parent fa22b6c commit d172204
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,18 @@ void MediaControlsRotateToFullscreenDelegate::OnDeviceOrientationAvailable(
dom_window->removeEventListener(EventTypeNames::deviceorientation, this,
false);

// MediaControlsOrientationLockDelegate needs beta and gamma only.
// MediaControlsOrientationLockDelegate needs Device Orientation events with
// beta and gamma in order to unlock screen orientation and exit fullscreen
// when the device is rotated. Some devices cannot provide beta and/or gamma
// values and must be excluded. Unfortunately, some other devices incorrectly
// return true for both CanProvideBeta() and CanProvideGamma() but their
// Beta() and Gamma() values are permanently stuck on zero (crbug/760737); so
// we have to also exclude devices where both of these values are exactly
// zero, even though that's a valid (albeit unlikely) device orientation.
DeviceOrientationData* data = event->Orientation();
device_orientation_supported_ =
WTF::make_optional(data->CanProvideBeta() && data->CanProvideGamma());
WTF::make_optional(data->CanProvideBeta() && data->CanProvideGamma() &&
(data->Beta() != 0.0 || data->Gamma() != 0.0));
}

void MediaControlsRotateToFullscreenDelegate::OnScreenOrientationChange() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,36 @@ TEST_F(MediaControlsRotateToFullscreenDelegateTest,
EXPECT_FALSE(GetVideo().IsFullscreen());
}

TEST_F(MediaControlsRotateToFullscreenDelegateTest,
EnterFailZeroDeviceOrientation) {
// Portrait screen, landscape video.
InitScreenAndVideo(kWebScreenOrientationPortraitPrimary, WebSize(640, 480),
false /* with_device_orientation */);
EXPECT_EQ(SimpleOrientation::kPortrait, ObservedScreenOrientation());
EXPECT_EQ(SimpleOrientation::kLandscape, ComputeVideoOrientation());

// Dispatch a Device Orientation event where all values are zero, as happens
// on poorly configured devices that lack the necessary hardware to support
// the Device Orientation API, but don't properly expose that lack.
DeviceOrientationController::From(GetDocument())
.SetOverride(
DeviceOrientationData::Create(0.0 /* alpha */, 0.0 /* beta */,
0.0 /* gamma */, false /* absolute */));
testing::RunPendingTasks();

// Play video.
PlayVideo();
UpdateVisibilityObserver();

EXPECT_TRUE(ObservedVisibility());

// Rotate screen to landscape.
RotateTo(kWebScreenOrientationLandscapePrimary);

// Should not enter fullscreen since Device Orientation is not available.
EXPECT_FALSE(GetVideo().IsFullscreen());
}

TEST_F(MediaControlsRotateToFullscreenDelegateTest, EnterFailPaused) {
// Portrait screen, landscape video.
InitScreenAndVideo(kWebScreenOrientationPortraitPrimary, WebSize(640, 480));
Expand Down

0 comments on commit d172204

Please sign in to comment.