Deprecating AutoplayExperimentHelper

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

Index: third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
index 5aba8cc8fca08d2ab2cdc0d41dec9571ad741b24..eb9d4d5bfe521816cc84785904260a71bafc938a 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -286,76 +286,6 @@ String preloadTypeToString(WebMediaPlayer::Preload preloadType) {
 
 }  // anonymous namespace
 
-class HTMLMediaElement::AutoplayHelperClientImpl
-    : public AutoplayExperimentHelper::Client {
- public:
-  static AutoplayHelperClientImpl* create(HTMLMediaElement* element) {
-    return new AutoplayHelperClientImpl(element);
-  }
-
-  virtual ~AutoplayHelperClientImpl();
-
-  using RecordMetricsBehavior = HTMLMediaElement::RecordMetricsBehavior;
-
-  double currentTime() const override { return m_element->currentTime(); }
-  double duration() const override { return m_element->duration(); }
-  bool paused() const override { return m_element->paused(); }
-  bool ended() const override { return m_element->ended(); }
-  bool muted() const override { return m_element->muted(); }
-  void setMuted(bool muted) override { m_element->setMuted(muted); }
-  void playInternal() override { m_element->playInternal(); }
-  void pauseInternal() override { m_element->pauseInternal(); }
-  bool isLockedPendingUserGesture() const override {
-    return m_element->isLockedPendingUserGesture();
-  }
-  void unlockUserGesture() override { m_element->unlockUserGesture(); }
-  void recordAutoplayMetric(AutoplayMetrics metric) override {
-    m_element->recordAutoplayMetric(metric);
-  }
-  bool shouldAutoplay() override {
-    return m_element->shouldAutoplay(RecordMetricsBehavior::DoNotRecord);
-  }
-  bool isHTMLVideoElement() const override {
-    return m_element->isHTMLVideoElement();
-  }
-  bool isHTMLAudioElement() const override {
-    return m_element->isHTMLAudioElement();
-  }
-
-  // Document
-  bool isLegacyViewportType() override;
-  PageVisibilityState pageVisibilityState() const override;
-  String autoplayExperimentMode() const override;
-
-  // Frame
-  bool isCrossOrigin() const override {
-    const LocalFrame* frame = m_element->document().frame();
-    return frame && frame->isCrossOriginSubframe();
-  }
-
-  bool isAutoplayAllowedPerSettings() const override;
-
-  // LayoutObject
-  void setRequestPositionUpdates(bool) override;
-  IntRect absoluteBoundingBoxRect() const override;
-
-  DEFINE_INLINE_VIRTUAL_TRACE() {
-    visitor->trace(m_element);
-    Client::trace(visitor);
-  }
-
- private:
-  AutoplayHelperClientImpl(HTMLMediaElement* element) : m_element(element) {}
-
-  Member<HTMLMediaElement> m_element;
-};
-
-void HTMLMediaElement::recordAutoplayMetric(AutoplayMetrics metric) {
-  DEFINE_STATIC_LOCAL(EnumerationHistogram, autoplayHistogram,
-                      ("Blink.MediaElement.Autoplay", NumberOfAutoplayMetrics));
-  autoplayHistogram.count(metric);
-}
-
 MIMETypeRegistry::SupportsType HTMLMediaElement::supportsType(
     const ContentType& contentType) {
   DEFINE_STATIC_LOCAL(const String, codecs, ("codecs"));
@@ -452,9 +382,6 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName,
       m_videoTracks(this, VideoTrackList::create(*this)),
       m_textTracks(this, nullptr),
       m_audioSourceNode(nullptr),
-      m_autoplayHelperClient(AutoplayHelperClientImpl::create(this)),
-      m_autoplayHelper(
-          AutoplayExperimentHelper::create(m_autoplayHelperClient.get())),
       m_autoplayUmaHelper(AutoplayUmaHelper::create(this)),
       m_remotePlaybackClient(nullptr),
       m_autoplayVisibilityObserver(nullptr) {
@@ -465,8 +392,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName,
   // If any experiment is enabled, then we want to enable a user gesture by
   // default, otherwise the experiment does nothing.
   if ((document.settings() &&
-       document.settings()->mediaPlaybackRequiresUserGesture()) ||
-      m_autoplayHelper->isExperimentEnabled()) {
+       document.settings()->mediaPlaybackRequiresUserGesture())) {
     m_lockedPendingUserGesture = true;
   }
 
@@ -513,12 +439,10 @@ void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument) {
   // default, otherwise the experiment does nothing.
   bool oldDocumentRequiresUserGesture =
       (oldDocument.settings() &&
-       oldDocument.settings()->mediaPlaybackRequiresUserGesture()) ||
-      m_autoplayHelper->isExperimentEnabled();
+       oldDocument.settings()->mediaPlaybackRequiresUserGesture());
   bool newDocumentRequiresUserGesture =
       (document().settings() &&
-       document().settings()->mediaPlaybackRequiresUserGesture()) ||
-      m_autoplayHelper->isExperimentEnabled();
+       document().settings()->mediaPlaybackRequiresUserGesture());
   if (newDocumentRequiresUserGesture && !oldDocumentRequiresUserGesture) {
     m_lockedPendingUserGesture = true;
   }
