Fix AutoplayUmaHelper when autoplay is initiated from multiple sources

This CL makes AutoplayUmaHelper to handle the case where the page
autoplays by both attribute and play() method. Therefore we can avoid
counting the metrics into the wrong bucket.

BUG=699176

Review-Url: https://codereview.chromium.org/2767093002
Cr-Commit-Position: refs/heads/master@{#459215}

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

@@ -29,7 +29,6 @@ AutoplayUmaHelper* AutoplayUmaHelper::create(HTMLMediaElement* element) {
 AutoplayUmaHelper::AutoplayUmaHelper(HTMLMediaElement* element)
     : EventListener(CPPEventListenerType),
       ContextLifecycleObserver(nullptr),
-      m_source(AutoplaySource::NumberOfSources),
       m_element(element),
       m_mutedVideoPlayMethodVisibilityObserver(nullptr),
       m_mutedVideoAutoplayOffscreenStartTimeMS(0),
@@ -46,38 +45,53 @@ bool AutoplayUmaHelper::operator==(const EventListener& other) const {
 void AutoplayUmaHelper::onAutoplayInitiated(AutoplaySource source) {
   DEFINE_STATIC_LOCAL(EnumerationHistogram, videoHistogram,
                       ("Media.Video.Autoplay",
-                       static_cast<int>(AutoplaySource::NumberOfSources)));
+                       static_cast<int>(AutoplaySource::NumberOfUmaSources)));
   DEFINE_STATIC_LOCAL(EnumerationHistogram, mutedVideoHistogram,
                       ("Media.Video.Autoplay.Muted",
-                       static_cast<int>(AutoplaySource::NumberOfSources)));
+                       static_cast<int>(AutoplaySource::NumberOfUmaSources)));
   DEFINE_STATIC_LOCAL(EnumerationHistogram, audioHistogram,
                       ("Media.Audio.Autoplay",
-                       static_cast<int>(AutoplaySource::NumberOfSources)));
+                       static_cast<int>(AutoplaySource::NumberOfUmaSources)));
   DEFINE_STATIC_LOCAL(
       EnumerationHistogram, blockedMutedVideoHistogram,
       ("Media.Video.Autoplay.Muted.Blocked", AutoplayBlockedReasonMax));
 
   // Autoplay already initiated
-  // TODO(zqzhang): how about having autoplay attribute and calling `play()` in
-  // the script?
-  if (hasSource())
+  if (m_sources.count(source))
     return;
 
