Autoplay experiment metric fixes and additions.

third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h

 // Copyright 2015 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 AutoplayExperimentHelper_h
 #define AutoplayExperimentHelper_h
 
 #include "core/page/Page.h"
 #include "platform/Timer.h"
 #include "platform/geometry/IntRect.h"
 
 namespace blink {
 class Document;
 class HTMLMediaElement;
 class EventListener;
+class LayoutObject;
+class AutoplayExperimentTest;
 
 // These values are used for a histogram. Do not reorder.
 enum AutoplayMetrics {
     // Media element with autoplay seen.
     AutoplayMediaFound = 0,
     // Autoplay enabled and user stopped media play at any point.
     AutoplayPaused = 1,
     // Autoplay enabled but user bailed out on media play early.
     AutoplayBailout = 2,
     // Autoplay disabled but user manually started media.
@@ skipping 29 matching lines @@
 
     // play() failed to play due to gesture requirement.
     PlayMethodFailed = 10,
 
     // Some play, whether user initiated or not, started.
     AnyPlaybackStarted = 11,
     // Some play, whether user initiated or not, paused.
     AnyPlaybackPaused = 12,
     // Some playback, whether user initiated or not, bailed out early.
     AnyPlaybackBailout = 13,
+    // Some playback, whether user initiated or not, played to completion.
+    AnyPlaybackComplete = 14,
+
+    // Number of audio elements detected that reach the resource fetch algorithm.
+    AnyAudioElement = 15,
+    // Numer of video elements detected that reach the resource fetch algorithm.
+    AnyVideoElement = 16,
+
+    // User gesture was bypassed, and playback started, and media played to
+    // completion without a user-initiated pause.
+    AutoplayComplete = 17,
+
+    // Autoplay started after the gesture requirement was removed by a
+    // user gesture load().
+    GesturelessPlaybackEnabledByLoad = 18,
+
+    // Gestureless playback started after the gesture requirement was removed
+    // because src is media stream.
+    GesturelessPlaybackEnabledByStream = 19,
+
+    // Gestureless playback was started, but it is unknown why a user gesture
+    // was not required.  This includes the case where none is ever required.
+    GesturelessPlaybackUnknownReason = 20,

[philipj_slow, 2016/01/12 15:12:17]

As I said in an earlier comment, there should be no need to an "unknown" reason,
the precise reason is knowable. If it's only needed as the initial value, maybe
GesturelessPlaybackNotEnabled and assert in the appropriate places that it has
changed to one of the other values?

[liberato (no reviews please), 2016/01/29 08:25:24]

yeah, i tried to comment around it.

it is intended to catch cases in which the gesture requirement was not
overridden, possibly because it was not required.  for example, testing this on
desktop produces exclusively this value.

i don't expect to ever see this thing in practice, nor would i want to assert if
it does somehow slip through -- there's no reason to impact somebody's playback
experience.  if it shows up in the metrics, then i'll investigate.

i've updated the name to "GesturelessPlaybackNotOverridden", since that's
actually what it means.

[philipj_slow, 2016/02/02 08:08:15]

Acknowledged.

+
+    // Gestureless playback was enabled by a user gesture play() call.
+    GesturelessPlaybackEnabledByPlayMethod = 21,
 
     // This enum value must be last.
     NumberOfAutoplayMetrics,
 };
 
-class AutoplayExperimentHelper final {
-    DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
+class CORE_EXPORT AutoplayExperimentHelper final {
+    friend class blink::AutoplayExperimentTest;

[philipj_slow, 2016/01/12 15:12:18]

This is in namespace blink {}, is the prefix really needed?

[liberato (no reviews please), 2016/01/29 08:25:24]

Done.

+
 public:
-    explicit AutoplayExperimentHelper(HTMLMediaElement&);
+    // For easier testing, collect all the things we care about here.
+    class Client {
+    public:
+        virtual ~Client() {}
+
+        // HTMLMediaElement
+        virtual double currentTime() const = 0;
+        virtual double duration() const = 0;
+        virtual bool paused() const = 0;
+        virtual bool muted() const = 0;
+        virtual void setMuted(bool) = 0;
+        virtual void playInternal() = 0;
+        virtual bool isUserGestureRequiredForPlay() const = 0;
+        virtual void removeUserGestureRequirement() = 0;
+        virtual void recordAutoplayMetric(AutoplayMetrics) = 0;
+        enum class RecordMetricsBehavior { DoNotRecord,
+            RecordOnSandboxFailure };
+        virtual bool shouldAutoplay(RecordMetricsBehavior = RecordMetricsBehavior::DoNotRecord) = 0;
+        virtual bool isHTMLVideoElement() const = 0;
+        virtual bool isHTMLAudioElement() const = 0;
+
+        // Document
+        virtual bool isLegacyViewportType() = 0;
+        virtual PageVisibilityState pageVisibilityState() const = 0;
+        virtual WTF::String autoplayExperimentMode() const = 0;

[philipj_slow, 2016/01/12 15:12:18]

Is the WTF:: prefix really needed? It's not in HTMLMediaElement.h

[liberato (no reviews please), 2016/01/29 08:25:24]

Done.

+
+        // LayoutObject
+        virtual void setRequestPositionUpdates(bool) = 0;
+        virtual IntRect absoluteBoundingBoxRect() const = 0;
+    };
+
+    explicit AutoplayExperimentHelper(Client&);
     ~AutoplayExperimentHelper();
 
     void becameReadyToPlay();
     void playMethodCalled();
     void pauseMethodCalled();
+    void loadMethodCalled();
     void mutedChanged();
     void positionChanged(const IntRect&);
     void updatePositionNotificationRegistration();
+    void recordSandboxFailure();
+    void loadingStarted();
+    void playbackStarted();
+    void playbackEnded();
+    void initialPlayWithUserGesture();
 
+    // Remove the user gesture requirement, and record why.  If there is no
+    // gesture requirement, then this does nothing.
+    void removeUserGestureRequirement(AutoplayMetrics);
+
+    // Set the position to the current view's position, and
     void triggerAutoplayViewportCheckForTesting();
+    //

[philipj_slow, 2016/01/12 15:12:18]

Missing comment?

[liberato (no reviews please), 2016/01/29 08:25:24]

i meant to delete the method.

+    void triggerViewportTimerForTesting();
 
     enum Mode {
         // Do not enable the autoplay experiment.
         ExperimentOff = 0,
         // Enable gestureless autoplay for video elements.
         ForVideo      = 1 << 0,
         // Enable gestureless autoplay for audio elements.
         ForAudio      = 1 << 1,
         // Restrict gestureless autoplay to media that is in a visible page.
         IfPageVisible = 1 << 2,
         // Restrict gestureless autoplay to media that is visible in
         // the viewport.
         IfViewport    = 1 << 3,
         // Restrict gestureless autoplay to audio-less or muted media.
         IfMuted       = 1 << 4,
         // Restrict gestureless autoplay to sites which contain the
         // viewport tag.
         IfMobile      = 1 << 5,
         // If gestureless autoplay is allowed, then mute the media before
         // starting to play.
         PlayMuted     = 1 << 6,
     };
 
-    DEFINE_INLINE_TRACE()
-    {
-        visitor->trace(m_element);
-    }
-
 private:
     // Register to receive position updates, if we haven't already.  If we
     // have, then this does nothing.
     void registerForPositionUpdatesIfNeeded();
 
     // Un-register for position updates, if we are currently registered.
     void unregisterForPositionUpdatesIfNeeded();
 
     // Return true if any only if this player meets (most) of the eligibility
     // requirements for the experiment to override the need for a user
     // gesture.  This includes everything except the visibility test.
     bool isEligible() const;
 
     // Return false if and only if m_element is not visible, and we care
     // that it must be visible.
     bool meetsVisibilityRequirements() const;
 
     // Set the muted flag on the media if we're in an experiment mode that
     // requires it, else do nothing.
     void muteIfNeeded();
 
     // Maybe override the requirement for a user gesture, and start playing
     // autoplay media.  Returns true if only if it starts playback.
     bool maybeStartPlaying();
 
     // Configure internal state to record that the autoplay experiment is
     // going to start playback.  This doesn't actually start playback, since
     // there are several different cases.
-    void prepareToPlay(AutoplayMetrics);
+    void prepareToAutoplay(AutoplayMetrics);
+
+    // Record that an attempt to play without a user gesture has happened.
+    // This does not assume whether or not the play attempt will succeed.
+    // This method takes no action after it is called once.
+    void autoplayMediaEncountered();
+
+    // If we are about to enter a paused state, call this to record
+    // autoplay metrics.
+    void recordMetricsBeforePause();
 
     // Process a timer for checking visibility.
     void viewportTimerFired(Timer<AutoplayExperimentHelper>*);
 
     // Return our media element's document.
     Document& document() const;
 
-    HTMLMediaElement& element() const;
+    Client& client() const;
+
+    bool isUserGestureRequiredForPlay() const;
 
     inline bool enabled(Mode mode) const
     {
         return ((int)m_mode) & ((int)mode);
     }
 
     Mode fromString(const String& mode);
 
-    RawPtrWillBeMember<HTMLMediaElement> m_element;
+    void recordAutoplayMetric(AutoplayMetrics);
+
+    // Could stopping at this point be considered a bailout of playback?
+    // (as in, "The user really didn't want to play this").
+    bool isBailout() const;
+
+    Client& m_client;
 
     Mode m_mode;
 
     // Autoplay experiment state.
     // True if we've received a play() without a pause().
     bool m_playPending : 1;
 
     // Are we registered with the view for position updates?
     bool m_registeredWithLayoutObject : 1;
 
     // According to our last position update, are we in the viewport?
     bool m_wasInViewport : 1;
 
+    // Have we counted this autoplay media in the metrics yet?
+    bool m_autoplayMediaCounted : 1;
+
+    // Have we started playback of this media yet?
+    bool m_playbackStartedBefore : 1;
+
+    // Did the first playback of the media occur without a user gesture?
+    bool m_initialPlayWithoutUserGesture : 1;
+
+    // Did we record that this media element exists in the metrics yet?
+    bool m_recordedElement : 1;

[philipj_slow, 2016/01/12 15:12:18]

The distinction between this and m_autoplayMediaCounted isn't very clear from
the comments.

[liberato (no reviews please), 2016/01/29 08:25:24]

Done.

+
     // According to our last position update, where was our element?
     IntRect m_lastLocation;
     IntRect m_lastVisibleRect;
 
     // When was m_lastLocation set?
     double m_lastLocationUpdateTime;
 
     Timer<AutoplayExperimentHelper> m_viewportTimer;
+
+    // Reason that autoplay would be allowed to proceed without a user gesture.
+    AutoplayMetrics m_autoplayDeferredMetric;
 };
 
 inline AutoplayExperimentHelper::Mode& operator|=(AutoplayExperimentHelper::Mode& a,
     const AutoplayExperimentHelper::Mode& b)
 {
     a = static_cast<AutoplayExperimentHelper::Mode>(static_cast<int>(a) | static_cast<int>(b));
     return a;
 }
 
-
 } // namespace blink
 
 #endif // AutoplayExperimentHelper_h