Autoplay experiment metric fixes and additions.

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

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 329 matching lines @@
     , m_initialPlayWithoutUserGesture(false)
     , m_autoplayMediaCounted(false)
     , m_inOverlayFullscreenVideo(false)
     , m_audioTracks(AudioTrackList::create(*this))
     , m_videoTracks(VideoTrackList::create(*this))
     , m_textTracks(nullptr)
 #if ENABLE(WEB_AUDIO)
     , m_audioSourceNode(nullptr)
 #endif
     , m_autoplayHelper(*this)
+    , m_autoplayDeferredMetric(GesturelessPlaybackUnknownReason)
+    , m_recordedElement(false)
 {
 #if ENABLE(OILPAN)
     ThreadState::current()->registerPreFinalizer(this);
 #endif
     ASSERT(RuntimeEnabledFeatures::mediaEnabled());
 
     WTF_LOG(Media, "HTMLMediaElement::HTMLMediaElement(%p)", this);
 
     if (document.settings() && document.settings()->mediaPlaybackRequiresUserGesture())
         m_userGestureRequiredForPlay = true;
@@ skipping 302 matching lines @@
     case WebMimeRegistry::IsSupported:
         canPlay = "probably";
         break;
     }
 
     WTF_LOG(Media, "HTMLMediaElement::canPlayType(%p, %s, %s) -> %s", this, mimeType.utf8().data(), keySystem.utf8().data(), canPlay.utf8().data());
 
     return canPlay;
 }
 
-void HTMLMediaElement::recordMetricsIfPausing()
+void HTMLMediaElement::recordMetricsBeforePause()
 {
-    // If not playing, then nothing to record.
-    // TODO(liberato): test metrics.  this was m_paused.
-    if (m_paused)
-        return;
+    ASSERT(!m_paused);
 
     const bool bailout = isBailout();
 
     // Record that play was paused.  We don't care if it was autoplay,
     // play(), or the user manually started it.
     recordAutoplayMetric(AnyPlaybackPaused);
     if (bailout)
         recordAutoplayMetric(AnyPlaybackBailout);
 
     // If this was a gestureless play, then record that separately.
     // These cover attr and play() gestureless starts.
     if (m_initialPlayWithoutUserGesture) {
         m_initialPlayWithoutUserGesture = false;
 
         recordAutoplayMetric(AutoplayPaused);
 
         if (bailout)
             recordAutoplayMetric(AutoplayBailout);
     }
 }
 
+void HTMLMediaElement::recordMetricsAtPlaybackEnd()
+{
+    recordAutoplayMetric(AnyPlaybackComplete);
+    if (m_initialPlayWithoutUserGesture) {
+        m_initialPlayWithoutUserGesture = false;
+        recordAutoplayMetric(AutoplayComplete);
+    }
+}
+
 void HTMLMediaElement::load()
 {
     WTF_LOG(Media, "HTMLMediaElement::load(%p)", this);
 
-    recordMetricsIfPausing();
+    if (!m_paused)
+        recordMetricsBeforePause();
 
     if (UserGestureIndicator::processingUserGesture() && m_userGestureRequiredForPlay) {
         recordAutoplayMetric(AutoplayEnabledThroughLoad);
-        m_userGestureRequiredForPlay = false;
+        removeUserGestureRequirement(GesturelessPlaybackEnabledByLoad);
         // While usergesture-initiated load()s technically count as autoplayed,
         // they don't feel like such to the users and hence we don't want to
         // count them for the purposes of metrics.
         m_autoplayMediaCounted = true;
     }
 
     prepareForLoad();
     loadInternal();
     prepareToPlay();
 }
