Allow autoplay unmuted for WebAPK in the manifest scope

Allow autoplay unmuted for WebAPK in the manifest scope

Previously, we block all autoplay with sound on Android. To close
the gap between Web apps and native apps, this CL uses the
manifest scope as a whitelist, so that frames matching the scope
are allowed to autoplay with sound.

Design doc:
https://docs.google.com/a/google.com/document/d/12kghzV5FBk4JNgtOJk2pTGqNhMzNmXt2GEpstJ-4tHc/edit?usp=sharing

BUG=676312
Review-Url: https://codereview.chromium.org/2640143002
Cr-Commit-Position: refs/heads/master@{#450727}
Committed: https://chromium.googlesource.com/chromium/src/+/183dc82ce88ae08056adfcc97e48937c0fe1be59

[Zhiqiang Zhang (Slow), 2017-02-08 18:12:03]

Description was changed from

==========
[WIP] Allow autoplay unmuted for WebAPK

BUG=676312
==========

to

==========
Allow autoplay unmuted for WebAPK in the manifest scope

Previously, we block all autoplay with sound on Android. To close
the gap between Web apps and native apps, this CL uses the
manifest scope as a whitelist, so that frames matching the scope
are allowed to autoplay with sound.

Design doc:
https://docs.google.com/a/google.com/document/d/12kghzV5FBk4JNgtOJk2pTGqNhMzN...

BUG=676312
==========

[Zhiqiang Zhang (Slow), 2017-02-08 18:13:21]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 18:13:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 18:46:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 18:46:06]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Zhiqiang Zhang (Slow), 2017-02-08 18:56:50]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 18:57:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 19:26:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 19:26:02]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Zhiqiang Zhang (Slow), 2017-02-09 16:31:40]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-09 16:32:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 18:20:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-09 18:20:03]

Dry run: This issue passed the CQ dry run.

[Zhiqiang Zhang (Slow), 2017-02-09 21:36:35]

zqzhang@chromium.org changed reviewers:
+ jochen@chromium.org, tedchoc@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-09 21:36:36]

jochen@, tedchoc@: can you take an initial look at the design doc?

The main technical issue of this CL is how to pass specific WebPreferences to a
specific renderer. Are we doing in the right way?

[jochen (gone - plz use gerrit), 2017-02-10 09:06:45]

jochen@chromium.org changed reviewers:
+ mlamouri@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-10 11:04:02]

zqzhang@chromium.org changed reviewers:
+ hanxi@chromium.org

[mlamouri (slow - plz ping), 2017-02-10 14:28:46]

My only concern here is the '*'. Everything else looks great :)

https://codereview.chromium.org/2640143002/diff/80001/content/renderer/render...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/2640143002/diff/80001/content/renderer/render...
content/renderer/render_view_impl.cc:602: #endif
Instead of having `#if defined`, could we do:
```
if (command_line.HasSwitch(switches::kMediaPlayerGestureWhitelistScope)) {
  webview()->settings()->setMediaPlaybackGestureWhitelistScope(
      blink::WebString::fromASCII(command_line.GetSwitchValueASCII(
          switches::kMediaPlaybackGestureWhitelistScope)));
}
```

It will keep the feature multi-platform for those that might be interested to
use it.

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/autoplay-non-whitelisted-scope.html
(right):

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/autoplay-non-whitelisted-scope.html:41: var
expectedEvents = [ 'canplay'];
style: ['canplay'] or [ 'canplay' ] :)

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:300: return true;
How can that happen?

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:313:
document.settings()->getMediaPlaybackGestureWhitelistScope())) {
I guess you could just pass `document` and get the settings from the document
inside the method? You could even add an DCHECK() that the settings is
available.

[Zhiqiang Zhang (Slow), 2017-02-10 14:47:11]

mlamouri: PTAL

https://codereview.chromium.org/2640143002/diff/80001/content/renderer/render...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/2640143002/diff/80001/content/renderer/render...
content/renderer/render_view_impl.cc:602: #endif
On 2017/02/10 14:28:45, mlamouri wrote:
> Instead of having `#if defined`, could we do:
> ```
> if (command_line.HasSwitch(switches::kMediaPlayerGestureWhitelistScope)) {
>   webview()->settings()->setMediaPlaybackGestureWhitelistScope(
>       blink::WebString::fromASCII(command_line.GetSwitchValueASCII(
>           switches::kMediaPlaybackGestureWhitelistScope)));
> }
> ```
> 
> It will keep the feature multi-platform for those that might be interested to
> use it.

Done.

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/autoplay-non-whitelisted-scope.html
(right):

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/autoplay-non-whitelisted-scope.html:41: var
expectedEvents = [ 'canplay'];
On 2017/02/10 14:28:45, mlamouri wrote:
> style: ['canplay'] or [ 'canplay' ] :)

