Rename kDisableGestureRequirementForMediaPlayback and make it a test-only flag.

Rename kDisableGestureRequirementForMediaPlayback and make it a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2843913003
Cr-Commit-Position: refs/heads/master@{#467692}
Committed: https://chromium.googlesource.com/chromium/src/+/d1f8f6ca6c8b7cdaf02150099ce6db0cfc7223f6

[mlamouri (slow - plz ping), 2017-04-26 19:22:17]

Description was changed from

==========
Move kDisableGestureRequirementForMediaPlayback as a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
==========

to

==========
Move kDisableGestureRequirementForMediaPlayback as a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[mlamouri (slow - plz ping), 2017-04-26 19:24:23]

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

[mlamouri (slow - plz ping), 2017-04-26 19:24:24]

dalecurtis@, PTAL

This isn't doing much, really. I will follow-up on the Cast change in order to
set an autoplay policy (do not want to change the behaviour in this CL). After
this, I will streamline the logic when passing the prefs to the renderer process
and to Blink.

[DaleCurtis, 2017-04-26 19:39:21]

Description should be updated to indicate flag is renamed.

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1450:
{"disable-gesture-requirement-for-media-playback",
I think you should keep this, otherwise it prevents anyone from doing testing on
Android without the ability to set command line flags.

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/android/voic...
File chrome/browser/android/voice_search_tab_helper.cc (right):

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/android/voic...
chrome/browser/android/voice_search_tab_helper.cc:16: // TODO(715588): this
class shouldn't be playing with the user gesture
TODO(user_id): ... http://crbug.com/715588

https://codereview.chromium.org/2843913003/diff/1/chromecast/browser/cast_bro...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/2843913003/diff/1/chromecast/browser/cast_bro...
chromecast/browser/cast_browser_main_parts.cc:221: // TODO(714676): this should
probably set the no restrictions autoplay
Ditto on TODO formatting, also re-wrap comment.

https://codereview.chromium.org/2843913003/diff/1/chromecast/browser/cast_bro...
chromecast/browser/cast_browser_main_parts.cc:263: {NULL, NULL},  // Termination
Might as well make it nullptr, nullptr while you're here.

https://codereview.chromium.org/2843913003/diff/1/media/base/android/java/src...
File media/base/android/java/src/org/chromium/media/MediaSwitches.java (right):

https://codereview.chromium.org/2843913003/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/MediaSwitches.java:1: //
Copyright 2016 The Chromium Authors. All rights reserved.
2017

[mlamouri (slow - plz ping), 2017-04-26 22:13:47]

Will update tomorrow.

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1450:
{"disable-gesture-requirement-for-media-playback",
On 2017/04/26 at 19:39:20, DaleCurtis wrote:
> I think you should keep this, otherwise it prevents anyone from doing testing
on Android without the ability to set command line flags.

The autoplay policy dropdown should be enough for testing on Android, right?

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/android/voic...
File chrome/browser/android/voice_search_tab_helper.cc (right):

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/android/voic...
chrome/browser/android/voice_search_tab_helper.cc:16: // TODO(715588): this
class shouldn't be playing with the user gesture
On 2017/04/26 at 19:39:20, DaleCurtis wrote:
> TODO(user_id): ... http://crbug.com/715588

I've read in chromium-dev that we no longer have to do TODO(username) but happy
to change.

[DaleCurtis, 2017-04-26 22:37:43]

lgtm

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1450:
{"disable-gesture-requirement-for-media-playback",
On 2017/04/26 at 22:13:47, mlamouri wrote:
> On 2017/04/26 at 19:39:20, DaleCurtis wrote:
> > I think you should keep this, otherwise it prevents anyone from doing
testing on Android without the ability to set command line flags.
> 
> The autoplay policy dropdown should be enough for testing on Android, right?

Ah right, yes we don't need two of these.

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/android/voic...
File chrome/browser/android/voice_search_tab_helper.cc (right):

https://codereview.chromium.org/2843913003/diff/1/chrome/browser/android/voic...
chrome/browser/android/voice_search_tab_helper.cc:16: // TODO(715588): this
class shouldn't be playing with the user gesture
On 2017/04/26 at 22:13:47, mlamouri wrote:
> On 2017/04/26 at 19:39:20, DaleCurtis wrote:
> > TODO(user_id): ... http://crbug.com/715588
> 
> I've read in chromium-dev that we no longer have to do TODO(username) but
happy to change.

Ah, you're right,
https://google.github.io/styleguide/cppguide.html#TODO_Comments - I'm out of
date!

[mlamouri (slow - plz ping), 2017-04-27 08:59:24]

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

[commit-bot: I haz the power, 2017-04-27 08:59:40]

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-04-27 09:42:15]

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

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

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

[mlamouri (slow - plz ping), 2017-04-27 12:15:57]

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

[commit-bot: I haz the power, 2017-04-27 12:16:13]

Dry run: CQ is trying da patch.

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

[mlamouri (slow - plz ping), 2017-04-27 12:16:53]

Description was changed from

==========
Move kDisableGestureRequirementForMediaPlayback as a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename kDisableGestureRequirementForMediaPlayback and make it a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[mlamouri (slow - plz ping), 2017-04-27 12:17:35]

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

[mlamouri (slow - plz ping), 2017-04-27 12:17:36]

+jam@ for global rubber-stamp.

https://codereview.chromium.org/2843913003/diff/1/chromecast/browser/cast_bro...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/2843913003/diff/1/chromecast/browser/cast_bro...
chromecast/browser/cast_browser_main_parts.cc:221: // TODO(714676): this should
probably set the no restrictions autoplay
On 2017/04/26 at 19:39:21, DaleCurtis wrote:
> Ditto on TODO formatting, also re-wrap comment.

Done. I will blame `git cl format` for the insane formatting :)

https://codereview.chromium.org/2843913003/diff/1/chromecast/browser/cast_bro...
chromecast/browser/cast_browser_main_parts.cc:263: {NULL, NULL},  // Termination
On 2017/04/26 at 19:39:21, DaleCurtis wrote:
> Might as well make it nullptr, nullptr while you're here.

Done. I will again blame `git cl format` for re-formatting all of this. Will do
the nullptr change :)

https://codereview.chromium.org/2843913003/diff/1/media/base/android/java/src...
File media/base/android/java/src/org/chromium/media/MediaSwitches.java (right):

https://codereview.chromium.org/2843913003/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/MediaSwitches.java:1: //
Copyright 2016 The Chromium Authors. All rights reserved.
On 2017/04/26 at 19:39:21, DaleCurtis wrote:
> 2017

Done :)

