Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(477)

Unified Diff: third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h

Issue 1470153004: Autoplay experiment metric fixes and additions. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: rebased. Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h
diff --git a/third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h b/third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h
index a35406017692312b1c2aab08fc8c726e0ca14326..5766ee7cf00812e7d70178bba82b67bf6653b71a 100644
--- a/third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h
+++ b/third_party/WebKit/Source/core/html/AutoplayExperimentHelper.h
@@ -13,6 +13,8 @@ namespace blink {
class Document;
class HTMLMediaElement;
class EventListener;
+class LayoutObject;
+class AutoplayExperimentTest;
// These values are used for a histogram. Do not reorder.
enum AutoplayMetrics {
@@ -62,25 +64,96 @@ enum AutoplayMetrics {
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 n
liberato (no reviews please) 2016/01/29 08:25:24 yeah, i tried to comment around it. it is intende
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 reall
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 HTM
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.
@@ -104,11 +177,6 @@ public:
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.
@@ -137,7 +205,16 @@ private:
// 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>*);
@@ -145,7 +222,9 @@ private:
// Return our media element's document.
Document& document() const;
- HTMLMediaElement& element() const;
+ Client& client() const;
+
+ bool isUserGestureRequiredForPlay() const;
inline bool enabled(Mode mode) const
{
@@ -154,7 +233,13 @@ private:
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;
@@ -168,6 +253,18 @@ private:
// 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_autoplayMediaCo
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;
@@ -176,6 +273,9 @@ private:
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,
@@ -185,7 +285,6 @@ inline AutoplayExperimentHelper::Mode& operator|=(AutoplayExperimentHelper::Mode
return a;
}
-
} // namespace blink
#endif // AutoplayExperimentHelper_h

Powered by Google App Engine
This is Rietveld 408576698