third_party/WebKit/Source/core/html/HTMLMediaElement.cpp - Issue 1470153004: Autoplay experiment metric fixes and additions.

Unified Diff: third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

Issue 1470153004: Autoplay experiment metric fixes and additions. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: moved RecordMetricsBehavior back into HTMLMediaElement Created 4 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« third_party/WebKit/Source/core/html/HTMLMediaElement.h ('K') | « third_party/WebKit/Source/core/html/HTMLMediaElement.h ('k') | tools/metrics/histograms/histograms.xml » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

index 31187d887237c852d3593574fe45e9f87b59c304..aae4f0aeef871e37bc97541090ddaea74fe9dd92 100644

--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

@@ -37,6 +37,7 @@

#include "core/dom/Fullscreen.h"

#include "core/dom/shadow/ShadowRoot.h"

#include "core/events/Event.h"

+#include "core/frame/FrameView.h"

#include "core/frame/LocalFrame.h"

#include "core/frame/Settings.h"

#include "core/frame/UseCounter.h"

@@ -253,6 +254,56 @@ bool canLoadURL(const KURL& url, const ContentType& contentType)

} // anonymous namespace

+class HTMLMediaElement::AutoplayHelperClientImpl :

sof 2016/02/25 18:55:43 Move this up into the anon namespace and declare i

liberato (no reviews please) 2016/03/14 17:09:13 can't without friending.

+ public AutoplayExperimentHelper::Client {

+public:

+ static PassOwnPtrWillBeRawPtr<AutoplayHelperClientImpl> create(HTMLMediaElement* element)

+ {

+ return adoptPtrWillBeNoop(new AutoplayHelperClientImpl(element));

+ }

+ virtual ~AutoplayHelperClientImpl();

+ using RecordMetricsBehavior = HTMLMediaElement::RecordMetricsBehavior;

+ double currentTime() const override { return m_element->currentTime(); }

+ double duration() const override { return m_element->duration(); }

+ bool paused() const override { return m_element->paused(); }

+ bool muted() const override { return m_element->muted(); }

+ void setMuted(bool muted) override { m_element->setMuted(muted); }

+ void playInternal() override { m_element->playInternal(); }

+ bool isUserGestureRequiredForPlay() const override { return m_element->isUserGestureRequiredForPlay(); }

+ void removeUserGestureRequirement() override { m_element->removeUserGestureRequirement(); }

+ void recordAutoplayMetric(AutoplayMetrics metric) override { m_element->recordAutoplayMetric(metric); }

+ bool shouldAutoplay() override

+ {

+ return m_element->shouldAutoplay(RecordMetricsBehavior::DoNotRecord);

+ }

+ bool isHTMLVideoElement() const override { return m_element->isHTMLVideoElement(); }

+ bool isHTMLAudioElement() const override { return m_element->isHTMLAudioElement(); }

+ // Document

+ bool isLegacyViewportType() override;

+ PageVisibilityState pageVisibilityState() const override;

+ String autoplayExperimentMode() const override;

+ // LayoutObject

+ void setRequestPositionUpdates(bool) override;

+ IntRect absoluteBoundingBoxRect() const override;

+ DEFINE_INLINE_VIRTUAL_TRACE() {

sof 2016/02/25 18:55:43 DEFINE_INLINE_VIRTUAL_TRACE() { ... }

liberato (no reviews please) 2016/03/14 17:09:13 Done.

+ visitor->trace(m_element);

+ Client::trace(visitor);

+ }

+private:

+ AutoplayHelperClientImpl(HTMLMediaElement* element) : m_element(element) {}

+ RawPtrWillBeMember<HTMLMediaElement> m_element;

+};

void HTMLMediaElement::recordAutoplayMetric(AutoplayMetrics metric)