@@ skipping 236 matching lines @@
     if (fastHasAttribute(mutedAttr))
         m_muted = true;
     updateVolume();
 
     ASSERT(!m_mediaSource);
 
     bool attemptLoad = true;
 
     if (url.protocolIs(mediaSourceBlobProtocol)) {
         if (isMediaStreamURL(url.string())) {
             m_userGestureRequiredForPlay = false;

[philipj_slow, 2015/11/26 15:30:56]

Drop this line, it looks like it would make removeUserGestureRequirement() a
no-op.

[liberato (no reviews please), 2015/12/02 00:58:08]

Done.

+            removeUserGestureRequirement(GesturelessPlaybackEnabledByStream);
         } else {
             m_mediaSource = HTMLMediaSource::lookup(url.string());
 
             if (m_mediaSource) {
                 if (!m_mediaSource->attachToElement(this)) {
                     // Forget our reference to the MediaSource, so we leave it alone
                     // while processing remainder of load failure.
                     m_mediaSource = nullptr;
                     attemptLoad = false;
                 }
@@ skipping 551 matching lines @@
     if (m_readyState >= HAVE_CURRENT_DATA && oldState < HAVE_CURRENT_DATA && !m_haveFiredLoadedData) {
         m_haveFiredLoadedData = true;
         shouldUpdateDisplayState = true;
         scheduleEvent(EventTypeNames::loadeddata);
         setShouldDelayLoadEvent(false);
     }
 
     bool isPotentiallyPlaying = potentiallyPlaying();
     if (m_readyState == HAVE_FUTURE_DATA && oldState <= HAVE_CURRENT_DATA && tracksAreReady) {
         scheduleEvent(EventTypeNames::canplay);
-        if (isPotentiallyPlaying)
+        if (isPotentiallyPlaying) {
             scheduleEvent(EventTypeNames::playing);
+        }
         shouldUpdateDisplayState = true;
     }
 
     if (m_readyState == HAVE_ENOUGH_DATA && oldState < HAVE_ENOUGH_DATA && tracksAreReady) {
         if (oldState <= HAVE_CURRENT_DATA) {
             scheduleEvent(EventTypeNames::canplay);
-            if (isPotentiallyPlaying)
+            if (isPotentiallyPlaying) {
                 scheduleEvent(EventTypeNames::playing);
+            }
         }
 
         // Check for autoplay, and record metrics about it if needed.
-        if (shouldAutoplay(RecordMetricsBehavior::DoRecord)) {
+        if (shouldAutoplay(RecordMetricsBehavior::RecordOnSandboxFailure)) {
             // If the autoplay experiment says that it's okay to play now,
             // then don't require a user gesture.
             m_autoplayHelper.becameReadyToPlay();
 
+            // Record that we have encounted autoplay media.  We do this
+            // now, after the experiment has had a chance to override the
+            // user gesture requirement, since it assumes that the media
+            // will play if and only if a user gesture is not required.
+            autoplayMediaEncountered();
+
             if (!m_userGestureRequiredForPlay) {
                 m_paused = false;
                 invalidateCachedTime();
                 scheduleEvent(EventTypeNames::play);
                 scheduleEvent(EventTypeNames::playing);
             }
         }
 
         scheduleEvent(EventTypeNames::canplaythrough);
 
@@ skipping 299 matching lines @@
 }
 
 bool HTMLMediaElement::autoplay() const
 {
     return fastHasAttribute(autoplayAttr);
 }
 
 bool HTMLMediaElement::shouldAutoplay(const RecordMetricsBehavior recordMetrics)
 {
     if (m_autoplaying && m_paused && autoplay()) {
-        if (recordMetrics == RecordMetricsBehavior::DoRecord)
-            autoplayMediaEncountered();
-
         if (document().isSandboxed(SandboxAutomaticFeatures)) {
-            if (recordMetrics == RecordMetricsBehavior::DoRecord)
+            if (recordMetrics == RecordMetricsBehavior::RecordOnSandboxFailure) {
+                // We record autoplayMediaEncountered here because we know
+                // that the autoplay attempt will fail.
+                autoplayMediaEncountered();
                 recordAutoplayMetric(AutoplayDisabledBySandbox);
+            }
             return false;
         }
 
         return true;
     }
 
     return false;
 }
 
 String HTMLMediaElement::preload() const
@@ skipping 68 matching lines @@
 
         if (m_userGestureRequiredForPlay) {
             recordAutoplayMetric(PlayMethodFailed);
             String message = ExceptionMessages::failedToExecute("play", "HTMLMediaElement", "API can only be initiated by a user gesture.");
             document().addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, message));
             return;
         }
     } else if (m_userGestureRequiredForPlay) {
         if (m_autoplayMediaCounted)
             recordAutoplayMetric(AutoplayManualStart);
+        // Don't let future gestureless playbacks affect metrics.  This is
+        // also why we don't have to go through removeUserGestureRequirement.
+        m_autoplayMediaCounted = true;
+
         m_userGestureRequiredForPlay = false;

[philipj_slow, 2015/11/26 15:30:56]

Call recordAutoplayMetric(GesturelessPlaybackEnabledByPlay)? I don't think you
should need an unknown case, we know exactly what removes the gesture if we just
track it.

[liberato (no reviews please), 2015/12/02 00:58:09]

updated to removeUserGestureRequirement.  if gestureless playback starts later,
then we'll record why it was allowed.  i have to test this better tomorrow when
i'm in the office.

     }
 
     playInternal();
 }
 
 void HTMLMediaElement::playInternal()
 {
     WTF_LOG(Media, "HTMLMediaElement::playInternal(%p)", this);
 
     // 4.8.10.9. Playing the media resource
     if (m_networkState == NETWORK_EMPTY)
         scheduleDelayedAction(LoadMediaResource);
 
     // Generally "ended" and "looping" are exclusive. Here, the loop attribute
     // is ignored to seek back to start in case loop was set after playback
     // ended. See http://crbug.com/364442
     if (endedPlayback(LoopCondition::Ignored))
         seek(0);
 
     if (m_paused) {
         m_paused = false;
         invalidateCachedTime();
         scheduleEvent(EventTypeNames::play);
 
-        if (m_readyState <= HAVE_CURRENT_DATA)
+        if (m_readyState <= HAVE_CURRENT_DATA) {

[philipj_slow, 2015/11/26 15:30:56]

Drop the added braces here and elsewhere: git checkout -p origin/master

[liberato (no reviews please), 2015/12/02 00:58:08]

Done.

             scheduleEvent(EventTypeNames::waiting);
-        else if (m_readyState >= HAVE_FUTURE_DATA)
+        } else if (m_readyState >= HAVE_FUTURE_DATA) {
             scheduleEvent(EventTypeNames::playing);
+        }
     }
-    m_autoplaying = false;
 
     updatePlayState();
 }
 
 void HTMLMediaElement::autoplayMediaEncountered()
 {
     if (!m_autoplayMediaCounted) {
         m_autoplayMediaCounted = true;
         recordAutoplayMetric(AutoplayMediaFound);
 
-        if (!m_userGestureRequiredForPlay)
+        // If no user gesture was required, then assume that playback will
+        // actually start.
+        if (!m_userGestureRequiredForPlay) {
             m_initialPlayWithoutUserGesture = true;
+            recordAutoplayMetric(m_autoplayDeferredMetric);

[philipj_slow, 2015/11/26 15:30:56]

I'm still finding this a bit hard to follow. In general, I would like to see
this:

1. All state is stored on AutoplayExperimentHelper, including the existing
m_initialPlayWithoutUserGesture and m_autoplayMediaCounted.
2. HTMLMediaElement calls methods to set relevant state, like
m_autoplayHelper.userGestureNotRequired(reason).
3. HTMLMediaElement calls the helper at points where it might want to record
metrics based on that state, like the existing
m_autoplayHelper.playMethodCalled() or
m_autoplayHelper.playerPlaying()/m_autoplayHelper.playerPaused() in
updatePlayState().

I think that would make the footprint in HTMLMediaElement a lot smaller, and
hopefully reduce the amount of if (m_this) { m_that = true } that's hard to
track, by just recording what events have occurred and laying the puzzle only
the place where the metrics are actually recorded.

If that makes it easier to write unit tests (for AutoplayExperimentHelper)
that'd be a bonus.

[liberato (no reviews please), 2015/12/02 00:58:08]

oh, i didn't realize that you were suggesting to refactor all of it earlier.
sure, that makes sense.

+        }
     }
 }
 
 bool HTMLMediaElement::isBailout() const
 {
     // We count the user as having bailed-out on the video if they watched
     // less than one minute and less than 50% of it.
     const double playedTime = currentTime();
     const double progress = playedTime / duration();
     return (playedTime < 60) && (progress < 0.5);
 }
 
 void HTMLMediaElement::pause()
 {
     WTF_LOG(Media, "HTMLMediaElement::pause(%p)", this);
 
     if (m_networkState == NETWORK_EMPTY)
         scheduleDelayedAction(LoadMediaResource);
 
     m_autoplayHelper.pauseMethodCalled();
 
     m_autoplaying = false;
 
     if (!m_paused) {
-        recordMetricsIfPausing();
+        recordMetricsBeforePause();
 
         m_paused = true;
         scheduleTimeupdateEvent(false);
         scheduleEvent(EventTypeNames::pause);
     }
 
     updatePlayState();
 }
 
 void HTMLMediaElement::requestRemotePlayback()
