Adding an experimental flag to block autoplay with sound in cross-origin iframes

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

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 1d90b0d8fedb9d495cd032a65a8c456d2a276532..7045eea94250d6ad31a307c6dc8f2ce020df35ef 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -282,6 +282,11 @@ String preloadTypeToString(WebMediaPlayer::Preload preloadType) {
   return String();
 }
 
+bool isCrossOrigin(Document& document) {
+  const LocalFrame* frame = document.frame();
+  return frame && frame->isCrossOriginSubframe();
+}
+
 }  // anonymous namespace
 
 class HTMLMediaElement::AutoplayHelperClientImpl
@@ -471,6 +476,9 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName,
   // default, otherwise the experiment does nothing.
   if ((document.settings() &&
        document.settings()->mediaPlaybackRequiresUserGesture()) ||
+      (document.settings() && isCrossOrigin(document) &&

[whywhat, 2016/11/16 17:37:17]

is it this that you don't like? you could've had a helper function that takes a
document and returns true if it requires user gesture for 

bool HTMLMediaElement::documentRequiresUserGesture(const Document& document) {
  // might be a good time to remove this? :)
  if (m_autoplayHelper->isExperimentEnabled()) return true;

  if (!document.settings()) return false;

  return document.settings()->mediaPlaybackRequiresUserGesture() ||
      (document.settings()->mediaPlaybackinCORequiresUserGesture() &&
isCrossOrigin(document));
}

[Zhiqiang Zhang (Slow), 2016/11/16 18:11:40]

Already merged them into a helper function in my local patch :)

I'm more concerned about the plumbing, i.e. plumbing another boolean flag from
chrome://flags to WebKit Settings. It lacks some flexibility in case if we have
more variations for autoplay. Also, currently we are having
"MediaPlaybackRequiresUserGesture" and
"MediaPlaybackInCrossOriginIframesRequiresUserGesture" side-by-side in Blink
neglect of which platform we are on, while they do not co-exist in content/
because of the #ifdef macros. This would bring some confusion.

So what I'm thinking is to make "MediaPlaybackRequiresUserGesture" available to
all platforms, and have an optional string flag indicating the autoplay
variations. WDYT?

+       document.settings()
+           ->mediaPlaybackInCrossOriginIframesRequiresUserGesture()) ||
       m_autoplayHelper->isExperimentEnabled()) {
     m_lockedPendingUserGesture = true;
   }
@@ -519,10 +527,17 @@ void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument) {
   bool oldDocumentRequiresUserGesture =
       (oldDocument.settings() &&
        oldDocument.settings()->mediaPlaybackRequiresUserGesture()) ||
+      (oldDocument.settings() && isCrossOrigin(oldDocument) &&
+       oldDocument.settings()
+           ->mediaPlaybackInCrossOriginIframesRequiresUserGesture()) ||
       m_autoplayHelper->isExperimentEnabled();
   bool newDocumentRequiresUserGesture =
       (document().settings() &&
        document().settings()->mediaPlaybackRequiresUserGesture()) ||
+      (document().settings() && isCrossOrigin(oldDocument) &&
+       document()
+           .settings()
+           ->mediaPlaybackInCrossOriginIframesRequiresUserGesture()) ||
       m_autoplayHelper->isExperimentEnabled();
   if (newDocumentRequiresUserGesture && !oldDocumentRequiresUserGesture) {
     m_lockedPendingUserGesture = true;