Autoplay: use an autoplay policy setting in Blink.

Autoplay: use an autoplay policy setting in Blink.

This is a continuity of the autoplay policy that was introduced in the
embedder.

BUG=714676
R=dalecurtis@chromium.org,dcheng@chromium.org,nasko@chromium.org

Review-Url: https://codereview.chromium.org/2850553002
Cr-Commit-Position: refs/heads/master@{#471256}
Committed: https://chromium.googlesource.com/chromium/src/+/276a6f39aefbc4f23fe98fb71ca4881a0a0ff29c

[mlamouri (slow - plz ping), 2017-04-27 15:47:41]

nasko@, PTAL at content/renderer/
dcheng@, PTAL at Blink changes
dalecurtis@, FYI because you reviewed the previous CLs

[Raymond Toy, 2017-04-27 15:51:59]

rtoy@chromium.org changed reviewers:
+ rtoy@chromium.org

[Raymond Toy, 2017-04-27 15:52:00]

DBC: So if the autoplay policy is set, a user gesture is required for any
WebAudio output in any frame?  If so, that's not in the spec.

[mlamouri (slow - plz ping), 2017-04-27 15:52:21]

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

[commit-bot: I haz the power, 2017-04-27 15:53:09]

Dry run: CQ is trying da patch.

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

[dcheng, 2017-04-27 15:57:17]

https://codereview.chromium.org/2850553002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/media/AutoplayPolicy.cpp (right):

https://codereview.chromium.org/2850553002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/media/AutoplayPolicy.cpp:62: if
(document.GetSettings())
Should this be !document.GetSettings()?

https://codereview.chromium.org/2850553002/diff/1/third_party/WebKit/public/w...
File third_party/WebKit/public/web/WebSettings.h (right):

https://codereview.chromium.org/2850553002/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebSettings.h:117: // web_preferences.h
Is this in another CL? Or is this referencing AutoplayPolicy::Type?

https://codereview.chromium.org/2850553002/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebSettings.h:120:
kCrossOriginUserGestureRequired,
FWIW, I find this name a bit confusing: to me, it reads "cross-origin user
gesture", where cross-origin is modifying user gesture, rather than "autoplay in
cross-origin subframe requires user gesture". *shrug*

kUserGestureRequiredForCrossOriginSubframe might be clearer, though it's also
more verbose.

[mlamouri (slow - plz ping), 2017-04-27 15:58:45]

On 2017/04/27 at 15:52:00, rtoy wrote:
> DBC: So if the autoplay policy is set, a user gesture is required for any
WebAudio output in any frame?  If so, that's not in the spec.

This is only refactoring the code that was related to the discussion we had at
TPAC.

As a reminder, the issue I opened at the time:
https://github.com/WebAudio/web-audio-api/issues/836
Related commit to the spec:
https://github.com/WebAudio/web-audio-api/commit/ee43f85e7969deaac313265a11fa...

The concept in the Web Audio spec that is related to autoplay is "allowed to
start", see: https://webaudio.github.io/web-audio-api/#dfn-allowed-to-start

[nasko, 2017-04-27 16:08:42]

content/ rubberstamp LGTM

[commit-bot: I haz the power, 2017-04-27 16:17:28]

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

[commit-bot: I haz the power, 2017-04-27 16:17:29]

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_...)

[Raymond Toy, 2017-04-27 16:50:49]

On Thu, Apr 27, 2017 at 8:58 AM, <mlamouri@chromium.org> wrote:

> On 2017/04/27 at 15:52:00, rtoy wrote:
> > DBC: So if the autoplay policy is set, a user gesture is required for any
> WebAudio output in any frame? If so, that's not in the spec.
>
> This is only refactoring the code that was related to the discussion we
> had at
> TPAC.
>
> As a reminder, the issue I opened at the time:
> https://github.com/WebAudio/web-audio-api/issues/836
> Related commit to the spec:
> https://github.com/WebAudio/web-audio-api/commit/
> ee43f85e7969deaac313265a11fa561636eb6cbb
>
> The concept in the Web Audio spec that is related to autoplay is "allowed
> to
> start", see: https://webaudio.github.io/web-audio-api/#dfn-allowed-to-
> start
>

Ah, yes. I keep thinking this was for cross-origin iframes, but it applies
everywhere.

Thanks for the clarification.

>
>
> https://codereview.chromium.org/2850553002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Raymond Toy, 2017-04-27 16:50:49]