@@ -748,7 +672,10 @@ String HTMLMediaElement::canPlayType(const String& mimeType) const {
 void HTMLMediaElement::load() {
   BLINK_MEDIA_LOG << "load(" << (void*)this << ")";
 
-  m_autoplayHelper->loadMethodCalled();
+  if (isLockedPendingUserGesture() &&
+      UserGestureIndicator::utilizeUserGesture()) {

[foolip, 2016/11/23 12:40:55]

This isn't what I had expected, but https://codereview.chromium.org/1799253002
explains why it's different from before the experiment.

[Zhiqiang Zhang (Slow), 2016/11/23 17:18:28]

This logic was flattened from AutoplayExperimentalHelper. Can you elaborate how
it's different and what do you expect?

[foolip, 2016/11/23 19:15:55]

Originally it was processingUserGesture(), so that's what I expected. Took me
some digging to figure out where utilizeUserGesture() had come from, commented
as an FYI if anyone else was surprised.

+    unlockUserGesture();
+  }
 
   m_ignorePreloadNone = true;
   invokeLoadAlgorithm();
@@ -1069,8 +996,6 @@ void HTMLMediaElement::loadResource(const WebMediaPlayerSource& source,
   // The resource fetch algorithm
   setNetworkState(kNetworkLoading);
 
-  m_autoplayHelper->loadingStarted();
-
   // Set m_currentSrc *before* changing to the cache url, the fact that we are
   // loading from the app cache is an internal detail not exposed through the
   // media element API.
@@ -1737,10 +1662,6 @@ void HTMLMediaElement::setReadyState(ReadyState state) {
     if (shouldAutoplay(RecordMetricsBehavior::DoRecord)) {
       m_autoplayUmaHelper->onAutoplayInitiated(AutoplaySource::Attribute);
 
-      // If the autoplay experiment says that it's okay to play now,
-      // then don't require a user gesture.
-      m_autoplayHelper->becameReadyToPlay();
-
       if (!isGestureNeededForPlayback()) {
         if (isHTMLVideoElement() && muted() &&
             RuntimeEnabledFeatures::autoplayMutedVideosEnabled()) {
@@ -2117,12 +2038,8 @@ bool HTMLMediaElement::autoplay() const {
 bool HTMLMediaElement::shouldAutoplay(
     const RecordMetricsBehavior recordMetrics) {

[foolip, 2016/11/23 12:40:55]

Remove the now unused argument.

[Zhiqiang Zhang (Slow), 2016/11/23 18:27:34]

Done.

   if (m_autoplaying && m_paused && autoplay()) {

[foolip, 2016/11/23 12:40:55]

This now ends up looking a bit silly and not trivial to understand. I suggest:

if (document().isSandboxed(SandboxAutomaticFeatures))
  return false;
return m_autoplaying && m_paused && autoplay();

[Zhiqiang Zhang (Slow), 2016/11/23 18:27:34]

Done.

-    if (document().isSandboxed(SandboxAutomaticFeatures)) {
-      if (recordMetrics == RecordMetricsBehavior::DoRecord)
-        m_autoplayHelper->recordSandboxFailure();
+    if (document().isSandboxed(SandboxAutomaticFeatures))
       return false;
-    }
-
     return true;
   }
 
@@ -2235,16 +2152,9 @@ ScriptPromise HTMLMediaElement::playForBindings(ScriptState* scriptState) {
 Nullable<ExceptionCode> HTMLMediaElement::play() {
   BLINK_MEDIA_LOG << "play(" << (void*)this << ")";
 
-  m_autoplayHelper->playMethodCalled();
-
   if (!UserGestureIndicator::processingUserGesture()) {
     m_autoplayUmaHelper->onAutoplayInitiated(AutoplaySource::Method);
     if (isGestureNeededForPlayback()) {
-      // If playback is deferred, then don't start playback but don't
-      // fail yet either.
-      if (m_autoplayHelper->isPlaybackDeferred())
-        return nullptr;
-
       // If we're already playing, then this play would do nothing anyway.
       // Call playInternal to handle scheduling the promise resolution.
       if (!m_paused) {
@@ -2252,7 +2162,6 @@ Nullable<ExceptionCode> HTMLMediaElement::play() {
         return nullptr;
       }
 
-      recordAutoplayMetric(PlayMethodFailed);
       String message = ExceptionMessages::failedToExecute(
           "play", "HTMLMediaElement",
           "API can only be initiated by a user gesture.");
@@ -2262,9 +2171,7 @@ Nullable<ExceptionCode> HTMLMediaElement::play() {
     }
   } else {
     UserGestureIndicator::utilizeUserGesture();

[foolip, 2016/11/23 12:40:55]

Optional nit: hoist utilizeUserGesture() to above. After
https://codereview.chromium.org/2529593002 the above processingUserGesture()
would otherwise be the only call. If you like I can include it in my CL, but
it'll conflict with this one.

[Zhiqiang Zhang (Slow), 2016/11/23 18:27:34]

Did you mean to move it to utilizeUserGesture() to the if statement (replacing
the processingUserGesture())?
Please do it in your CL and I'll do rebase later :)

[foolip, 2016/11/23 19:15:55]

Will do, but will base it on your CL since it's going through CQ now.

-    // We ask the helper to remove the gesture requirement for us, so that
-    // it can record the reason.
-    m_autoplayHelper->unlockUserGesture(GesturelessPlaybackEnabledByPlayMethod);
+    unlockUserGesture();
   }
 
   if (m_error && m_error->code() == MediaError::kMediaErrSrcNotSupported)
@@ -2331,8 +2238,6 @@ void HTMLMediaElement::pauseInternal() {
   if (m_networkState == kNetworkEmpty)
     invokeResourceSelectionAlgorithm();
 
-  m_autoplayHelper->pauseMethodCalled();
-
   m_autoplaying = false;
 
   if (!m_paused) {
@@ -2455,7 +2360,6 @@ void HTMLMediaElement::setMuted(bool muted) {
     unlockUserGesture();
 
   m_muted = muted;
-  m_autoplayHelper->mutedChanged();
 
   scheduleEvent(EventTypeNames::volumechange);
 
@@ -3345,7 +3249,6 @@ void HTMLMediaElement::updatePlayState() {
       webMediaPlayer()->setRate(playbackRate());
       webMediaPlayer()->setVolume(effectiveMediaVolume());
       webMediaPlayer()->play();
-      m_autoplayHelper->playbackStarted();
     }
 
     if (mediaControls())
@@ -3356,7 +3259,6 @@ void HTMLMediaElement::updatePlayState() {
   } else {  // Should not be playing right now
     if (isPlaying) {
       webMediaPlayer()->pause();
-      m_autoplayHelper->playbackStopped();
     }
 
     m_playbackProgressTimer.stop();
@@ -3838,8 +3740,6 @@ DEFINE_TRACE(HTMLMediaElement) {
   visitor->trace(m_playPromiseResolveList);
   visitor->trace(m_playPromiseRejectList);
   visitor->trace(m_audioSourceProvider);
-  visitor->trace(m_autoplayHelperClient);
-  visitor->trace(m_autoplayHelper);
   visitor->trace(m_autoplayUmaHelper);
   visitor->trace(m_srcObject);
   visitor->trace(m_autoplayVisibilityObserver);
@@ -3917,9 +3817,6 @@ bool HTMLMediaElement::isGestureNeededForPlayback() const {
     return false;
   }
 
-  if (m_autoplayHelper->isGestureRequirementOverridden())
-    return false;
-
   return true;
 }
 
@@ -3946,23 +3843,6 @@ void HTMLMediaElement::videoWillBeDrawnToCanvas() const {
     UseCounter::count(document(), UseCounter::HiddenAutoplayedVideoInCanvas);
 }
 
-void HTMLMediaElement::notifyPositionMayHaveChanged(
-    const IntRect& visibleRect) {
-  m_autoplayHelper->positionChanged(visibleRect);
-}
-
-void HTMLMediaElement::updatePositionNotificationRegistration() {
-  m_autoplayHelper->updatePositionNotificationRegistration();
-}
-
-// TODO(liberato): remove once autoplay gesture override experiment concludes.
-void HTMLMediaElement::triggerAutoplayViewportCheckForTesting() {
-  if (FrameView* view = document().view())
-    m_autoplayHelper->positionChanged(
-        view->rootFrameToContents(view->computeVisibleArea()));
-  m_autoplayHelper->triggerAutoplayViewportCheckForTesting();
-}
-
 void HTMLMediaElement::scheduleResolvePlayPromises() {
   // TODO(mlamouri): per spec, we should create a new task but we can't create
   // a new cancellable task without cancelling the previous one. There are two
@@ -4161,46 +4041,4 @@ DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl) {
   visitor->trace(m_client);
 }
 
-HTMLMediaElement::AutoplayHelperClientImpl::~AutoplayHelperClientImpl() {}
-
-bool HTMLMediaElement::AutoplayHelperClientImpl::isLegacyViewportType() {
-  return m_element->document().viewportDescription().isLegacyViewportType();
-}
-
-PageVisibilityState
-HTMLMediaElement::AutoplayHelperClientImpl::pageVisibilityState() const {
-  return m_element->document().pageVisibilityState();
-}
-
-String HTMLMediaElement::AutoplayHelperClientImpl::autoplayExperimentMode()
-    const {
-  String mode;
-  if (m_element->document().settings())
-    mode = m_element->document().settings()->autoplayExperimentMode();
-
-  return mode;
-}
-
-bool HTMLMediaElement::AutoplayHelperClientImpl::isAutoplayAllowedPerSettings()
-    const {
-  return m_element->isAutoplayAllowedPerSettings();
-}
-
-void HTMLMediaElement::AutoplayHelperClientImpl::setRequestPositionUpdates(
-    bool request) {
-  if (LayoutObject* layoutObject = m_element->layoutObject()) {
-    LayoutMediaItem layoutMediaItem =
-        LayoutMediaItem(toLayoutMedia(layoutObject));
-    layoutMediaItem.setRequestPositionUpdates(request);
-  }
-}
-
-IntRect HTMLMediaElement::AutoplayHelperClientImpl::absoluteBoundingBoxRect()
-    const {
-  IntRect result;
-  if (LayoutObject* object = m_element->layoutObject())
-    result = object->absoluteBoundingBoxRect();
-  return result;
-}
-
 }  // namespace blink