Adding Rappor metrics for disabling cross-origin autoplay with sound experiment

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..de40fc6b5efc92502a6f48d052d5df9ea23d9711 100644
--- a/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
+++ b/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
@@ -11,6 +11,7 @@
 #include "core/frame/Settings.h"
 #include "core/html/HTMLMediaElement.h"
 #include "platform/Histogram.h"
+#include "public/platform/Platform.h"
 #include "wtf/CurrentTime.h"
 
 namespace blink {
@@ -94,6 +95,52 @@ void AutoplayUmaHelper::onAutoplayInitiated(AutoplaySource source) {
   m_element->addEventListener(EventTypeNames::playing, this, false);
 }
 
+void AutoplayUmaHelper::recordCrossOriginExperimentMetric(
+    AutoplayCrossOriginExperimentMetric metric) {
+  if (!m_element->isHTMLVideoElement())

[whywhat, 2016/11/29 21:42:04]

nit: Should we avoid calling this for non-video elements and turn this into a
DCHECK?

[Zhiqiang Zhang (Slow), 2016/11/30 17:30:52]

I think that would introduce a lot of duplicate if statements, so I prefer not.

+    return;
+  if (!m_element->isCrossOrigin())

[whywhat, 2016/11/29 21:42:04]

nit: ditto

+    return;
+  if (m_recordedCrossOriginExperimentMetrics.count(metric))

[whywhat, 2016/11/29 21:42:04]

nit: add a comment why we only record each result once (per element I assume)?

[Zhiqiang Zhang (Slow), 2016/11/30 17:30:52]

Done.

+    return;
+
+  switch (metric) {
+    case AutoplayCrossOriginExperimentMetric::AutoplayAllowed:
+      // Record metric
+      Platform::current()->recordRapporURL(
+          "Media.Autoplay.CrossOriginExperiment.Allowed.ChildFrame",
+          m_element->document().url());
+      Platform::current()->recordRapporURL(
+          "Media.Autoplay.CrossOriginExperiment.Allowed.TopLevelFrame",
+          m_element->document().topDocument().url());
+      break;
+    case AutoplayCrossOriginExperimentMetric::AutoplayBlocked:
+      Platform::current()->recordRapporURL(
+          "Media.Autoplay.CrossOriginExperiment.Blocked.ChildFrame",
+          m_element->document().url());
+      Platform::current()->recordRapporURL(
+          "Media.Autoplay.CrossOriginExperiment.Blocked.TopLevelFrame",
+          m_element->document().topDocument().url());
+      break;
+    case AutoplayCrossOriginExperimentMetric::PlayedWithGesture:
+      if (!m_recordedCrossOriginExperimentMetrics.count(

[whywhat, 2016/11/29 21:42:04]

nit: a comment would help why this check is here.

[Zhiqiang Zhang (Slow), 2016/11/30 17:30:52]

Done.

+              AutoplayCrossOriginExperimentMetric::AutoplayBlocked)) {
+        return;
+      }
+      Platform::current()->recordRapporURL(
+          "Media.Autoplay.CrossOriginExperiment.PlayedWithGesture.ChildFrame",
+          m_element->document().url());
+      Platform::current()->recordRapporURL(
+          "Media.Autoplay.CrossOriginExperiment.PlayedWithGesture."
+          "TopLevelFrame",
+          m_element->document().topDocument().url());
+      break;
+    default:
+      NOTREACHED();
+  }
+  m_recordedCrossOriginExperimentMetrics.insert(metric);

[whywhat, 2016/11/29 21:42:04]

nit: put this next to the check if the metric is already recorded since these
two are coupled logically?

[Zhiqiang Zhang (Slow), 2016/11/30 17:30:52]

Done.

+}
+
 void AutoplayUmaHelper::recordAutoplayUnmuteStatus(
     AutoplayUnmuteActionStatus status) {
   DEFINE_STATIC_LOCAL(