[commit-bot: I haz the power, 2017-04-27 13:55:24]

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

[commit-bot: I haz the power, 2017-04-27 13:55:25]

Dry run: This issue passed the CQ dry run.

[jam, 2017-04-27 15:33:10]

On 2017/04/27 12:17:36, mlamouri wrote:
> +jam@ for global rubber-stamp.
rs lgtm

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

The CQ bit was checked by mlamouri@chromium.org

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

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2843913003/#ps20001
(title: "dalecurtis comments and deps update")

[commit-bot: I haz the power, 2017-04-27 15:42:02]

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-04-27 15:49:45]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493307697668520,
"parent_rev": "2cb37fe0beb13e6baa8852662362e9204d1ebfa3", "commit_rev":
"d1f8f6ca6c8b7cdaf02150099ce6db0cfc7223f6"}

[commit-bot: I haz the power, 2017-04-27 15:50:06]

Description was changed from

==========
Rename kDisableGestureRequirementForMediaPlayback and make it a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename kDisableGestureRequirementForMediaPlayback and make it a test-only flag.

The flag should only be set by tests and will allow all autoplay related
restrictions such as autoplay policy and background playback to be
disabled.

BUG=714676
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2843913003
Cr-Commit-Position: refs/heads/master@{#467692}
Committed:
https://chromium.googlesource.com/chromium/src/+/d1f8f6ca6c8b7cdaf02150099ce6...
==========

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

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d1f8f6ca6c8b7cdaf02150099ce6...

[timvolodine, 2017-04-27 17:12:43]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2851453003/ by timvolodine@chromium.org.

The reason for reverting is: broke clang-clankium-tot-builder crbug.com/716075

(FullscreenWebContentsActivityTest not updated).

[timvolodine, 2017-04-27 17:42:59]

On 2017/04/27 17:12:43, timvolodine wrote:
> A revert of this CL (patchset #2 id:20001) has been created in
> https://codereview.chromium.org/2851453003/ by
mailto:timvolodine@chromium.org.
> 
> The reason for reverting is: broke clang-clankium-tot-builder crbug.com/716075
> 
> (FullscreenWebContentsActivityTest not updated).

As per crbug.com/716075 the reason was likely a different patch
https://codereview.chromium.org/2807663002.
This patch is probably fine and can be relanded, apologies for the confusion.