[Blink>Media] Moving autoplay logic to AutoplayPolicy

third_party/WebKit/Source/core/html/media/AutoplayPolicy.h

+// Copyright 2017 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.
+
+#ifndef AutoplayPolicy_h
+#define AutoplayPolicy_h
+
+#include "bindings/core/v8/Nullable.h"
+#include "core/dom/ExceptionCode.h"
+#include "platform/heap/Handle.h"
+
+namespace blink {
+
+class AutoplayUmaHelper;
+class Document;
+class ElementVisibilityObserver;
+class HTMLMediaElement;
+
+class AutoplayPolicy final : public GarbageCollected<AutoplayPolicy> {
+ public:
+  AutoplayPolicy(HTMLMediaElement*);

[mlamouri (slow - plz ping), 2017/04/19 14:09:55]

explicit?

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Done.

+
+  AutoplayUmaHelper& GetAutoplayUmaHelper() { return *autoplay_uma_helper_; }

[mlamouri (slow - plz ping), 2017/04/19 14:09:55]

Doesn't seem to be called at all

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Done.

+
+  void VideoWillBeDrawnToCanvas() const;
+
+  // Called when the media element is moved to a new document.
+  void DidMoveToNewDocument(Document& old_document);
+
+  // Returns whether the media element is eligible to autoplay muted.
+  bool IsEligibleForAutoplayMuted() const;

[mlamouri (slow - plz ping), 2017/04/19 14:09:56]

It doesn't seem to be called outside of `AutoplayPolicy`.

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Moved to private.

+
+  // Start/stop autoplaying the video element whenever its visible.
+  void StartAutoplayMutedWhenVisible();
+  void StopAutoplayMutedWhenVisible();

[mlamouri (slow - plz ping), 2017/04/19 14:09:55]

StartAutoplayMutedWhenVisible() isn't called outside of `AutoplayPolicy`.
Can you add a TODO in my name to hide StopAutoplayMutedWhenVisible from
HTMLMediaElement? I have an idea in how to do it but let's not increase the
complexity of this CL :)

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Ah, yes. Added comments above, and moved Start...() to private.

+
+  // Request autoplay by attribute. This method will check the autoplay
+  // restrictions and record metrics. This method can only be called once per
+  // time the readyState changes to HAVE_ENOUGH_DATA.
+  bool RequestAutoplayByAttribute();
+
+  // Request the playback via play() method. This method will check the autoplay
+  // restrictions and record metrics. This method can only be called once
+  // per call of play().
+  Nullable<ExceptionCode> RequestPlay();
+
+  // Returns whether an umute action should pause an autoplaying element. The
+  // method will check autoplay restrictions and record metrics. This method can
+  // only be called once per call of setMuted().
+  bool RequestAutoplayUnmute();
+
+  bool IsAutoplayingMuted();

[mlamouri (slow - plz ping), 2017/04/19 14:09:56]

Could this be `const`?

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Done.

+
+  // Return true if and only if a user gesture is required to unlock this
+  // media element for unrestricted autoplay/script control.  Don't confuse
+  // this with isGestureNeededForPlayback().  The latter is usually what one
+  // should use, if checking to see if an action is allowed.
+  bool IsLockedPendingUserGesture() const;

[mlamouri (slow - plz ping), 2017/04/19 14:09:56]

I can't see this being called outside of this class.

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:46]

Moved to private

+
+  bool IsLockedPendingUserGestureIfCrossOriginExperimentEnabled() const;

[mlamouri (slow - plz ping), 2017/04/19 14:09:55]

ditto

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Moved to private

+
+  // Unlock user gesture if a user gesture can be utilized.
+  void TryUnlockingUserGesture();
+
+  // If the user gesture is required, then this will remove it.  Note that
+  // one should not generally call this method directly; use the one on
+  // m_helper and give it a reason.
+  void UnlockUserGesture();
+
+  // Return true if and only if a user gesture is requried for playback.  Even
+  // if isLockedPendingUserGesture() return true, this might return false if
+  // the requirement is currently overridden.  This does not check if a user
+  // gesture is currently being processed.
+  bool IsGestureNeededForPlayback() const;
+
+  bool IsGestureNeededForPlaybackIfCrossOriginExperimentEnabled() const;

[mlamouri (slow - plz ping), 2017/04/19 14:09:55]

This doesn't seem to be called by anything outside of `AutoplayPolicy`. Does it
have to be public?

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Moved to private

+
+  bool IsGestureNeededForPlaybackIfPendingUserGestureIsLocked() const;

[mlamouri (slow - plz ping), 2017/04/19 14:09:56]

Same here

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Moved to private

+
+  // Return true if and only if the settings allow autoplay of media on this
+  // frame.
+  bool IsAutoplayAllowedPerSettings() const;

[mlamouri (slow - plz ping), 2017/04/19 14:09:55]

and here

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

This one is used by AutoplayUmaHelper :)

+
+  // Called when the video visibility changes while autoplaying muted, will
+  // pause the video when invisible and resume the video when visible.
+  void OnVisibilityChangedForAutoplay(bool is_visible);

[mlamouri (slow - plz ping), 2017/04/19 14:09:56]

and here :)

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Moved to private

+
+  DECLARE_VIRTUAL_TRACE();
+
+ private:
+  friend class AutoplayUmaHelperTest;
+
+  bool ShouldAutoplay();
+
+  bool locked_pending_user_gesture_;
+  bool locked_pending_user_gesture_if_cross_origin_experiment_enabled_;

[mlamouri (slow - plz ping), 2017/04/19 14:09:56]

Maybe these bool should be initialised here?

[Zhiqiang Zhang (Slow), 2017/04/20 10:30:45]

Done. Not sure if it matters as we initialize then in the ctor anyway.

+
+  Member<HTMLMediaElement> element_;
+  Member<ElementVisibilityObserver> autoplay_visibility_observer_;
+
+  Member<AutoplayUmaHelper> autoplay_uma_helper_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoplayPolicy);
+};
+
+}  // namespace blink
+
+#endif  // AutoplayPolicy_h