{

DEFINE_STATIC_LOCAL(EnumerationHistogram, autoplayHistogram, ("Blink.MediaElement.Autoplay", NumberOfAutoplayMetrics));

@@ -341,14 +392,13 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum

, m_remoteRoutesAvailable(false)

, m_playingRemotely(false)

, m_isFinalizing(false)

- , m_initialPlayWithoutUserGesture(false)

- , m_autoplayMediaCounted(false)

, m_inOverlayFullscreenVideo(false)

, m_audioTracks(AudioTrackList::create(*this))

, m_videoTracks(VideoTrackList::create(*this))

, m_textTracks(nullptr)

, m_audioSourceNode(nullptr)

- , m_autoplayHelper(*this)

+ , m_autoplayHelperClient(AutoplayHelperClientImpl::create(this))

+ , m_autoplayHelper(AutoplayExperimentHelper::create(m_autoplayHelperClient))

{

#if ENABLE(OILPAN)

ThreadState::current()->registerPreFinalizer(this);

@@ -371,6 +421,11 @@ HTMLMediaElement::~HTMLMediaElement()

WTF_LOG(Media, "HTMLMediaElement::~HTMLMediaElement(%p)", this);

#if !ENABLE(OILPAN)

+ // Destruction of the autoplay helper requires the client, so be sure that

+ // this happens before the client is destructed.

+ if (m_autoplayHelper)

+ m_autoplayHelper.clear();

// HTMLMediaElement and m_asyncEventQueue always become unreachable

// together. So HTMLMediaElement and m_asyncEventQueue are destructed in

// the same GC. We don't need to close it explicitly in Oilpan.

@@ -410,6 +465,10 @@ HTMLMediaElement::~HTMLMediaElement()

#if ENABLE(OILPAN)

void HTMLMediaElement::dispose()

{

+ // This must happen before we're destructed.

+ if (m_autoplayHelper)

+ m_autoplayHelper->dispose();

// If the HTMLMediaElement dies with the Document we are not

// allowed to touch the Document to adjust delay load event counts

// from the destructor, as the Document could have been already

@@ -670,47 +729,11 @@ String HTMLMediaElement::canPlayType(const String& mimeType) const

return canPlay;

}

-void HTMLMediaElement::recordMetricsIfPausing()

- // If not playing, then nothing to record.

- // TODO(liberato): test metrics. this was m_paused.

- if (m_paused)

- return;

- const bool bailout = isBailout();

- // Record that play was paused. We don't care if it was autoplay,

- // play(), or the user manually started it.

- recordAutoplayMetric(AnyPlaybackPaused);

- if (bailout)

- recordAutoplayMetric(AnyPlaybackBailout);

- // If this was a gestureless play, then record that separately.

- // These cover attr and play() gestureless starts.

- if (m_initialPlayWithoutUserGesture) {

- m_initialPlayWithoutUserGesture = false;

- recordAutoplayMetric(AutoplayPaused);

- if (bailout)

- recordAutoplayMetric(AutoplayBailout);

- }

void HTMLMediaElement::load()

{

WTF_LOG(Media, "HTMLMediaElement::load(%p)", this);

- recordMetricsIfPausing();

- if (UserGestureIndicator::processingUserGesture() && m_userGestureRequiredForPlay) {

- recordAutoplayMetric(AutoplayEnabledThroughLoad);

- m_userGestureRequiredForPlay = false;

- // While usergesture-initiated load()s technically count as autoplayed,

- // they don't feel like such to the users and hence we don't want to

- // count them for the purposes of metrics.

- m_autoplayMediaCounted = true;

- }

+ m_autoplayHelper->loadMethodCalled();

prepareForLoad();

loadInternal();

@@ -931,6 +954,8 @@ void HTMLMediaElement::loadResource(const KURL& url, ContentType& contentType)

// The resource fetch algorithm

setNetworkState(NETWORK_LOADING);

+ m_autoplayHelper->loadingStarted();

// Set m_currentSrc *before* changing to the cache url, the fact that we are loading from the app

// cache is an internal detail not exposed through the media element API.

m_currentSrc = url;

@@ -957,7 +982,7 @@ void HTMLMediaElement::loadResource(const KURL& url, ContentType& contentType)

if (url.protocolIs(mediaSourceBlobProtocol)) {

if (isMediaStreamURL(url.string())) {

- m_userGestureRequiredForPlay = false;

+ m_autoplayHelper->removeUserGestureRequirement(GesturelessPlaybackEnabledByStream);

} else {

m_mediaSource = HTMLMediaSource::lookup(url.string());

@@ -1543,7 +1568,7 @@ void HTMLMediaElement::setReadyState(ReadyState state)

if (shouldAutoplay(RecordMetricsBehavior::DoRecord)) {

// If the autoplay experiment says that it's okay to play now,

// then don't require a user gesture.

- m_autoplayHelper.becameReadyToPlay();

+ m_autoplayHelper->becameReadyToPlay();

if (!m_userGestureRequiredForPlay) {

m_paused = false;

@@ -1682,8 +1707,6 @@ void HTMLMediaElement::seek(double time)

// 11 - Set the current playback position to the given new playback position.

webMediaPlayer()->seek(time);

- m_initialPlayWithoutUserGesture = false;

// 14-17 are handled, if necessary, when the engine signals a readystate change or otherwise

// satisfies seek completion and signals a time change.

}

@@ -1865,12 +1888,9 @@ bool HTMLMediaElement::autoplay() const

bool HTMLMediaElement::shouldAutoplay(const RecordMetricsBehavior recordMetrics)

{

if (m_autoplaying && m_paused && autoplay()) {

- if (recordMetrics == RecordMetricsBehavior::DoRecord)

- autoplayMediaEncountered();

if (document().isSandboxed(SandboxAutomaticFeatures)) {

if (recordMetrics == RecordMetricsBehavior::DoRecord)

- recordAutoplayMetric(AutoplayDisabledBySandbox);

+ m_autoplayHelper->recordSandboxFailure();

return false;

}

@@ -1945,21 +1965,19 @@ void HTMLMediaElement::play()

{

WTF_LOG(Media, "HTMLMediaElement::play(%p)", this);

- m_autoplayHelper.playMethodCalled();

+ m_autoplayHelper->playMethodCalled();

if (!UserGestureIndicator::processingUserGesture()) {

- autoplayMediaEncountered();

if (m_userGestureRequiredForPlay) {

recordAutoplayMetric(PlayMethodFailed);

String message = ExceptionMessages::failedToExecute("play", "HTMLMediaElement", "API can only be initiated by a user gesture.");

document().addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, message));

return;

}

- } else if (m_userGestureRequiredForPlay) {

- if (m_autoplayMediaCounted)

- recordAutoplayMetric(AutoplayManualStart);

- m_userGestureRequiredForPlay = false;

+ } else {

+ // We ask the helper to remove the gesture requirement for us, so that

+ // it can record the reason.

+ m_autoplayHelper->removeUserGestureRequirement(GesturelessPlaybackEnabledByPlayMethod);

}

playInternal();

@@ -2000,26 +2018,6 @@ void HTMLMediaElement::playInternal()

updatePlayState();

}

-void HTMLMediaElement::autoplayMediaEncountered()

- if (!m_autoplayMediaCounted) {

- m_autoplayMediaCounted = true;

- recordAutoplayMetric(AutoplayMediaFound);

- if (!m_userGestureRequiredForPlay)

- m_initialPlayWithoutUserGesture = true;

- }

-bool HTMLMediaElement::isBailout() const

- // We count the user as having bailed-out on the video if they watched

- // less than one minute and less than 50% of it.

- const double playedTime = currentTime();

- const double progress = playedTime / duration();

- return (playedTime < 60) && (progress < 0.5);

void HTMLMediaElement::pause()

{

WTF_LOG(Media, "HTMLMediaElement::pause(%p)", this);

@@ -2039,13 +2037,11 @@ void HTMLMediaElement::pauseInternal()

if (m_networkState == NETWORK_EMPTY)

scheduleDelayedAction(LoadMediaResource);

- m_autoplayHelper.pauseMethodCalled();

+ m_autoplayHelper->pauseMethodCalled();

m_autoplaying = false;

if (!m_paused) {

- recordMetricsIfPausing();

m_paused = true;

scheduleTimeupdateEvent(false);

scheduleEvent(EventTypeNames::pause);

@@ -2150,7 +2146,7 @@ void HTMLMediaElement::setMuted(bool muted)

m_muted = muted;

- m_autoplayHelper.mutedChanged();

+ m_autoplayHelper->mutedChanged();

updateVolume();

@@ -2746,6 +2742,7 @@ void HTMLMediaElement::timeChanged()

// forwards, and paused is false,

if (!m_paused) {

// changes paused to true and fires a simple event named pause at the media element.

+ m_autoplayHelper->playbackEnded();

m_paused = true;

scheduleEvent(EventTypeNames::pause);

}

@@ -2754,7 +2751,6 @@ void HTMLMediaElement::timeChanged()

m_sentEndEvent = true;

scheduleEvent(EventTypeNames::ended);

}

- recordMetricsIfPausing();

}

} else {

m_sentEndEvent = false;

@@ -2961,17 +2957,18 @@ void HTMLMediaElement::updatePlayState()

webMediaPlayer()->setRate(playbackRate());

updateVolume();

webMediaPlayer()->play();

+ m_autoplayHelper->playbackStarted();

}

if (mediaControls())

mediaControls()->playbackStarted();

startPlaybackProgressTimer();

m_playing = true;

- recordAutoplayMetric(AnyPlaybackStarted);

} else { // Should not be playing right now

if (isPlaying)

webMediaPlayer()->pause();

refreshCachedTime();

m_playbackProgressTimer.stop();

@@ -3039,8 +3036,6 @@ void HTMLMediaElement::stop()

{

WTF_LOG(Media, "HTMLMediaElement::stop(%p)", this);

- recordMetricsIfPausing();

// Close the async event queue so that no events are enqueued.

cancelPendingEventsAndCallbacks();

m_asyncEventQueue->close();

@@ -3514,8 +3509,9 @@ DEFINE_TRACE(HTMLMediaElement)

visitor->trace(m_textTracks);

visitor->trace(m_textTracksWhenResourceSelectionBegan);

visitor->trace(m_audioSourceProvider);

- visitor->template registerWeakMembers<HTMLMediaElement, &HTMLMediaElement::clearWeakMembers>(this);

+ visitor->trace(m_autoplayHelperClient);

visitor->trace(m_autoplayHelper);

+ visitor->template registerWeakMembers<HTMLMediaElement, &HTMLMediaElement::clearWeakMembers>(this);

HeapSupplementable<HTMLMediaElement>::trace(visitor);

#endif

HTMLElement::trace(visitor);

@@ -3560,11 +3556,6 @@ void HTMLMediaElement::removeUserGestureRequirement()

m_userGestureRequiredForPlay = false;

}

-void HTMLMediaElement::setInitialPlayWithoutUserGestures(bool value)

- m_initialPlayWithoutUserGesture = value;

void HTMLMediaElement::setNetworkState(NetworkState state)

{

if (m_networkState != state) {

@@ -3576,18 +3567,20 @@ void HTMLMediaElement::setNetworkState(NetworkState state)

void HTMLMediaElement::notifyPositionMayHaveChanged(const IntRect& visibleRect)

{

- m_autoplayHelper.positionChanged(visibleRect);

+ m_autoplayHelper->positionChanged(visibleRect);

}

void HTMLMediaElement::updatePositionNotificationRegistration()

{

- m_autoplayHelper.updatePositionNotificationRegistration();

+ m_autoplayHelper->updatePositionNotificationRegistration();

}

// TODO(liberato): remove once autoplay gesture override experiment concludes.

void HTMLMediaElement::triggerAutoplayViewportCheckForTesting()

{

- m_autoplayHelper.triggerAutoplayViewportCheckForTesting();

+ if (FrameView* view = document().view())

+ m_autoplayHelper->positionChanged(view->rootFrameToContents(view->computeVisibleArea()));

+ m_autoplayHelper->triggerAutoplayViewportCheckForTesting();

}

void HTMLMediaElement::clearWeakMembers(Visitor* visitor)

@@ -3656,4 +3649,43 @@ DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)

visitor->trace(m_client);

}

+HTMLMediaElement::AutoplayHelperClientImpl::~AutoplayHelperClientImpl()

sof 2016/02/25 18:55:43 nit: it's ok this way, but defining these shorter

+bool HTMLMediaElement::AutoplayHelperClientImpl::isLegacyViewportType()

+ return m_element->document().viewportDescription().isLegacyViewportType();

+PageVisibilityState HTMLMediaElement::AutoplayHelperClientImpl::pageVisibilityState() const

+ return m_element->document().pageVisibilityState();

+String HTMLMediaElement::AutoplayHelperClientImpl::autoplayExperimentMode() const

+ String mode;

+ if (m_element->document().settings())

+ mode = m_element->document().settings()->autoplayExperimentMode();

+ return mode;

+void HTMLMediaElement::AutoplayHelperClientImpl::setRequestPositionUpdates(bool request)

+ if (LayoutObject* layoutObject = m_element->layoutObject()) {

+ LayoutMedia* layoutMedia = toLayoutMedia(layoutObject);

+ layoutMedia->setRequestPositionUpdates(request);

+ }

+IntRect HTMLMediaElement::AutoplayHelperClientImpl::absoluteBoundingBoxRect() const

+ IntRect result;

+ if (LayoutObject* object = m_element->layoutObject())

+ result = object->absoluteBoundingBoxRect();

+ return result;

} // namespace blink