Monitor the intersection of video and viewport.

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

Index: third_party/WebKit/Source/core/html/AutoplayUmaHelper.h
diff --git a/third_party/WebKit/Source/core/html/AutoplayUmaHelper.h b/third_party/WebKit/Source/core/html/AutoplayUmaHelper.h
index 7dda64bb56d55bc2b799c5290a8a9b9f2c420e88..251d75e489b5e7c255fe771024007eaaad234c9b 100644
--- a/third_party/WebKit/Source/core/html/AutoplayUmaHelper.h
+++ b/third_party/WebKit/Source/core/html/AutoplayUmaHelper.h
@@ -58,6 +58,8 @@ class AutoplayUmaHelper final : public EventListener {
 
   bool hasSource() const { return m_source != AutoplaySource::NumberOfSources; }
 
+  void onVisibilityChangedForMutedVideo(bool isVisible);
+
   DECLARE_VIRTUAL_TRACE();
 
  private:
@@ -77,9 +79,6 @@ class AutoplayUmaHelper final : public EventListener {
   void maybeStartRecordingMutedVideoOffscreenDuration();
   void maybeStopRecordingMutedVideoOffscreenDuration();
 
-  void onVisibilityChangedForMutedVideoOffscreenDuration(bool isVisibile);
-  void onVisibilityChangedForMutedVideoPlayMethodBecomeVisible(bool isVisible);
-
   bool shouldListenToUnloadEvent() const;
 
   // The autoplay source. Use AutoplaySource::NumberOfSources for invalid
@@ -88,10 +87,10 @@ class AutoplayUmaHelper final : public EventListener {
   // The media element this UMA helper is attached to. |m_element| owns |this|.
   Member<HTMLMediaElement> m_element;
 
-  // The observer is used to observe whether a muted video autoplaying by play()
+  // Indicates starting observing whether a muted video autoplaying by play()

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

With better variable naming, I think you should delete this comment. Otherwise,
the comment, as it is, needs to be rewritten (grammar issue).

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

Done.

   // method become visible at some point.
   // The UMA is pending for recording as long as this observer is non-null.

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

It's not an observer anymore. Maybe this should say something like "when this
boolean transitions from true to false, a UMA is recorded."

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

Done.

-  Member<ElementVisibilityObserver> m_mutedVideoPlayMethodVisibilityObserver;
+  bool m_mutedVideoPlayMethodVisibilityObserver = false;

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

naming: Wow. The existing code's naming of members and methods is so
complicated. If I were changing this code, I'd feel compelled to do massive
cleanup here.

Regardless, for your change, I'd suggest something much simpler like (IIUC how
this boolean is used):

  m_isObservingVisibilityOfMutedAutoplayVideo;

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

To be honest, I'm not sure this boolean is even necessary.

If you get events that say the video is visible or not visible, you should be
able to tell when a transition occurs.

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

I still keep this boolean. The observing may only start when playing events are
handled for a muted video, and will immediately stop once it becomes visible.

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

Done.

 
   // -----------------------------------------------------------------------
   // Variables used for recording the duration of autoplay muted video playing
@@ -108,11 +107,10 @@ class AutoplayUmaHelper final : public EventListener {
   // Whether an autoplaying muted video is visible.
   bool m_isVisible;
 
-  // The observer is used to observer an autoplaying muted video changing it's
+  // Indicates starting observing an autoplaying muted video changing it's
   // visibility, which is used for offscreen duration UMA.  The UMA is pending
   // for recording as long as this observer is non-null.
-  Member<ElementVisibilityObserver>
-      m_mutedVideoOffscreenDurationVisibilityObserver;
+  bool m_mutedVideoOffscreenDurationVisibilityObserver = false;

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

ditto here: Same problems with code comment, variable naming, etc.

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

Done.

 };
 
 }  // namespace blink