Enable user gesture requirement for autoplay experiment.

Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679
Committed: https://crrev.com/9e93489f36f1d34cb0eb0ae828e78d9a84ae8b95
Cr-Commit-Position: refs/heads/master@{#391318}

[liberato (no reviews please), 2016-04-04 16:10:25]

Description was changed from

==========
Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679
==========

to

==========
Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679
==========

[liberato (no reviews please), 2016-04-04 16:10:25]

liberato@chromium.org changed reviewers:
+ philipj@opera.com

[liberato (no reviews please), 2016-04-04 16:11:05]

very small change to the autoplay experiment to let us try out stuff on desktop.

thanks
-fl

[philipj_slow, 2016-04-05 11:17:52]

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

https://codereview.chromium.org/1850903002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:432: // default, else
the experiment does nothing.
s/else/otherwise/

https://codereview.chromium.org/1850903002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:433: if
((document.settings() &&
document.settings()->mediaPlaybackRequiresUserGesture())
Can't the same code that enables the experiment also set this setting?

[mlamouri (slow - plz ping), 2016-04-21 17:11:35]

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

[mlamouri (slow - plz ping), 2016-04-21 17:11:36]

Should we clean up the experiements currently in AutoplayExperimentHelper.h?

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

https://codereview.chromium.org/1850903002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:433: if
((document.settings() &&
document.settings()->mediaPlaybackRequiresUserGesture())
On 2016/04/05 at 11:17:51, philipj_slow wrote:
> Can't the same code that enables the experiment also set this setting?

They are fairly close:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

[liberato (no reviews please), 2016-04-25 22:38:44]

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

https://codereview.chromium.org/1850903002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:432: // default, else
the experiment does nothing.
On 2016/04/05 11:17:51, philipj wrote:
> s/else/otherwise/

Done.

https://codereview.chromium.org/1850903002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:433: if
((document.settings() &&
document.settings()->mediaPlaybackRequiresUserGesture())
On 2016/04/05 11:17:51, philipj wrote:
> Can't the same code that enables the experiment also set this setting?

the experiment is enabled by copying the string around, until it ends up in
blink::Settings::autoplayExperimentMode.  nothing attempts to interpret it;
that's all done in HTMLMediaElement.

that's why i just override in the element.  it keeps the interpretation of the
string centralized.

https://codereview.chromium.org/1850903002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:433: if
((document.settings() &&
document.settings()->mediaPlaybackRequiresUserGesture())
On 2016/04/21 17:11:35, Mounir Lamouri wrote:
> On 2016/04/05 at 11:17:51, philipj_slow wrote:
> > Can't the same code that enables the experiment also set this setting?
> 
> They are fairly close:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

in PS{2,3}, i've left it as-is to avoid parsing the experiment string.

if either of you feel strongly, please let me know and i'll move it.

[mlamouri (slow - plz ping), 2016-04-26 14:48:40]

lgtm

[mlamouri (slow - plz ping), 2016-04-26 14:49:21]

Note: I only own the Blink files. You will have to get another reviewer for the
content/ changes.

[liberato (no reviews please), 2016-04-26 15:29:57]

liberato@chromium.org changed reviewers:
+ brettw@chromium.org

[liberato (no reviews please), 2016-04-26 15:30:59]

brettw: PTAL @content/
mounir: thanks for the discussion!

thanks
-fl

[mlamouri (slow - plz ping), 2016-05-02 13:51:18]

+jochen@ (also content/ owner)

[mlamouri (slow - plz ping), 2016-05-02 13:51:26]

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

[jochen (gone - plz use gerrit), 2016-05-02 16:07:04]

lgtm

[liberato (no reviews please), 2016-05-03 16:27:22]

brettw: PTAL @ content/ for owners approval

jochen, mlamouri: thanks

thanks
-fl

[liberato (no reviews please), 2016-05-03 16:48:05]

brettw: never mind, thanks!

[liberato (no reviews please), 2016-05-03 16:48:09]

The CQ bit was checked by liberato@chromium.org

[liberato (no reviews please), 2016-05-03 16:48:10]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1850903002/#ps60001
(title: "rebased.")

[commit-bot: I haz the power, 2016-05-03 16:48:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1850903002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1850903002/60001

[commit-bot: I haz the power, 2016-05-03 19:00:36]

Description was changed from

==========
Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679
==========

to

==========
Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679
==========

[commit-bot: I haz the power, 2016-05-03 19:00:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-03 19:02:46]

Description was changed from

==========
Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679
==========

to

==========
Enable user gesture requirement for autoplay experiment.

For android, the autoplay experiment operated independently of the
user gesture requirement for media elements.  To deploy the
experiment on desktop, the experiment has to enable the user gesture
requirement, else there won't be much for it to do.

BUG=599679

Committed: https://crrev.com/9e93489f36f1d34cb0eb0ae828e78d9a84ae8b95
Cr-Commit-Position: refs/heads/master@{#391318}
==========

[commit-bot: I haz the power, 2016-05-03 19:02:47]

Patchset 4 (id:??) landed as
https://crrev.com/9e93489f36f1d34cb0eb0ae828e78d9a84ae8b95
Cr-Commit-Position: refs/heads/master@{#391318}