WebAudio: record whether user gesture was available and provided.

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

Index: third_party/WebKit/Source/modules/webaudio/AbstractAudioContext.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/AbstractAudioContext.cpp b/third_party/WebKit/Source/modules/webaudio/AbstractAudioContext.cpp
index de36a35f9e0956e38df02b948b4fc8e3c06f7948..e6945ceb7d611d8803827981b7d72c19d9e4ab83 100644
--- a/third_party/WebKit/Source/modules/webaudio/AbstractAudioContext.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/AbstractAudioContext.cpp
@@ -32,6 +32,7 @@
 #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"
@@ -64,13 +65,27 @@
 #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);
@@ -86,6 +101,7 @@ AbstractAudioContext::AbstractAudioContext(Document* document)
     , m_destinationNode(nullptr)
     , m_isCleared(false)
     , m_isResolvingResumePromises(false)
+    , m_userGestureRequired(true)
     , m_connectionCount(0)
     , m_deferredTaskHandler(DeferredTaskHandler::create())
     , m_contextState(Suspended)
@@ -95,6 +111,12 @@ AbstractAudioContext::AbstractAudioContext(Document* document)
     , 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();
@@ -102,19 +124,7 @@ AbstractAudioContext::AbstractAudioContext(Document* document)
 
 // 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.

 {
 }
 
@@ -473,6 +483,25 @@ PeriodicWave* AbstractAudioContext::periodicWave(int type)
     }
 }
 
+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.
@@ -717,6 +746,8 @@ void AbstractAudioContext::startRendering()
     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);