On Thu, Apr 27, 2017 at 8:58 AM, <mlamouri@chromium.org> wrote:

> On 2017/04/27 at 15:52:00, rtoy wrote:
> > DBC: So if the autoplay policy is set, a user gesture is required for any
> WebAudio output in any frame? If so, that's not in the spec.
>
> This is only refactoring the code that was related to the discussion we
> had at
> TPAC.
>
> As a reminder, the issue I opened at the time:
> https://github.com/WebAudio/web-audio-api/issues/836
> Related commit to the spec:
> https://github.com/WebAudio/web-audio-api/commit/
> ee43f85e7969deaac313265a11fa561636eb6cbb
>
> The concept in the Web Audio spec that is related to autoplay is "allowed
> to
> start", see: https://webaudio.github.io/web-audio-api/#dfn-allowed-to-
> start
>

Ah, yes. I keep thinking this was for cross-origin iframes, but it applies
everywhere.

Thanks for the clarification.

>
>
> https://codereview.chromium.org/2850553002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mlamouri (slow - plz ping), 2017-04-27 17:30:21]

On 2017/04/27 at 16:50:49, rtoy wrote:
> On Thu, Apr 27, 2017 at 8:58 AM, <mlamouri@chromium.org> wrote:
> 
> > On 2017/04/27 at 15:52:00, rtoy wrote:
> > > DBC: So if the autoplay policy is set, a user gesture is required for any
> > WebAudio output in any frame? If so, that's not in the spec.
> >
> > This is only refactoring the code that was related to the discussion we
> > had at
> > TPAC.
> >
> > As a reminder, the issue I opened at the time:
> > https://github.com/WebAudio/web-audio-api/issues/836
> > Related commit to the spec:
> > https://github.com/WebAudio/web-audio-api/commit/
> > ee43f85e7969deaac313265a11fa561636eb6cbb
> >
> > The concept in the Web Audio spec that is related to autoplay is "allowed
> > to
> > start", see: https://webaudio.github.io/web-audio-api/#dfn-allowed-to-
> > start
> >
> 
> Ah, yes. I keep thinking this was for cross-origin iframes, but it applies
> everywhere.
> 
> Thanks for the clarification.

Note that the code still block this only for cross-origin iframe.

[mlamouri (slow - plz ping), 2017-05-09 12:26:21]

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

[commit-bot: I haz the power, 2017-05-09 12:26:43]

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-05-09 14:42:12]

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

[commit-bot: I haz the power, 2017-05-09 14:42:13]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mlamouri (slow - plz ping), 2017-05-09 17:53:15]

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

[commit-bot: I haz the power, 2017-05-09 17:54:08]

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-05-09 18:27:40]

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

[commit-bot: I haz the power, 2017-05-09 18:27:41]

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_...)

[mlamouri (slow - plz ping), 2017-05-11 11:05:08]

dcheng@, PTAL :)

[mlamouri (slow - plz ping), 2017-05-11 11:05:09]

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

[commit-bot: I haz the power, 2017-05-11 11:05:43]

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-05-11 14:05:15]

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

[commit-bot: I haz the power, 2017-05-11 14:05:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2017-05-12 07:24:08]

lgtm

[mlamouri (slow - plz ping), 2017-05-12 08:40:44]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-05-12 08:40:44]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2850553002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-12 08:40:57]

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-05-12 09:24:32]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494578444091660,
"parent_rev": "041528ae6e6abe67ac2f5c14cd4991b8e181ecf2", "commit_rev":
"276a6f39aefbc4f23fe98fb71ca4881a0a0ff29c"}

[commit-bot: I haz the power, 2017-05-12 09:24:43]

Description was changed from

==========
Autoplay: use an autoplay policy setting in Blink.

This is a continuity of the autoplay policy that was introduced in the
embedder.

BUG=714676
R=dalecurtis@chromium.org,dcheng@chromium.org,nasko@chromium.org
==========

to

==========
Autoplay: use an autoplay policy setting in Blink.

This is a continuity of the autoplay policy that was introduced in the
embedder.

BUG=714676
R=dalecurtis@chromium.org,dcheng@chromium.org,nasko@chromium.org

Review-Url: https://codereview.chromium.org/2850553002
Cr-Commit-Position: refs/heads/master@{#471256}
Committed:
https://chromium.googlesource.com/chromium/src/+/276a6f39aefbc4f23fe98fb71ca4...
==========

[commit-bot: I haz the power, 2017-05-12 09:24:44]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/276a6f39aefbc4f23fe98fb71ca4...