Measure whether muted videos that started playing with play() become visible at some point

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
new file mode 100644
index 0000000000000000000000000000000000000000..ddfc743dca8e9c57f8d9e2766c4165439dda90eb
--- /dev/null
+++ b/third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp
@@ -0,0 +1,120 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/html/AutoplayUmaHelper.h"
+
+#include "core/dom/ElementVisibilityObserver.h"
+#include "core/events/Event.h"
+#include "core/html/HTMLMediaElement.h"
+#include "platform/Histogram.h"
+
+namespace blink {
+
+namespace {
+
+void recordVideoAutoplayMutedPlayMethodBecomesVisibleUma(bool visible)
+{
+    DEFINE_STATIC_LOCAL(BooleanHistogram, histogram, ("Media.Video.Autoplay.Muted.PlayMethod.BecomesVisible"));
+    histogram.count(visible);
+}
+
+void recordAutoplaySourceUma(HTMLMediaElement* element, AutoplaySource source)
+{
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, videoHistogram, ("Media.Video.Autoplay", static_cast<int>(AutoplaySource::NumberOfSources)));
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, mutedVideoHistogram, ("Media.Video.Autoplay.Muted", static_cast<int>(AutoplaySource::NumberOfSources)));
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, audioHistogram, ("Media.Audio.Autoplay", static_cast<int>(AutoplaySource::NumberOfSources)));
+
+    if (element->isHTMLVideoElement()) {
+        videoHistogram.count(static_cast<int>(source));
+        if (element->muted())
+            mutedVideoHistogram.count(static_cast<int>(source));
+    } else {
+        audioHistogram.count(static_cast<int>(source));
+    }
+}
+
+void recordAutoplayUnmuteStatus(AutoplayUnmuteActionStatus status)
+{
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, autoplayUnmuteHistogram, ("Media.Video.Autoplay.Muted.UnmuteAction", static_cast<int>(AutoplayUnmuteActionStatus::NumberOfStatus)));
+
+    autoplayUnmuteHistogram.count(static_cast<int>(status));
+}
+
+} // namespace
+
+AutoplayUmaHelper* AutoplayUmaHelper::create(HTMLMediaElement* element)
+{
+    return new AutoplayUmaHelper(element);
+}
+
+AutoplayUmaHelper::AutoplayUmaHelper(HTMLMediaElement* element)
+    : EventListener(CPPEventListenerType)
+    , m_source(AutoplaySource::NumberOfSources)
+    , m_element(element)
+    , m_videoMutedPlayMethodVisibilityObserver(nullptr)
+{
+m_element->addEventListener(EventTypeNames::playing, this, false);

[haraken, 2016/07/07 01:16:36]

Nit: Add indent.

[haraken, 2016/07/07 01:16:36]

Wouldn't this cause a flood of events? (i.e., how frequently will the event
listeners get fired?)

[mlamouri (slow - plz ping), 2016/07/07 09:23:40]

The event is fired when the media element starts playback. For example, after
`play()` is called, `playing` event will fire when the loading is done. If the
users pauses then resume, the event will be fired again. I don't think it would
be too much.

[Zhiqiang Zhang (Slow), 2016/07/07 11:01:09]

Done adding indent.

+}
+
+AutoplayUmaHelper::~AutoplayUmaHelper() = default;
+
+bool AutoplayUmaHelper::operator==(const EventListener& other) const
+{
+    return this == &other;
+}
+
+void AutoplayUmaHelper::onAutoplayInitiated(AutoplaySource source)
+{
+    m_source = source;
+    recordAutoplaySourceUma(m_element, m_source);

[mlamouri (slow - plz ping), 2016/07/07 10:17:52]

That might be a silly question but why not move the content of the anonymous
method in there?

[Zhiqiang Zhang (Slow), 2016/07/07 11:01:09]

Done.
I thought it would help if we record the same from different call sites.
Since there's only one call site, it's good to move the body here :)

+}

[mlamouri (slow - plz ping), 2016/07/07 10:17:52]

Also, would it make sense to do the addEventListener() here if m_source is
method so we only listen when needed?

[Zhiqiang Zhang (Slow), 2016/07/07 11:01:09]

Done.

+
+void AutoplayUmaHelper::onElementDestroyed()
+{
+    if (m_videoMutedPlayMethodVisibilityObserver) {
+        recordVideoAutoplayMutedPlayMethodBecomesVisibleUma(false);
+        m_videoMutedPlayMethodVisibilityObserver->stop();
+        m_videoMutedPlayMethodVisibilityObserver = nullptr;
+    }
+}
+
+void AutoplayUmaHelper::recordAutoplayUnmuteStatus(AutoplayUnmuteActionStatus status)
+{
+    blink::recordAutoplayUnmuteStatus(status);

[mlamouri (slow - plz ping), 2016/07/07 10:17:52]

That might be a silly question but why not move the content of the anonymous
method in there?

[Zhiqiang Zhang (Slow), 2016/07/07 11:01:09]

Done.

+}
+
+void AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod(bool isVisible)
+{
+    if (!isVisible)
+        return;
+
+    recordVideoAutoplayMutedPlayMethodBecomesVisibleUma(true);
+    m_videoMutedPlayMethodVisibilityObserver->stop();
+    m_videoMutedPlayMethodVisibilityObserver = nullptr;
+}
+
+void AutoplayUmaHelper::handleEvent(ExecutionContext* executionContext, Event* event)
+{
+    if (event->type() == EventTypeNames::playing)

[mlamouri (slow - plz ping), 2016/07/07 10:17:52]

What about:
```
switch (event->type()) {
  case EventTypeNames::playing:
    handlePlayingEvent();
    break;
  default:
    ASSERT_NOT_REACHED();
}
``` 

[Zhiqiang Zhang (Slow), 2016/07/07 11:01:09]

Actually event->type() is AtomicString XD

+        handlePlayEvent();
+}
+
+void AutoplayUmaHelper::handlePlayEvent()
+{
+    if (m_source == AutoplaySource::Method && m_element->isHTMLVideoElement() && m_element->muted()) {
+        if (!m_videoMutedPlayMethodVisibilityObserver) {
+            m_videoMutedPlayMethodVisibilityObserver = new ElementVisibilityObserver(m_element, WTF::bind(&AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod, wrapPersistent(this)));
+            m_videoMutedPlayMethodVisibilityObserver->start();
+        }
+    }

[mlamouri (slow - plz ping), 2016/07/07 10:17:53]

Would it make sense to removeEventListener()?

[Zhiqiang Zhang (Slow), 2016/07/07 11:01:09]

Done.

+}
+
+DEFINE_TRACE(AutoplayUmaHelper)
+{
+    EventListener::trace(visitor);
+    visitor->trace(m_element);
+    visitor->trace(m_videoMutedPlayMethodVisibilityObserver);
+}
+
+} // namespace blink