Done.

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:300: return true;
On 2017/02/10 14:28:46, mlamouri wrote:
> How can that happen?

Ah, I was thinking of when the manifest is unscoped. However actually WebAPK
trims the start_url for the scope if the manifest is unscoped. That says WebAPKs
are always scoped.

https://codereview.chromium.org/2640143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:313:
document.settings()->getMediaPlaybackGestureWhitelistScope())) {
On 2017/02/10 14:28:46, mlamouri wrote:
> I guess you could just pass `document` and get the settings from the document
> inside the method? You could even add an DCHECK() that the settings is
> available.

Done.

[Zhiqiang Zhang (Slow), 2017-02-10 14:47:31]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-10 14:47:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 15:38:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-10 15:38:34]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Xi Han, 2017-02-10 18:02:42]

https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
(right):

https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:188:
additionalCommandLine.add("--" +
ContentSwitches.MEDIA_PLAYBACK_GESTURE_WHITELIST_SCOPE
Do you miss a "=" between the flag and the value?

https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java
(right):

https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java:203:
for (String param : mCommandLineParams) fullCommandLine.add(param);
It's better to keep the style consistent with lines 210 and 212:
 for (...) {
   statement;
 }

[Ted C, 2017-02-10 18:55:30]

On 2017/02/10 18:02:42, Xi Han wrote:
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
> (right):
> 
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:188:
> additionalCommandLine.add("--" +
> ContentSwitches.MEDIA_PLAYBACK_GESTURE_WHITELIST_SCOPE
> Do you miss a "=" between the flag and the value?
> 
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> File
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java
> (right):
> 
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java:203:
> for (String param : mCommandLineParams) fullCommandLine.add(param);
> It's better to keep the style consistent with lines 210 and 212:
>  for (...) {
>    statement;
>  }

I have the same general question about this approach as:
https://bugs.chromium.org/p/chromium/issues/detail?id=611842

The creation of renderer processes is controlled by the security
policy.  That is an area that I'm not familiar with.  Before we
start landing code that lightens the security restrictions for
processes, we need to verify that there is no way a webapk will
share an existing renderer with anything else.

I know that at some point in the past we would share a renderer
under "some" circumstances, but maybe those are no longer the
case.

Instead of using command line, I'm wondering if we could do the
work here:
ChromeContentBrowserClient::OverrideWebkitPrefs

From RenderViewHost, we could potentially get out to Android's
tab via WebContents::FromRenderViewHost (but maybe the RVH
here is created prior to being attached to a webcontents).

But before we go the path of using command line params, I think
we need to be 100% positive that renderer's aren't shared under
any circumstances (i'd ask nasko@ or creis@ for that).

[Xi Han, 2017-02-10 20:30:17]

On 2017/02/10 18:55:30, Ted C wrote:
> On 2017/02/10 18:02:42, Xi Han wrote:
> >
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> > File
> >
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:188:
> > additionalCommandLine.add("--" +
> > ContentSwitches.MEDIA_PLAYBACK_GESTURE_WHITELIST_SCOPE
> > Do you miss a "=" between the flag and the value?
> > 
> >
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> > File
> >
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> >
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java:203:
> > for (String param : mCommandLineParams) fullCommandLine.add(param);
> > It's better to keep the style consistent with lines 210 and 212:
> >  for (...) {
> >    statement;
> >  }
> 
> I have the same general question about this approach as:
> https://bugs.chromium.org/p/chromium/issues/detail?id=611842
> 
> The creation of renderer processes is controlled by the security
> policy.  That is an area that I'm not familiar with.  Before we
> start landing code that lightens the security restrictions for
> processes, we need to verify that there is no way a webapk will
> share an existing renderer with anything else.
> 
> I know that at some point in the past we would share a renderer
> under "some" circumstances, but maybe those are no longer the
> case.
Before crbug.com/664530 is fixed, we will still in the same situation.

> Instead of using command line, I'm wondering if we could do the
> work here:
> ChromeContentBrowserClient::OverrideWebkitPrefs
> 
> From RenderViewHost, we could potentially get out to Android's
> tab via WebContents::FromRenderViewHost (but maybe the RVH
> here is created prior to being attached to a webcontents).
> 
> But before we go the path of using command line params, I think
> we need to be 100% positive that renderer's aren't shared under
> any circumstances (i'd ask nasko@ or creis@ for that).
Totally agree, it would be great if we don't need to use the
ChildProcessCreationParams at all.

[mlamouri (slow - plz ping), 2017-02-13 12:03:15]

lgtm, assuming content owners are happy with the change

https://codereview.chromium.org/2640143002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2640143002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:315: }
style: drop the { }

[Zhiqiang Zhang (Slow), 2017-02-13 12:13:28]

On 2017/02/10 18:55:30, Ted C wrote:
> On 2017/02/10 18:02:42, Xi Han wrote:
> >
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> > File
> >
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:188:
> > additionalCommandLine.add("--" +
> > ContentSwitches.MEDIA_PLAYBACK_GESTURE_WHITELIST_SCOPE
> > Do you miss a "=" between the flag and the value?
> > 
> >
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> > File
> >
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> >
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java:203:
> > for (String param : mCommandLineParams) fullCommandLine.add(param);
> > It's better to keep the style consistent with lines 210 and 212:
> >  for (...) {
> >    statement;
> >  }
> 
> I have the same general question about this approach as:
> https://bugs.chromium.org/p/chromium/issues/detail?id=611842
> 
> The creation of renderer processes is controlled by the security
> policy.  That is an area that I'm not familiar with.  Before we
> start landing code that lightens the security restrictions for
> processes, we need to verify that there is no way a webapk will
> share an existing renderer with anything else.
> 
> I know that at some point in the past we would share a renderer
> under "some" circumstances, but maybe those are no longer the
> case.
> 
> Instead of using command line, I'm wondering if we could do the
> work here:
> ChromeContentBrowserClient::OverrideWebkitPrefs

ChromeContentBrowserClient seems to be a global instance in the browser process,
by which it seems hard to do something specific to a tab.

> 
> From RenderViewHost, we could potentially get out to Android's
> tab via WebContents::FromRenderViewHost (but maybe the RVH
> here is created prior to being attached to a webcontents).
> 
> But before we go the path of using command line params, I think
> we need to be 100% positive that renderer's aren't shared under
> any circumstances (i'd ask nasko@ or creis@ for that).

I'll try reaching Android's tab from RVH and see if it works.

[Zhiqiang Zhang (Slow), 2017-02-13 21:58:55]

On 2017/02/13 12:13:28, Zhiqiang Zhang wrote:
> On 2017/02/10 18:55:30, Ted C wrote:
> > On 2017/02/10 18:02:42, Xi Han wrote:
> > >
> >
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> > > File
> > >
> >
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2640143002/diff/100001/chrome/android/java/sr...
> > >
> >
>
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:188:
> > > additionalCommandLine.add("--" +
> > > ContentSwitches.MEDIA_PLAYBACK_GESTURE_WHITELIST_SCOPE
> > > Do you miss a "=" between the flag and the value?
> > > 
> > >
> >
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> > > File
> > >
> >
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2640143002/diff/100001/content/public/android...
> > >
> >
>
content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java:203:
> > > for (String param : mCommandLineParams) fullCommandLine.add(param);
> > > It's better to keep the style consistent with lines 210 and 212:
> > >  for (...) {
> > >    statement;
> > >  }
> > 
> > I have the same general question about this approach as:
> > https://bugs.chromium.org/p/chromium/issues/detail?id=611842
> > 
> > The creation of renderer processes is controlled by the security
> > policy.  That is an area that I'm not familiar with.  Before we
> > start landing code that lightens the security restrictions for
> > processes, we need to verify that there is no way a webapk will
> > share an existing renderer with anything else.
> > 
> > I know that at some point in the past we would share a renderer
> > under "some" circumstances, but maybe those are no longer the
> > case.
> > 
> > Instead of using command line, I'm wondering if we could do the
> > work here:
> > ChromeContentBrowserClient::OverrideWebkitPrefs
> 
> ChromeContentBrowserClient seems to be a global instance in the browser
process,
> by which it seems hard to do something specific to a tab.
> 
Ah, you are right. This is doable. Will complete the CL and send out tomorrow.

[jochen (gone - plz use gerrit), 2017-02-14 09:06:53]

waiting for somebody to review the android bits, and green trybots

[Zhiqiang Zhang (Slow), 2017-02-14 15:18:10]

zqzhang@chromium.org changed reviewers:
+ pkotwicz@chromium.org
- hanxi@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-14 15:18:11]

tedchoc@, pkotwicz@: PTAL the Android and WebAPK bits

Moving hanxi@ to CC and +pkotwicz@ for review

[pkotwicz, 2017-02-14 17:00:39]

I'll take a look this afternoon

[pkotwicz, 2017-02-15 03:21:07]

LGTM for the WebAPK bits

[pkotwicz, 2017-02-15 03:21:34]

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
(right):

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:50:
getActivityTab().setWebappManifestScope(mWebappInfo.scopeUri().toString());
Nit: I think we should put this inside the if(isInitialized()) block too

[Ted C, 2017-02-15 05:07:26]

lgtm

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:3030: public
void setWebappManifestScope(String scope) {
this needs javadoc...because I have no idea what this actually means

https://codereview.chromium.org/2640143002/diff/120001/chrome/browser/android...
File chrome/browser/android/tab_android.h (right):

https://codereview.chromium.org/2640143002/diff/120001/chrome/browser/android...
chrome/browser/android/tab_android.h:234: void SetWebappManifestScope(
ideally, I'd rather not see this hang off of Tab.  It would be nicer to have it
in a user data that hangs off of web contents to make it more platform agnostic.

that is more plumbing and the like, so I won't push too strongly now.  If we end
up needing to add anything more for webapps, I'll push more strongly though.

[jochen (gone - plz use gerrit), 2017-02-15 10:01:02]

lgtm

[Zhiqiang Zhang (Slow), 2017-02-15 11:17:28]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 11:17:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Zhiqiang Zhang (Slow), 2017-02-15 11:29:52]

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:3030: public
void setWebappManifestScope(String scope) {
On 2017/02/15 05:07:25, Ted C wrote:
> this needs javadoc...because I have no idea what this actually means

Done.

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
(right):

https://codereview.chromium.org/2640143002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java:50:
getActivityTab().setWebappManifestScope(mWebappInfo.scopeUri().toString());
On 2017/02/15 03:21:34, pkotwicz wrote:
> Nit: I think we should put this inside the if(isInitialized()) block too

Done.

https://codereview.chromium.org/2640143002/diff/120001/chrome/browser/android...
File chrome/browser/android/tab_android.h (right):

https://codereview.chromium.org/2640143002/diff/120001/chrome/browser/android...
chrome/browser/android/tab_android.h:234: void SetWebappManifestScope(
On 2017/02/15 05:07:25, Ted C wrote:
> ideally, I'd rather not see this hang off of Tab.  It would be nicer to have
it
> in a user data that hangs off of web contents to make it more platform
agnostic.
> 
> that is more plumbing and the like, so I won't push too strongly now.  If we
end
> up needing to add anything more for webapps, I'll push more strongly though.

I agree :)

[commit-bot: I haz the power, 2017-02-15 13:17:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 13:17:33]

Dry run: This issue passed the CQ dry run.

[Zhiqiang Zhang (Slow), 2017-02-15 15:43:51]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-15 15:43:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org, tedchoc@chromium.org, pkotwicz@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2640143002/#ps140001
(title: "addressed nits")

[commit-bot: I haz the power, 2017-02-15 15:44:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Zhiqiang Zhang (Slow), 2017-02-15 15:47:43]

The CQ bit was unchecked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-15 15:49:07]

zqzhang@chromium.org changed reviewers:
+ kenrb@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-15 15:49:08]

+kenrb@: common_param_traits_macros.h

[jochen (gone - plz use gerrit), 2017-02-15 15:50:10]

btw, please also send a PSA to blink-dev about this change (assuming that you go
through the chrome launch review anyways), otherwise, please send an intent to
ship instead of a PSA

[Zhiqiang Zhang (Slow), 2017-02-15 15:54:24]

On 2017/02/15 15:50:10, jochen wrote:
> btw, please also send a PSA to blink-dev about this change (assuming that you
go
> through the chrome launch review anyways), otherwise, please send an intent to
> ship instead of a PSA

Yes, I've drafted a PSA and will send out shortly.

[kenrb, 2017-02-15 16:51:29]

ipc lgtm

[Zhiqiang Zhang (Slow), 2017-02-15 16:55:04]

The CQ bit was checked by zqzhang@chromium.org

[commit-bot: I haz the power, 2017-02-15 16:55:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 17:04:22]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1487177704656150,
"parent_rev": "da4637b0c789f6360dacdea216dd5316972862ee", "commit_rev":
"183dc82ce88ae08056adfcc97e48937c0fe1be59"}

[commit-bot: I haz the power, 2017-02-15 17:05:09]

Description was changed from

==========
Allow autoplay unmuted for WebAPK in the manifest scope

Previously, we block all autoplay with sound on Android. To close
the gap between Web apps and native apps, this CL uses the
manifest scope as a whitelist, so that frames matching the scope
are allowed to autoplay with sound.

Design doc:
https://docs.google.com/a/google.com/document/d/12kghzV5FBk4JNgtOJk2pTGqNhMzN...

BUG=676312
==========

to

==========
Allow autoplay unmuted for WebAPK in the manifest scope

Previously, we block all autoplay with sound on Android. To close
the gap between Web apps and native apps, this CL uses the
manifest scope as a whitelist, so that frames matching the scope
are allowed to autoplay with sound.

Design doc:
https://docs.google.com/a/google.com/document/d/12kghzV5FBk4JNgtOJk2pTGqNhMzN...

BUG=676312

Review-Url: https://codereview.chromium.org/2640143002
Cr-Commit-Position: refs/heads/master@{#450727}
Committed:
https://chromium.googlesource.com/chromium/src/+/183dc82ce88ae08056adfcc97e48...
==========

[commit-bot: I haz the power, 2017-02-15 17:05:13]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/183dc82ce88ae08056adfcc97e48...