WebAudio: record whether user gesture was available and provided.

third_party/WebKit/Source/modules/webaudio/AbstractAudioContext.cpp

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 14 matching lines @@
 #include "modules/webaudio/AbstractAudioContext.h"
 #include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ExceptionMessages.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContextTask.h"
+#include "core/frame/Settings.h"
 #include "core/html/HTMLMediaElement.h"
 #include "modules/mediastream/MediaStream.h"
 #include "modules/webaudio/AnalyserNode.h"
 #include "modules/webaudio/AudioBuffer.h"
 #include "modules/webaudio/AudioBufferCallback.h"
 #include "modules/webaudio/AudioBufferSourceNode.h"
 #include "modules/webaudio/AudioContext.h"
 #include "modules/webaudio/AudioListener.h"
 #include "modules/webaudio/AudioNodeInput.h"
 #include "modules/webaudio/AudioNodeOutput.h"
@@ skipping 12 matching lines @@
 #include "modules/webaudio/OfflineAudioCompletionEvent.h"
 #include "modules/webaudio/OfflineAudioContext.h"
 #include "modules/webaudio/OfflineAudioDestinationNode.h"
 #include "modules/webaudio/OscillatorNode.h"
 #include "modules/webaudio/PannerNode.h"
 #include "modules/webaudio/PeriodicWave.h"
 #include "modules/webaudio/PeriodicWaveConstraints.h"
 #include "modules/webaudio/ScriptProcessorNode.h"
 #include "modules/webaudio/StereoPannerNode.h"
 #include "modules/webaudio/WaveShaperNode.h"
+#include "platform/Histogram.h"
 #include "platform/ThreadSafeFunctional.h"
+#include "platform/UserGestureIndicator.h"
 #include "platform/audio/IIRFilter.h"
 #include "public/platform/Platform.h"
 #include "wtf/text/WTFString.h"
 
 namespace blink {
 
+namespace {
+
+enum UserGestureRecord {
+    UserGestureRequiredAndAvailable = 0,
+    UserGestureRequiredAndNotAvailable,
+    UserGestureNotRequiredAndAvailable,
+    UserGestureNotRequiredAndNotAvailable,
+    UserGestureRecordMax
+};
+
+} // anonymous namespace
+
 AbstractAudioContext* AbstractAudioContext::create(Document& document, ExceptionState& exceptionState)
 {
     return AudioContext::create(document, exceptionState);
 }
 
 // FIXME(dominicc): Devolve these constructors to AudioContext
 // and OfflineAudioContext respectively.
 
 // Constructor for rendering to the audio hardware.
 AbstractAudioContext::AbstractAudioContext(Document* document)
     : ActiveScriptWrappable(this)
     , ActiveDOMObject(document)
     , m_destinationNode(nullptr)
     , m_isCleared(false)
     , m_isResolvingResumePromises(false)
+    , m_userGestureRequired(true)
     , m_connectionCount(0)
     , m_deferredTaskHandler(DeferredTaskHandler::create())
     , m_contextState(Suspended)
     , m_closedContextSampleRate(-1)
     , m_periodicWaveSine(nullptr)
     , m_periodicWaveSquare(nullptr)
     , m_periodicWaveSawtooth(nullptr)
     , m_periodicWaveTriangle(nullptr)
 {
+    // TODO(mlamouri): we might want to use other ways of checking for this but
+    // in order to record metrics, re-using the HTMLMediaElement setting is
+    // probably the simplest solution.
+    if (document->settings() && document->settings()->mediaPlaybackRequiresUserGesture())
+        m_userGestureRequired = true;

[Raymond Toy, 2016/06/03 17:57:52]

You initialized m_userGestureRequired to true in line 184.  Did you mean false
there?  Otherwise these lines do nothing.

[mlamouri (slow - plz ping), 2016/06/05 14:44:57]

I meant `false` above. I changed before uploading to test that things were
working. The issue of sending a CL on Friday evening before leaving the office
:)

+
     m_destinationNode = DefaultAudioDestinationNode::create(this);
 
     initialize();
 }
 
 // Constructor for offline (non-realtime) rendering.
 AbstractAudioContext::AbstractAudioContext(Document* document, unsigned numberOfChannels, size_t numberOfFrames, float sampleRate)
-    : ActiveScriptWrappable(this)
-    , ActiveDOMObject(document)
-    , m_destinationNode(nullptr)
-    , m_isCleared(false)
-    , m_isResolvingResumePromises(false)
-    , m_connectionCount(0)
-    , m_deferredTaskHandler(DeferredTaskHandler::create())
-    , m_contextState(Suspended)
-    , m_closedContextSampleRate(-1)
-    , m_periodicWaveSine(nullptr)
-    , m_periodicWaveSquare(nullptr)
-    , m_periodicWaveSawtooth(nullptr)
-    , m_periodicWaveTriangle(nullptr)
+    : AbstractAudioContext(document)

