Disable the autoplay setting entry when Data Saver is ON.

Disable the autoplay setting entry when Data Saver is ON.

The reason is that we will not autoplay muted videos when Data Saver is on
and we should avoid confusing users by showing a setting that is marked as
enabled.

BUG=624326
Committed: https://crrev.com/a56744ee0e2d32af569ebc4f47fea812a229ab76
Cr-Commit-Position: refs/heads/master@{#402905}

[mlamouri (slow - plz ping), 2016-06-28 15:59:36]

mlamouri@chromium.org changed reviewers:
+ bauerb@chromium.org, miguelg@chromium.org

[mlamouri (slow - plz ping), 2016-06-28 15:59:37]

miguelg@ and bauerb@, can one of you have a look at this? so I can land if and
when UX confirms that it's good to go.

Thanks :)

[Miguel Garcia, 2016-06-28 16:04:05]

lgtm

is there a way we can add a test for this logic in a separate CL?

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java
(right):

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java:209:
if (AUTOPLAY_KEY.equals(prefName) && checked
Can you add a comment explaining what's going on here?

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/strings...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/strings...
chrome/android/java/strings/android_chrome_strings.grd:673: Not allowed because
Data Saver is ON
I assume you will get  UX approval on this one too?

[mlamouri (slow - plz ping), 2016-06-28 16:09:51]

I will look into the tests side of things.

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java
(right):

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java:209:
if (AUTOPLAY_KEY.equals(prefName) && checked
On 2016/06/28 at 16:04:05, Miguel Garcia wrote:
> Can you add a comment explaining what's going on here?

Done. Also fixed the logic (ie. remove `checked`).

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/strings...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2106823002/diff/1/chrome/android/java/strings...
chrome/android/java/strings/android_chrome_strings.grd:673: Not allowed because
Data Saver is ON
On 2016/06/28 at 16:04:05, Miguel Garcia wrote:
> I assume you will get  UX approval on this one too?

Yes, will make sure they are happy with the string. Changing to Block because it
seems to be more common.

[mlamouri (slow - plz ping), 2016-06-29 09:59:20]

Description was changed from

==========
Disable the autoplay setting entry when Data Saver is ON.

The strings and exact behaviour are TBC by UX.

BUG=<TBD>
==========

to

==========
Disable the autoplay setting entry when Data Saver is ON.

The reason is that we will not autoplay muted videos when Data Saver is on
and we should avoid confusing users by showing a setting that is marked as
enabled.

BUG=624326
==========

[mlamouri (slow - plz ping), 2016-06-29 09:59:26]

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

[commit-bot: I haz the power, 2016-06-29 09:59:58]

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, 2016-06-29 12:00:23]

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

[commit-bot: I haz the power, 2016-06-29 12:00:24]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2016-06-29 19:04:15]

I've added tinting on the icon so it also looks disabled.

[mlamouri (slow - plz ping), 2016-06-29 19:04:19]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-06-29 19:04:19]

The patchset sent to the CQ was uploaded after l-g-t-m from miguelg@chromium.org
Link to the patchset: https://codereview.chromium.org/2106823002/#ps60001
(title: "disabled icon")

[commit-bot: I haz the power, 2016-06-29 19:04:56]

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, 2016-06-29 19:45:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-29 19:45:28]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-29 19:46:36]

Description was changed from

==========
Disable the autoplay setting entry when Data Saver is ON.

The reason is that we will not autoplay muted videos when Data Saver is on
and we should avoid confusing users by showing a setting that is marked as
enabled.

BUG=624326
==========

to

==========
Disable the autoplay setting entry when Data Saver is ON.

The reason is that we will not autoplay muted videos when Data Saver is on
and we should avoid confusing users by showing a setting that is marked as
enabled.

BUG=624326

Committed: https://crrev.com/a56744ee0e2d32af569ebc4f47fea812a229ab76
Cr-Commit-Position: refs/heads/master@{#402905}
==========

[commit-bot: I haz the power, 2016-06-29 19:46:37]

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