Monitor the intersection of video and viewport.

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 56ad755a6e9c9743151d0a62dc99d5335758bdd9..ebc064e784aec71c02a4db6356c5502803f711b5 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -464,8 +464,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName,
       m_autoplayHelper(
           AutoplayExperimentHelper::create(m_autoplayHelperClient.get())),
       m_autoplayUmaHelper(AutoplayUmaHelper::create(this)),
-      m_remotePlaybackClient(nullptr),
-      m_autoplayVisibilityObserver(nullptr) {
+      m_remotePlaybackClient(nullptr) {
   ThreadState::current()->registerPreFinalizer(this);
 
   BLINK_MEDIA_LOG << "HTMLMediaElement(" << (void*)this << ")";
@@ -482,6 +481,12 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName,
   addElementToDocumentMap(this, &document);
 
   UseCounter::count(document, UseCounter::HTMLMediaElement);
+
+  m_videoViewportRatioObserver = new ElementVisibilityObserver(
+      this, WTF::bind(&HTMLMediaElement::onVideoViewportRatioChanged,
+                      wrapWeakPersistent(this)));
+  m_videoViewportRatioObserver->start(
+      Vector<float>({std::numeric_limits<float>::min(), 0.85}));

[miu, 2016/11/05 02:47:47]

1. Where does 0.85 come from, and how do we change it? This puts logic into
Blink that really depends on our feature in Chromium code. We want to keep all
control/decision logic in the Chromium code.

2. Our Chromium code will want to know all ratios (including zero), because we
will want to "debounce" before deciding to commit to starting or stopping
remoting. Thus, I think you don't need the second
ElementVisibilityObserver::start(vector) method.

[xjz, 2016/11/09 02:24:28]

Done. Now is able to monitor every changes on video/viewport ratio.

 }
 
 HTMLMediaElement::~HTMLMediaElement() {
@@ -497,6 +502,8 @@ HTMLMediaElement::~HTMLMediaElement() {
 void HTMLMediaElement::dispose() {
   closeMediaSource();
 
+  m_videoViewportRatioObserver->stop();
+
   // Destroying the player may cause a resource load to be canceled,
   // which could result in LocalDOMWindow::dispatchWindowLoadEvent() being
   // called via ResourceFetch::didLoadResource(), then
@@ -1723,16 +1730,7 @@ void HTMLMediaElement::setReadyState(ReadyState state) {
       if (!isGestureNeededForPlayback()) {
         if (isHTMLVideoElement() && muted() &&
             RuntimeEnabledFeatures::autoplayMutedVideosEnabled()) {
-          // We might end up in a situation where the previous
-          // observer didn't had time to fire yet. We can avoid
-          // creating a new one in this case.
-          if (!m_autoplayVisibilityObserver) {
-            m_autoplayVisibilityObserver = new ElementVisibilityObserver(
-                this,
-                WTF::bind(&HTMLMediaElement::onVisibilityChangedForAutoplay,
-                          wrapWeakPersistent(this)));
-            m_autoplayVisibilityObserver->start();
-          }
+          m_autoplayWhenVisible = true;
         } else {
           m_paused = false;
           invalidateCachedTime();
@@ -2355,8 +2353,8 @@ void HTMLMediaElement::setMuted(bool muted) {
 
   bool wasAutoplayingMuted =
       !paused() && m_muted && isLockedPendingUserGesture();
-  bool wasPendingAutoplayMuted = m_autoplayVisibilityObserver && paused() &&
-                                 m_muted && isLockedPendingUserGesture();
+  bool wasPendingAutoplayMuted = m_autoplayWhenVisible && paused() && m_muted &&

[miu, 2016/11/05 02:47:47]

Is the m_autoplayWhenVisible boolean necessary? Why can't this expression be:

  bool wasPendingAutoplayMuted = !isGestureNeededForPlayback() &&
isHTMLVideoElement() && RuntimeEnabledFeatures::autoplayMutedVideosEnabled() &&
paused() && m_muted && isLockedPendingUserGesture();

(Perhaps you might have the previous author of these LOC look over this logic,
to be sure it is correct.)

[xjz, 2016/11/09 02:24:28]

As replied to the comment in .h file, I keeps the boolean to save a bunch of
checks, and also avoid the risk changing the original logic.

+                                 isLockedPendingUserGesture();
 
   if (UserGestureIndicator::processingUserGesture())
     unlockUserGesture();
@@ -2384,8 +2382,7 @@ void HTMLMediaElement::setMuted(bool muted) {
   // If an element was a candidate for autoplay muted but not visible, it will
   // have a visibility observer ready to start its playback.

[miu, 2016/11/05 02:47:46]

Please adjust comment.

[xjz, 2016/11/09 02:24:28]

Done.

   if (wasPendingAutoplayMuted) {
-    m_autoplayVisibilityObserver->stop();
-    m_autoplayVisibilityObserver = nullptr;
+    m_autoplayWhenVisible = false;
   }
 }
 
@@ -3742,7 +3739,7 @@ DEFINE_TRACE(HTMLMediaElement) {
   visitor->trace(m_autoplayHelper);
   visitor->trace(m_autoplayUmaHelper);
   visitor->trace(m_srcObject);
-  visitor->trace(m_autoplayVisibilityObserver);
+  visitor->trace(m_videoViewportRatioObserver);
   visitor->template registerWeakMembers<HTMLMediaElement,
                                         &HTMLMediaElement::clearWeakMembers>(
       this);
@@ -3968,11 +3965,12 @@ EnumerationHistogram& HTMLMediaElement::showControlsHistogram() const {
   return histogram;
 }
 
-void HTMLMediaElement::onVisibilityChangedForAutoplay(bool isVisible) {
+void HTMLMediaElement::onVideoViewportRatioChanged(bool isVisible) {
+  m_autoplayUmaHelper->onVisibilityChangedForMutedVideo(isVisible);
   if (!isVisible)
     return;
 
-  if (shouldAutoplay()) {
+  if (shouldAutoplay() && m_autoplayWhenVisible) {
     m_paused = false;
     invalidateCachedTime();
     scheduleEvent(EventTypeNames::play);
@@ -3981,13 +3979,12 @@ void HTMLMediaElement::onVisibilityChangedForAutoplay(bool isVisible) {
 
     updatePlayState();
   }
+  m_autoplayWhenVisible = false;
 
-  // TODO(zqzhang): There's still flaky leak if onVisibilityChangedForAutoplay()
-  // is never called.  The leak comes from either ElementVisibilityObserver or
-  // IntersectionObserver. Should keep an eye on it.  See
-  // https://crbug.com/627539
-  m_autoplayVisibilityObserver->stop();
-  m_autoplayVisibilityObserver = nullptr;
+  if (m_webMediaPlayer) {
+    m_webMediaPlayer->videoViewportRatioChanged(
+        m_videoViewportRatioObserver->ratio());
+  }
 }
 
 void HTMLMediaElement::clearWeakMembers(Visitor* visitor) {