@@ skipping 672 matching lines @@
         // If the media element has a loop attribute specified
         if (loop()) {
             m_sentEndEvent = false;
             //  then seek to the earliest possible position of the media resource and abort these steps.
             seek(0);
         } else {
             // If the media element has still ended playback, and the direction of playback is still
             // forwards, and paused is false,
             if (!m_paused) {
                 // changes paused to true and fires a simple event named pause at the media element.
+                recordMetricsAtPlaybackEnd();
                 m_paused = true;
                 scheduleEvent(EventTypeNames::pause);
             }
             // Queue a task to fire a simple event named ended at the media element.
             if (!m_sentEndEvent) {
                 m_sentEndEvent = true;
                 scheduleEvent(EventTypeNames::ended);
             }
-            recordMetricsIfPausing();
         }
     } else {
         m_sentEndEvent = false;
     }
 
     updatePlayState();
 }
 
 void HTMLMediaElement::durationChanged()
 {
@@ skipping 186 matching lines @@
     if (shouldBePlaying) {
         setDisplayMode(Video);
         invalidateCachedTime();
 
         if (!isPlaying) {
             // Set rate, muted before calling play in case they were set before the media engine was setup.
             // The media engine should just stash the rate and muted values since it isn't already playing.
             webMediaPlayer()->setRate(playbackRate());
             updateVolume();
             webMediaPlayer()->play();
+            recordAutoplayMetric(AnyPlaybackStarted);
         }
 
         if (mediaControls())
             mediaControls()->playbackStarted();
         startPlaybackProgressTimer();
         m_playing = true;
-        recordAutoplayMetric(AnyPlaybackStarted);
+        m_autoplaying = false;

[liberato (no reviews please), 2015/11/25 18:58:54]

i moved this from playInternal().  not sure that this is per-spec (it only
mentions removing autoplay on a play() call).  otherwise, though, letting a
video element with an autoplay attribute play to completion, then seeking to the
middle, starts playback immediately when updating the network state.  that seems
really weird.

[philipj_slow, 2015/11/26 15:30:56]

Nice catch! Fixed in https://github.com/whatwg/html/pull/362

Can you fix this in a separate CL with a test suitable for upstreaming to
web-platform-tests? hopefully there's some really short media resource you can
use, and if not you could always use two samples of silence PCM as a data: URL.

[liberato (no reviews please), 2015/12/02 00:58:09]

https://codereview.chromium.org/1482393003/

 
     } else { // Should not be playing right now
         if (isPlaying)
             webMediaPlayer()->pause();
+
         refreshCachedTime();
 
         m_playbackProgressTimer.stop();
         m_playing = false;
         double time = currentTime();
         if (time > m_lastSeekTime)
             addPlayedRange(m_lastSeekTime, time);
 
         if (couldPlayIfEnoughData())
             prepareToPlay();
@@ skipping 95 matching lines @@
         mediaControls()->refreshCastButtonVisibilityWithoutUpdate();
 
     if (layoutObject())
         layoutObject()->setShouldDoFullPaintInvalidation();
 }
 
 void HTMLMediaElement::stop()
 {
     WTF_LOG(Media, "HTMLMediaElement::stop(%p)", this);
 
-    recordMetricsIfPausing();
+    if (!m_paused)
+        recordMetricsBeforePause();

[philipj_slow, 2015/11/26 15:30:56]

As I think I pointed out in an earlier review, this is ActiveDOMObject::stop()
and it's probably not a good idea to consider it like a user-initiated pause.
Maybe this will be reached if the tab was closed, but I don't think so.

[liberato (no reviews please), 2015/12/02 00:58:08]

Done.

 
     // Close the async event queue so that no events are enqueued by userCancelledLoad.
     cancelPendingEventsAndCallbacks();
     m_asyncEventQueue->close();
 
     userCancelledLoad();
 
     // Stop the playback without generating events
     m_playing = false;
     m_paused = true;
@@ skipping 489 matching lines @@
     // Select the first video track if a video track hasn't been selected yet.
     if (videoTracks().length() > 0 && videoTracks().selectedIndex() == -1)
         videoTracks().anonymousIndexedGetter(0)->setSelected(true);
 }
 
 bool HTMLMediaElement::isUserGestureRequiredForPlay() const
 {
     return m_userGestureRequiredForPlay;
 }
 
-void HTMLMediaElement::removeUserGestureRequirement()
+void HTMLMediaElement::removeUserGestureRequirement(AutoplayMetrics deferredMetric)
 {
-    m_userGestureRequiredForPlay = false;
-}
-
-void HTMLMediaElement::setInitialPlayWithoutUserGestures(bool value)
-{
-    m_initialPlayWithoutUserGesture = value;
+    if (m_userGestureRequiredForPlay) {
+        m_userGestureRequiredForPlay = false;
+        m_autoplayDeferredMetric = deferredMetric;
+    }
 }
 
 void HTMLMediaElement::setNetworkState(NetworkState state)
 {
     if (m_networkState != state) {
         m_networkState = state;
         if (MediaControls* controls = mediaControls())
             controls->networkStateChanged();
+
+        if (m_networkState == NETWORK_LOADING && !m_recordedElement) {

[philipj_slow, 2015/11/26 15:30:56]

Why here instead of in HTMLMediaElement::loadResource at the beginning of the
resource fetch algorithm?

[liberato (no reviews please), 2015/12/02 00:58:09]

Done.

+            m_recordedElement = true;
+            recordAutoplayMetric(isHTMLVideoElement()
+                ? AnyVideoElement : AnyAudioElement);
+        }
     }
 }
 
 void HTMLMediaElement::notifyPositionMayHaveChanged(const IntRect& visibleRect)
 {
     m_autoplayHelper.positionChanged(visibleRect);
 }
 
 void HTMLMediaElement::updatePositionNotificationRegistration()
 {
@@ skipping 68 matching lines @@
     visitor->trace(m_client);
 }
 
 DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)
 {
     visitor->trace(m_client);
 }
 #endif
 
 }