-  m_source = source;
+  m_sources.insert(source);
 
   // Record the source.
   if (m_element->isHTMLVideoElement()) {
-    videoHistogram.count(static_cast<int>(m_source));
+    videoHistogram.count(static_cast<int>(source));
     if (m_element->muted())
-      mutedVideoHistogram.count(static_cast<int>(m_source));
+      mutedVideoHistogram.count(static_cast<int>(source));
   } else {
-    audioHistogram.count(static_cast<int>(m_source));
+    audioHistogram.count(static_cast<int>(source));
+  }
+
+  // Record dual source.
+  if (m_sources.size() ==
+      static_cast<size_t>(AutoplaySource::NumberOfSources)) {
+    if (m_element->isHTMLVideoElement()) {
+      videoHistogram.count(static_cast<int>(AutoplaySource::DualSource));
+      if (m_element->muted())
+        mutedVideoHistogram.count(static_cast<int>(AutoplaySource::DualSource));
+    } else {
+      audioHistogram.count(static_cast<int>(AutoplaySource::DualSource));
+    }
   }
 
   // Record the child frame and top-level frame URLs for autoplay muted videos
   // by attribute.
   if (m_element->isHTMLVideoElement() && m_element->muted()) {
-    if (source == AutoplaySource::Attribute) {
+    if (m_sources.size() ==
+        static_cast<size_t>(AutoplaySource::NumberOfSources)) {
+      Platform::current()->recordRapporURL(
+          "Media.Video.Autoplay.Muted.DualSource.Frame",
+          m_element->document().url());
+    } else if (source == AutoplaySource::Attribute) {
       Platform::current()->recordRapporURL(
           "Media.Video.Autoplay.Muted.Attribute.Frame",
           m_element->document().url());
@@ -261,8 +275,8 @@ void AutoplayUmaHelper::handleContextDestroyed() {
 }
 
 void AutoplayUmaHelper::maybeStartRecordingMutedVideoPlayMethodBecomeVisible() {
-  if (m_source != AutoplaySource::Method || !m_element->isHTMLVideoElement() ||
-      !m_element->muted())
+  if (!m_sources.count(AutoplaySource::Method) ||
+      !m_element->isHTMLVideoElement() || !m_element->muted())
     return;
 
   m_mutedVideoPlayMethodVisibilityObserver = new ElementVisibilityObserver(
@@ -289,7 +303,8 @@ void AutoplayUmaHelper::maybeStopRecordingMutedVideoPlayMethodBecomeVisible(
 }
 
 void AutoplayUmaHelper::maybeStartRecordingMutedVideoOffscreenDuration() {
-  if (!m_element->isHTMLVideoElement() || !m_element->muted())
+  if (!m_element->isHTMLVideoElement() || !m_element->muted() ||
+      !m_sources.count(AutoplaySource::Method))
     return;
 
   // Start recording muted video playing offscreen duration.
@@ -322,13 +337,14 @@ void AutoplayUmaHelper::maybeStopRecordingMutedVideoOffscreenDuration() {
       std::min<int64_t>(m_mutedVideoAutoplayOffscreenDurationMS,
                         std::numeric_limits<int32_t>::max()));
 
-  if (m_source == AutoplaySource::Method) {
-    DEFINE_STATIC_LOCAL(
-        CustomCountHistogram, durationHistogram,
-        ("Media.Video.Autoplay.Muted.PlayMethod.OffscreenDuration", 1,
-         maxOffscreenDurationUmaMS, offscreenDurationUmaBucketCount));
-    durationHistogram.count(boundedTime);
-  }
+  DCHECK(m_sources.count(AutoplaySource::Method));
+
+  DEFINE_STATIC_LOCAL(
+      CustomCountHistogram, durationHistogram,
+      ("Media.Video.Autoplay.Muted.PlayMethod.OffscreenDuration", 1,
+       maxOffscreenDurationUmaMS, offscreenDurationUmaBucketCount));
+  durationHistogram.count(boundedTime);
+
   m_mutedVideoOffscreenDurationVisibilityObserver->stop();
   m_mutedVideoOffscreenDurationVisibilityObserver = nullptr;
   m_mutedVideoAutoplayOffscreenDurationMS = 0;
@@ -363,7 +379,7 @@ bool AutoplayUmaHelper::shouldListenToContextDestroyed() const {
 bool AutoplayUmaHelper::shouldRecordUserPausedAutoplayingCrossOriginVideo()
     const {
   return m_element->isInCrossOriginFrame() && m_element->isHTMLVideoElement() &&
-         m_source != AutoplaySource::NumberOfSources &&
+         !m_sources.empty() &&
          !m_recordedCrossOriginAutoplayResults.count(
              CrossOriginAutoplayResult::UserPaused);
 }

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

@@ -20,8 +20,12 @@ enum class AutoplaySource {
   Attribute = 0,
   // Autoplay comes from `play()` method.
   Method = 1,
-  // This enum value must be last.
+  // Used for checking dual source.
   NumberOfSources = 2,
+  // Both sources are used.
+  DualSource = 2,
+  // This enum value must be last.
+  NumberOfUmaSources = 3,
 };
 
 // These values are used for histograms. Do not reorder.
@@ -75,7 +79,7 @@ class CORE_EXPORT AutoplayUmaHelper : public EventListener,
 
   bool isVisible() const { return m_isVisible; }
 
-  bool hasSource() const { return m_source != AutoplaySource::NumberOfSources; }
+  bool hasSource() const { return !m_sources.empty(); }
 
   DECLARE_VIRTUAL_TRACE();
 
@@ -105,9 +109,9 @@ class CORE_EXPORT AutoplayUmaHelper : public EventListener,
   bool shouldListenToContextDestroyed() const;
   bool shouldRecordUserPausedAutoplayingCrossOriginVideo() const;
 
-  // The autoplay source. Use AutoplaySource::NumberOfSources for invalid
-  // source.
-  AutoplaySource m_source;
+  // The autoplay sources.
+  std::set<AutoplaySource> m_sources;
+
   // The media element this UMA helper is attached to. |m_element| owns |this|.
   Member<HTMLMediaElement> m_element;
 

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

@@ -70,7 +70,7 @@ TEST_F(AutoplayUmaHelperTest, VisibilityChangeWhenUnload) {
   EXPECT_CALL(umaHelper(), handleContextDestroyed());
 
   mediaElement().setMuted(true);
-  umaHelper().onAutoplayInitiated(AutoplaySource::Attribute);
+  umaHelper().onAutoplayInitiated(AutoplaySource::Method);
   umaHelper().handlePlayingEvent();
   pageHolder().reset();
   ::testing::Mock::VerifyAndClear(&umaHelper());

tools/metrics/histograms/histograms.xml

@@ -82438,6 +82438,7 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 <enum name="AutoplaySource" type="int">
   <int value="0" label="autoplay attribute"/>
   <int value="1" label="play() method"/>
+  <int value="2" label="dual source"/>
 </enum>
 
 <enum name="AutoSigninFirstRun" type="int">

tools/metrics/rappor/rappor.xml

@@ -1362,6 +1362,17 @@ now we are only interested in H264, VP8 and VP9.
   </summary>
 </rappor-metric>
 
+<rappor-metric name="Media.Video.Autoplay.Muted.DualSource.Frame"
+    type="ETLD_PLUS_ONE">
+  <owner>avayvod@chromium.org</owner>
+  <owner>mlamouri@chromium.org</owner>
+  <owner>zqzhang@chromium.org</owner>
+  <summary>
+    The eTLD+1 of the frame URL that has an autoplay muted video by both
+    attribute and play() method.
+  </summary>
+</rappor-metric>
+
 <rappor-metric name="Media.Video.Autoplay.Muted.PlayMethod.Frame"
     type="ETLD_PLUS_ONE">
   <owner>avayvod@chromium.org</owner>