Media: lock orientation when <video> goes fullscreen.

third_party/WebKit/Source/core/html/shadow/MediaControlsOrientationLockDelegate.cpp

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "MediaControlsOrientationLockDelegate.h"
+
+#include "core/events/Event.h"
+#include "core/frame/FrameHost.h"
+#include "core/frame/ScreenOrientationController.h"
+#include "core/html/HTMLVideoElement.h"
+#include "core/page/ChromeClient.h"
+#include "public/platform/WebScreenInfo.h"
+#include "public/platform/modules/screen_orientation/WebLockOrientationCallback.h"
+
+namespace blink {
+
+namespace {
+
+// WebLockOrientationCallback implementation that will not react to a success
+// nor a failure.
+class DummyScreenOrientationCallback : public WebLockOrientationCallback {
+ public:
+  void onSuccess() override {}
+  void onError(WebLockOrientationError) override {}
+};
+
+}  // anonymous namespace
+
+MediaControlsOrientationLockDelegate::MediaControlsOrientationLockDelegate(
+    MediaControls* mediaControls)
+    : EventListener(CPPEventListenerType), m_mediaControls(mediaControls) {
+  DCHECK(m_mediaControls->mediaElement().isHTMLVideoElement());
+
+  document().addEventListener(EventTypeNames::fullscreenchange, this, false);
+  document().addEventListener(EventTypeNames::webkitfullscreenchange, this,

[foolip, 2016/12/12 23:16:05]

Because webkitfullscreenchange targets and element and propagation could be
stopped so that this code is never run, this isn't entirely reliable. If one
might be left with screen orientation stuck because of this, I'd suggest instead
using the MediaControls::enteredFullscreen() and ::exitedFullscreen().

That is, unless CPPEventListenerType always runs before any scripts anyway, but
that would mean that the whole event phase is run multiple times, so I suspect
not.

[mlamouri (slow - plz ping), 2016/12/13 10:25:17]

Is your point that HTMLMediaElement is always notified when entering/leaving
fullscreen and doesn't rely on events? Shouldn't we aim to change this? What
about handling the event in the capture phase?

[foolip, 2016/12/13 10:35:39]

It would be nice to not have a separate mechanism, but I'm not sure how at this
point. One way would be to always fire the fullscreenchange event, but there's
some risk to that and my plan was to always fire either prefixed or unprefixed
events, where webkitfullscreenchange is problematic.

Actually, now that I think about it, if you just listen for the
webkitfullscreenchange event on the video itself, then maybe it's impossible for
scripts to interfere. The video is the event target, so maybe an internal event
listener in the capturing phase would work. Can you verify?

[mlamouri (slow - plz ping), 2016/12/13 21:15:16]

Listening to `fullscreenchange` on document and `webkitfullscreenchange` on the
video on the capture phase should make sure we receive the events as long as the
controls are created before any page event listener on the capture phase calling
`stopImmediatePropagation()` is called. Unlikely.

I hope we could find a safer way to handle events from internal code though.

+                              false);
+
+  videoElement().addEventListener(EventTypeNames::loadedmetadata, this, false);

[Zhiqiang Zhang (Slow), 2016/12/13 11:53:28]

nit: Wouldn't this listen to loadedmetadata too often?

We could add/remove this listener on demand I think, but the logic might be a
bit complicated.

[mlamouri (slow - plz ping), 2016/12/13 21:15:16]

loadedmetadata is generally called once. The overhead is very low and it will
avoid bugs.

+}
+
+bool MediaControlsOrientationLockDelegate::operator==(
+    const EventListener& other) const {
+  return this == &other;
+}
+
+void MediaControlsOrientationLockDelegate::maybeLockOrientation() {
+  DCHECK(m_state != State::MaybeLockedFullscreen);
+
+  if (videoElement().getReadyState() == HTMLMediaElement::kHaveNothing) {
+    m_state = State::PendingMetadata;
+    return;
+  }
+
+  m_state = State::MaybeLockedFullscreen;
+
+  auto controller = ScreenOrientationController::from(*document().frame());

[foolip, 2016/12/12 23:16:05]

Is it impossible for document().frame() to be null here? Because this is only
called from handleEvent()? It is null checked inside computeOrientationLock()
right after, which seems suspicious.

[mlamouri (slow - plz ping), 2016/12/13 21:15:16]

Fixed.

+  if (controller->maybeHasActiveLock())
+    return;
+
+  controller->lock(computeOrientationLock(),
+                   WTF::wrapUnique(new DummyScreenOrientationCallback));
+  m_shouldUnlockOrientation = true;
+}
+
+void MediaControlsOrientationLockDelegate::maybeUnlockOrientation() {
+  DCHECK(m_state != State::PendingFullscreen);
+
+  m_state = State::PendingFullscreen;
+
+  if (!m_shouldUnlockOrientation)
+    return;
+
+  ScreenOrientationController::from(*document().frame())->unlock();
+  m_shouldUnlockOrientation = false;
+}
+
+HTMLVideoElement& MediaControlsOrientationLockDelegate::videoElement() const {
+  return toHTMLVideoElement(m_mediaControls->mediaElement());
+}
+
+Document& MediaControlsOrientationLockDelegate::document() const {
+  return videoElement().document();
+}
+
+void MediaControlsOrientationLockDelegate::handleEvent(
+    ExecutionContext* executionContext,
+    Event* event) {
+  if (event->type() == EventTypeNames::fullscreenchange ||
+      event->type() == EventTypeNames::webkitfullscreenchange) {
+    if (videoElement().isFullscreen()) {
+      if (m_state == State::PendingFullscreen)
+        maybeLockOrientation();
+    } else {
+      if (m_state != State::PendingFullscreen)
+        maybeUnlockOrientation();
+    }
+
+    return;
+  }
+
+  if (event->type() == EventTypeNames::loadedmetadata) {
+    if (m_state == State::PendingMetadata)
+      maybeLockOrientation();
+
+    return;
+  }
+
+  NOTREACHED();
+}
+
+WebScreenOrientationLockType
+MediaControlsOrientationLockDelegate::computeOrientationLock() const {

[DaleCurtis, 2016/12/12 23:25:13]

Doesn't this need to know the current rotation? There's no accessor for it
currently, but one could easily be plumbed to HTMLMediaElement.

https://cs.chromium.org/chromium/src/media/blink/webmediaplayer_impl.cc?l=1154

[mlamouri (slow - plz ping), 2016/12/13 10:25:17]

I was planning to handle this as a follow-up. I have a couple of questions about
`video_rotation`.

+  const unsigned width = videoElement().videoWidth();

[foolip, 2016/12/12 23:16:05]

DCHECK something about readyState?

[mlamouri (slow - plz ping), 2016/12/13 21:15:16]

Done.

+  const unsigned height = videoElement().videoHeight();
+
+  if (width > height)
+    return WebScreenOrientationLockLandscape;
+
+  if (height > width)
+    return WebScreenOrientationLockPortrait;
+
+  // For square videos, try to lock to the current screen orientation for
+  // consistency. Use WebScreenOrientationLockLandscape as a fallback value.
+  // TODO(mlamouri): we could improve this by having direct access to
+  // `window.screen.orientation.type`.
+  Frame* frame = document().frame();
+  if (!frame || !frame->host())
+    return WebScreenOrientationLockLandscape;
+
+  switch (frame->host()->chromeClient().screenInfo().orientationType) {

[foolip, 2016/12/12 23:16:05]

If you use frame->chromeClient() you can skip the FrameHost.h include. It'll
depend on EmptyChromeClients returning WebScreenOrientationUndefined and that
mapping to WebScreenOrientationLockLandscape, maybe that's OK.

[mlamouri (slow - plz ping), 2016/12/13 21:15:16]

Done.

+    case WebScreenOrientationPortraitPrimary:
+    case WebScreenOrientationPortraitSecondary:
+      return WebScreenOrientationLockPortrait;
+    case WebScreenOrientationLandscapePrimary:
+    case WebScreenOrientationLandscapeSecondary:
+      return WebScreenOrientationLockLandscape;
+    case WebScreenOrientationUndefined:
+      return WebScreenOrientationLockLandscape;
+  }
+}
+
+DEFINE_TRACE(MediaControlsOrientationLockDelegate) {
+  EventListener::trace(visitor);
+  visitor->trace(m_mediaControls);
+}
+
+}  // namespace blink