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

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

Issue 1329853004: Include viewport visibility checks for autoplay experiment. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@autoplay_step1
Patch Set: rebased. Created 5 years, 3 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: Source/core/html/AutoplayExperimentHelper.h
diff --git a/Source/core/html/AutoplayExperimentHelper.h b/Source/core/html/AutoplayExperimentHelper.h
index c9d015f062bce1126923f2ec3613ad7067c92219..8cb0b2358396325c28530e9692734f81f0d90ad1 100644
--- a/Source/core/html/AutoplayExperimentHelper.h
+++ b/Source/core/html/AutoplayExperimentHelper.h
@@ -79,12 +79,58 @@ public:
void mutedChanged();
void positionChanged();
+ // For testing.
+ void triggerAutoplayViewportCheck();
+
private:
+ // The location and size of our element, and the viewport. Also supports
+ // "not valid", in case some of the information isn't available.
philipj_slow 2015/09/21 15:02:05 I looks like PageVisibilityState is doesn't have a
liberato (no reviews please) 2015/09/23 06:14:57 i don't see a way to make it work automatically.
philipj_slow 2015/09/25 15:32:27 OK, looks like it was successfully removed.
+ class LocationState {
+ public:
+ LocationState() : m_valid(false) {}
+ LocationState(Element&);
+
+ bool valid() const { return m_valid; }
+ PageVisibilityState visibilityState() const { return m_visibilityState; }
+ const IntRect& element() const { return m_element; }
+ const IntRect& screen() const { return m_screen; }
+
+ bool operator==(const LocationState&) const;
+ bool operator!=(const LocationState&) const;
+
+ // Return true if and only if the player is visible.
philipj_slow 2015/09/21 15:02:05 s/player/element/ and maybe rename to isElementVis
liberato (no reviews please) 2015/09/23 06:14:57 Done.
+ bool isInViewport() const;
+
+ // Return true if and only if the element's page is visible.
+ bool isPageVisible() const;
+
+ private:
+ bool m_valid;
+ PageVisibilityState m_visibilityState;
+ IntRect m_element;
+ IntRect m_screen;
+ };
+
+ // 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
philipj_slow 2015/09/25 15:32:27 Another "player" here if you want to fix it.
// 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 the element described by LocationState is
+ // not visible, and we care that it must be visible.
philipj_slow 2015/09/21 15:02:05 It's pretty hard to tell from this header when you
liberato (no reviews please) 2015/09/23 06:14:57 if yuo have LocationState, pass it in. if not, th
philipj_slow 2015/09/25 15:32:27 Acknowledged.
+ bool meetsVisibilityRequirements(const LocationState&) const;
+
+ // Return false if and only if the player is not visible, and we care
philipj_slow 2015/09/21 15:02:05 s/player/element/?
liberato (no reviews please) 2015/09/23 06:14:57 Done.
+ // 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();
@@ -98,6 +144,9 @@ private:
// there are several different cases.
void prepareToPlay(AutoplayMetrics);
+ // Process a timer for checking visibility.
+ void viewportTimerFired(Timer<AutoplayExperimentHelper>*);
+
// Return our media element's document.
Document& document() const;
@@ -115,6 +164,20 @@ private:
// 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_registeredWithView : 1;
+
+ // According to our last position update, are we in the viewport?
+ bool m_wasInViewport : 1;
+
+ // According to our last position update, where was our element?
+ LocationState m_lastLocation;
+
+ // When was m_lastLocation set?
+ double m_lastLocationUpdateTime;
+
+ Timer<AutoplayExperimentHelper> m_viewportTimer;
+
friend class Internals;
};

Powered by Google App Engine
This is Rietveld 408576698