Web Audio: when media playback requires a user gesture, apply rule to cross origin iframes.

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

Index: third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp b/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
index 2d1e489aa55875d48d66576e229779f104f79649..c7fed11f6ea105e363c24dfbfaf1ffafcd434239 100644
--- a/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
@@ -33,7 +33,6 @@
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContextTask.h"
-#include "core/frame/Deprecation.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLMediaElement.h"
 #include "modules/mediastream/MediaStream.h"
@@ -76,18 +75,6 @@
 
 namespace blink {
 
-namespace {
-
-enum UserGestureRecord {
-    UserGestureRequiredAndAvailable = 0,
-    UserGestureRequiredAndNotAvailable,
-    UserGestureNotRequiredAndAvailable,
-    UserGestureNotRequiredAndNotAvailable,
-    UserGestureRecordMax
-};
-
-} // anonymous namespace
-
 BaseAudioContext* BaseAudioContext::create(Document& document, ExceptionState& exceptionState)
 {
     return AudioContext::create(document, exceptionState);
@@ -113,11 +100,12 @@ BaseAudioContext::BaseAudioContext(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())
+    // If mediaPlaybackRequiresUserGesture is enabled, cross origin iframes will
+    // require user gesture for the AudioContext to produce sound.
+    if (document->settings() && document->settings()->mediaPlaybackRequiresUserGesture()
+        && document->frame()) && document->frame()->isCrossOriginSubframe()) {
         m_userGestureRequired = true;
+    }
 
     m_destinationNode = DefaultAudioDestinationNode::create(this);
 
@@ -141,11 +129,12 @@ BaseAudioContext::BaseAudioContext(Document* document, unsigned numberOfChannels
     , 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())
+    // If mediaPlaybackRequiresUserGesture is enabled, cross origin iframes will
+    // require user gesture for the AudioContext to produce sound.
+    if (document->settings() && document->settings()->mediaPlaybackRequiresUserGesture()
+        && document->frame() && document->frame()->isCrossOriginSubframe()) {
         m_userGestureRequired = true;
+    }

[hongchan, 2016/09/07 16:59:00]

Blocking audio rendering should be limited to the realtime audio context because
OfflineAudioContext does not render the audio stream into the actual hardware,
so there is no reason to block it. I am not sure if this is discussed in the
working group discussion.

[mlamouri (slow - plz ping), 2016/09/08 13:12:42]

The goal here is to be compatible with Safari iOS and as far as I can tell, they
don't make any difference between regular and offline audio contexts. I'm not
sure why and that's why I prefer to be on the safe side and relax later. Though,
if you prefer to not do this, I can make this change.

 }
 
 BaseAudioContext::~BaseAudioContext()
@@ -543,30 +532,11 @@ PeriodicWave* BaseAudioContext::periodicWave(int type)
     }
 }
 
-void BaseAudioContext::recordUserGestureState()
+void BaseAudioContext::startRenderingIfNeeded()
 {
-    DEFINE_STATIC_LOCAL(EnumerationHistogram, userGestureHistogram, ("WebAudio.UserGesture", UserGestureRecordMax));
-
-    if (!m_userGestureRequired) {
-        if (UserGestureIndicator::processingUserGesture())
-            userGestureHistogram.count(UserGestureNotRequiredAndAvailable);
-        else
-            userGestureHistogram.count(UserGestureNotRequiredAndNotAvailable);
-        return;
-    }
-
-    DCHECK(m_userGestureRequired);
-    if (!UserGestureIndicator::processingUserGesture()) {
-        userGestureHistogram.count(UserGestureRequiredAndNotAvailable);
-
-        Document* document = toDocument(getExecutionContext());
-        if (document)
-            Deprecation::countDeprecationCrossOriginIframe(*document, UseCounter::WebAudioAutoplayCrossOriginIframe);
-
+    if (!m_userGestureRequired)
         return;
-    }
-    userGestureHistogram.count(UserGestureRequiredAndAvailable);
-    m_userGestureRequired = false;
+    startRendering();
 }
 
 String BaseAudioContext::state() const
@@ -701,6 +671,7 @@ void BaseAudioContext::handleStoppableSourceNodes()
 
 void BaseAudioContext::handlePreRenderTasks()
 {
+    LOG(INFO) << "handlePreRenderTasks";

[Raymond Toy, 2016/09/07 16:13:19]

handlePreRenderTask gets called about every 3 ms or less (depends on sample
rate).  Do you really want logged every time?

[mlamouri (slow - plz ping), 2016/09/08 13:12:42]

sorry, that's a leftover from some investigations.

     DCHECK(isAudioThread());
 
     // At the beginning of every render quantum, try to update the internal rendering graph state (from main thread changes).
@@ -813,7 +784,13 @@ void BaseAudioContext::startRendering()
     DCHECK(isMainThread());
     DCHECK(m_destinationNode);
 
-    recordUserGestureState();
+    if (m_userGestureRequired) {
+        if (!UserGestureIndicator::processingUserGesture()) {
+            UseCounter::count(getExecutionContext(), UseCounter::WebAudioAutoplayCrossOriginIframe);

[hongchan, 2016/09/07 16:59:00]

Can you add a comment on what this does?

This looks like it will always increment the "WebAudioAutoplayCrossOriginIframe"
count. Is this what you want? How do you know this method is called from a
cross-origin iframe? Perhaps I am missing something.

[mlamouri (slow - plz ping), 2016/09/08 13:12:42]

m_userGestureRequired only applies for cross origin iframes but again, I didn't
mean to leave this here either. Sent my CL a bit too early it seems.

+            return;
+        }
+        m_userGestureRequired = false;
+    }
 
     if (m_contextState == Suspended) {
         destination()->audioDestinationHandler().startRendering();