Let AutoplayUmaHelper listen to visibilitychange instead of unload

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

Index: third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
diff --git a/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp b/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
index 72ab6fe5645646c002559684f5da5634e1bf8242..c581a6aa437174c422f01aaa4a035239302cc65a 100644
--- a/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
+++ b/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
@@ -7,7 +7,6 @@
 #include "core/dom/Document.h"
 #include "core/dom/ElementVisibilityObserver.h"
 #include "core/events/Event.h"
-#include "core/frame/LocalDOMWindow.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLMediaElement.h"
 #include "platform/Histogram.h"
@@ -105,15 +104,13 @@ void AutoplayUmaHelper::recordAutoplayUnmuteStatus(
 }
 
 void AutoplayUmaHelper::didMoveToNewDocument(Document& oldDocument) {
-  if (!shouldListenToUnloadEvent())
+  if (!shouldListenToPageVisibilityChangeEvent())
     return;
 
-  if (oldDocument.domWindow())
-    oldDocument.domWindow()->removeEventListener(EventTypeNames::unload, this,
-                                                 false);
-  if (m_element->document().domWindow())
-    m_element->document().domWindow()->addEventListener(EventTypeNames::unload,
-                                                        this, false);
+  oldDocument.removeEventListener(EventTypeNames::visibilitychange, this,
+                                  false);
+  m_element->document().addEventListener(EventTypeNames::visibilitychange, this,
+                                         false);
 }
 
 void AutoplayUmaHelper::onVisibilityChangedForMutedVideoPlayMethodBecomeVisible(
@@ -146,8 +143,8 @@ void AutoplayUmaHelper::handleEvent(ExecutionContext* executionContext,
     handlePlayingEvent();
   else if (event->type() == EventTypeNames::pause)
     handlePauseEvent();
-  else if (event->type() == EventTypeNames::unload)
-    handleUnloadEvent();
+  else if (event->type() == EventTypeNames::visibilitychange)
+    handlePageVisibilityChangeEvent();
   else
     NOTREACHED();
 }
@@ -155,7 +152,6 @@ void AutoplayUmaHelper::handleEvent(ExecutionContext* executionContext,
 void AutoplayUmaHelper::handlePlayingEvent() {
   maybeStartRecordingMutedVideoPlayMethodBecomeVisible();
   maybeStartRecordingMutedVideoOffscreenDuration();
-

[mlamouri (slow - plz ping), 2016/10/31 16:33:31]

nit: is this change intentional?

[Zhiqiang Zhang (Slow), 2016/10/31 16:48:54]

Undone this accidental change :)

   m_element->removeEventListener(EventTypeNames::playing, this, false);
 }
 
@@ -163,7 +159,9 @@ void AutoplayUmaHelper::handlePauseEvent() {
   maybeStopRecordingMutedVideoOffscreenDuration();
 }
 
-void AutoplayUmaHelper::handleUnloadEvent() {
+void AutoplayUmaHelper::handlePageVisibilityChangeEvent() {
+  if (!m_element->document().hidden())
+    return;
   maybeStopRecordingMutedVideoPlayMethodBecomeVisible(false);
   maybeStopRecordingMutedVideoOffscreenDuration();
 }
@@ -179,9 +177,8 @@ void AutoplayUmaHelper::maybeStartRecordingMutedVideoPlayMethodBecomeVisible() {
                     onVisibilityChangedForMutedVideoPlayMethodBecomeVisible,
                 wrapWeakPersistent(this)));
   m_mutedVideoPlayMethodVisibilityObserver->start();
-  if (m_element->document().domWindow())
-    m_element->document().domWindow()->addEventListener(EventTypeNames::unload,
-                                                        this, false);
+  m_element->document().addEventListener(EventTypeNames::visibilitychange, this,
+                                         false);
 }
 
 void AutoplayUmaHelper::maybeStopRecordingMutedVideoPlayMethodBecomeVisible(
@@ -195,7 +192,7 @@ void AutoplayUmaHelper::maybeStopRecordingMutedVideoPlayMethodBecomeVisible(
   histogram.count(visible);
   m_mutedVideoPlayMethodVisibilityObserver->stop();
   m_mutedVideoPlayMethodVisibilityObserver = nullptr;
-  maybeUnregisterUnloadListener();
+  maybeUnregisterPageVisibilityChangeListener();
 }
 
 void AutoplayUmaHelper::maybeStartRecordingMutedVideoOffscreenDuration() {
@@ -214,9 +211,8 @@ void AutoplayUmaHelper::maybeStartRecordingMutedVideoOffscreenDuration() {
                     wrapWeakPersistent(this)));
   m_mutedVideoOffscreenDurationVisibilityObserver->start();
   m_element->addEventListener(EventTypeNames::pause, this, false);
-  if (m_element->document().domWindow())
-    m_element->document().domWindow()->addEventListener(EventTypeNames::unload,
-                                                        this, false);
+  m_element->document().addEventListener(EventTypeNames::visibilitychange, this,
+                                         false);
 }
 
 void AutoplayUmaHelper::maybeStopRecordingMutedVideoOffscreenDuration() {
@@ -251,16 +247,17 @@ void AutoplayUmaHelper::maybeStopRecordingMutedVideoOffscreenDuration() {
   m_mutedVideoOffscreenDurationVisibilityObserver = nullptr;
   m_mutedVideoAutoplayOffscreenDurationMS = 0;
   m_element->removeEventListener(EventTypeNames::pause, this, false);
-  maybeUnregisterUnloadListener();
+  maybeUnregisterPageVisibilityChangeListener();
 }
 
-void AutoplayUmaHelper::maybeUnregisterUnloadListener() {
-  if (!shouldListenToUnloadEvent() && m_element->document().domWindow())
-    m_element->document().domWindow()->removeEventListener(
-        EventTypeNames::unload, this, false);
+void AutoplayUmaHelper::maybeUnregisterPageVisibilityChangeListener() {
+  if (!shouldListenToPageVisibilityChangeEvent()) {
+    m_element->document().removeEventListener(EventTypeNames::visibilitychange,
+                                              this, false);
+  }
 }
 
-bool AutoplayUmaHelper::shouldListenToUnloadEvent() const {
+bool AutoplayUmaHelper::shouldListenToPageVisibilityChangeEvent() const {
   return m_mutedVideoPlayMethodVisibilityObserver ||
          m_mutedVideoOffscreenDurationVisibilityObserver;
 }