[Raymond Toy, 2016/06/03 17:57:52]

Why the change? It seems that it nothing to do with this CL, so I'd prefer if
this change were not made.

[mlamouri (slow - plz ping), 2016/06/05 14:44:57]

Instead of adding yet another init in there, I think we should re-use the ctor
above. Though, put the stuff back as you asked.

[Raymond Toy, 2016/06/06 15:10:07]

Thanks.  We'll do this fix some other day; it makes sense to re-use since
there's nothing different.

 {
 }
 
 AbstractAudioContext::~AbstractAudioContext()
 {
     deferredTaskHandler().contextWillBeDestroyed();
     // AudioNodes keep a reference to their context, so there should be no way to be in the destructor if there are still AudioNodes around.
     ASSERT(!isDestinationInitialized());
     ASSERT(!m_activeSourceNodes.size());
     ASSERT(!m_finishedSourceHandlers.size());
@@ skipping 338 matching lines @@
         // Initialize the table if necessary
         if (!m_periodicWaveTriangle)
             m_periodicWaveTriangle = PeriodicWave::createTriangle(sampleRate());
         return m_periodicWaveTriangle;
     default:
         ASSERT_NOT_REACHED();
         return nullptr;
     }
 }
 
+void AbstractAudioContext::recordUserGesture()

[Raymond Toy, 2016/06/03 17:57:52]

I think recordUserGestureState() is a better name.  When I read the current
name, I think you are recording a gesture from the user.

[mlamouri (slow - plz ping), 2016/06/05 14:44:57]

Done.

+{
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, userGestureHistogram, ("WebAudio.UserGesture", UserGestureRecordMax));
+
+    if (!m_userGestureRequired) {
+        if (UserGestureIndicator::processingUserGesture())
+            userGestureHistogram.count(UserGestureNotRequiredAndAvailable);
+        else
+            userGestureHistogram.count(UserGestureNotRequiredAndNotAvailable);
+        return;
+    }
+    if (!UserGestureIndicator::processingUserGesture()) {
+        userGestureHistogram.count(UserGestureRequiredAndNotAvailable);
+        return;
+    }
+    userGestureHistogram.count(UserGestureRequiredAndAvailable);
+    m_userGestureRequired = false;
+}
+
 String AbstractAudioContext::state() const
 {
     // These strings had better match the strings for AudioContextState in AudioContext.idl.
     switch (m_contextState) {
     case Suspended:
         return "suspended";
     case Running:
         return "running";
     case Closed:
         return "closed";
@@ skipping 224 matching lines @@
 {
     return ActiveDOMObject::getExecutionContext();
 }
 
 void AbstractAudioContext::startRendering()
 {
     // This is called for both online and offline contexts.
     ASSERT(isMainThread());
     ASSERT(m_destinationNode);
 
+    recordUserGesture();

[Raymond Toy, 2016/06/03 17:57:52]

Can you explain why this is here?  startRendering() is called basically when the
context is created.

And you very probably don't want to call this for an offline context since no
audio is ever played out in that case.

[mlamouri (slow - plz ping), 2016/06/05 14:44:57]

This is basically added to places where WebKit does check for user gesture. The
intent here being that we want to make sure that having an implementation
following WebKit would not break websites (because of UA sniffing). WebKit will
not allow an AudioContext to start unless it was "unlocked". I think the main
reason is for ScriptProcessor but it might be for other reasons.

Regarding offline contexts, I don't think WebKit does anything specific (based
on the code). Would you have a test page for offline audio context?

[Raymond Toy, 2016/06/06 15:10:07]

Ok, it makes some sense to do the same.
What kind of test?  Almost all of the webaudio layout tests in
third_party/WebKit/LayoutTests/webaudio use offline contexts.  Almost all can
just be loaded up and run in a browser.

+
     if (m_contextState == Suspended) {
         destination()->audioDestinationHandler().startRendering();
         setContextState(Running);
     }
 }
 
 DEFINE_TRACE(AbstractAudioContext)
 {
     visitor->trace(m_destinationNode);
     visitor->trace(m_listener);
@@ skipping 11 matching lines @@
 
 SecurityOrigin* AbstractAudioContext::getSecurityOrigin() const
 {
     if (getExecutionContext())
         return getExecutionContext()->getSecurityOrigin();
 
     return nullptr;
 }
 
 } // namespace blink