[subresource_filter] Site Details UI for default state

[subresource_filter] Site Details UI for default state

This patch changes the subresource filter Site Details page to show up
even if the setting is in the default state. Additionally it shows a
custom block string (a placeholder for now).

BUG=689487
Review-Url: https://codereview.chromium.org/2888473003
Cr-Commit-Position: refs/heads/master@{#477673}
Committed: https://chromium.googlesource.com/chromium/src/+/9f6c7fe99a9bd92bce38ebc9c629c89ccacd07eb

[Charlie Harrison, 2017-05-16 04:37:05]

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

[commit-bot: I haz the power, 2017-05-16 04:37:41]

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-16 05:48:29]

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

[commit-bot: I haz the power, 2017-05-16 05:48:30]

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

[Charlie Harrison, 2017-05-16 14:30:44]

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

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

Dry run: CQ is trying da patch.

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

[Charlie Harrison, 2017-05-16 14:57:00]

csharrison@chromium.org changed reviewers:
+ raymes@chromium.org

[Charlie Harrison, 2017-05-16 14:57:00]

OK here's the updated patch for site details UI. Would you take a look at the
overall approach?

Since this interacts with the existing header for OS warnings, I've uploaded a
screenshot on the linked bug with both headers enabled.

[commit-bot: I haz the power, 2017-05-16 15:47:18]

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

[commit-bot: I haz the power, 2017-05-16 15:47:19]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-05-17 00:07:43]

lgtm with the comment below, however I don't know this code very well at all :)

https://codereview.chromium.org/2888473003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:545:
: R.string.website_settings_permissions_allow;
nit: Not sure if we need to base this on whether it's activated based on the
discussion from yesterday

[Charlie Harrison, 2017-05-17 00:24:57]

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

[commit-bot: I haz the power, 2017-05-17 00:26:26]

Dry run: CQ is trying da patch.

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

[Charlie Harrison, 2017-05-17 00:38:32]

Description was changed from

==========
[subresource_filter] Implement the Site Details UI to match mocks

This patch changes the subresource filter Site Details page to
 - Include a header row of extra information if the site is activated
 - Have custom allow/block strings, which change depending on activation

BUG=689487
==========

to

==========
[subresource_filter] Implement the Site Details UI to match mocks

This patch changes the subresource filter Site Details page to
 - Include a header row of extra information if the site is activated
 - Have a custom block string

BUG=689487
==========

[Charlie Harrison, 2017-05-17 00:38:51]

csharrison@chromium.org changed reviewers:
+ finnur@chromium.org

[Charlie Harrison, 2017-05-17 00:38:52]

Thanks!
finnur, would you also take a look? The linked bug should have some context for
this work + a screenshot.

https://codereview.chromium.org/2888473003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:545:
: R.string.website_settings_permissions_allow;
On 2017/05/17 00:07:43, raymes wrote:
> nit: Not sure if we need to base this on whether it's activated based on the
> discussion from yesterday

Ah you're right that's my mistake. We don't need to branch on |activated| at all
really, and can reuse the standard Allow string.

[commit-bot: I haz the power, 2017-05-17 03:20:25]

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

[commit-bot: I haz the power, 2017-05-17 03:20:26]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-05-22 13:36:48]

finnur, friendly ping

[Finnur, 2017-05-23 15:37:13]

Sorry for the late response, this fell through the cracks (thought I was done
with that review).

In the future, don't wait so long to ping me if I don't reply in a timely
manner.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
File chrome/android/java/res/xml/single_website_preferences.xml (right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
chrome/android/java/res/xml/single_website_preferences.xml:19:
android:layout="@layout/divider_preference" />
I think it would lessen the cognitive overload for the user (and potentially
code complexity for us devs also) if this was a simple subtitle under the
subresource_filter_permission_list preference. I think that was the original
intent of the designer and we should explore that option, unless someone
objects.

A secondary, and perhaps moot concern, is that a preference might not be the
best widget to use to show a static information message, as it has certain
behavior on-click, which indicate that an action takes place on clicking (which
would then need to be disabled to avoid confusion). We should either find a way
to disable these behaviors or find a replacement. I'm not sure what is a good
replacement is, if we end up going that route, but maybe twellington@ knows.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
chrome/android/java/res/xml/single_website_preferences.xml:55:
android:key="subresource_filter_permission_list" />
Friendly reminder to replace the use of the... I think... popups icon (if not
done already). :)

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:68:
public static final String PREF_EXTRA_INFO_DIVIDER =
"website_settings_extra_info_divider";
These key names are a bit generic, as if to support future reuse for other info
items. I doubt that is useful, though, because you could have these two and any
future info item we might add present on screen at the same time. It would make
more sense to programmatically add them, but for now I would just rename it to
be more specific.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:275:
// gating the extra info header for subresource filtering.
nit: "gating a header" is language that I (as a non-native speaker) struggle
with... :)

But taking a step back, maybe it would be better to explain what it means for a
SubresourceFilter to be activated and how these can be at odds with one another,
as it seems. At first I confused it with the SubresourceFilter content setting
value, which this UI is toggling.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:518:
private void setUpSubresourceFilterPreference(Preference preference, boolean
activated) {
This function has some non-obvious functionality that would benefit from a
comment, including the param |activated| and how it affects this all.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:521:
// The subresource filter permission shows sometimes even if there is no
permission set.
I don't understand this comment, nor it's placement (above the permission ==
null block)... 

Is this a hack to work around a bug or describing how the implementation works?

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:532:
permission, ""));
This feels a bit hacky... I'm a little concerned that we're modifying the mSite
object in order to, AFAICT, show some string in the UI. I don't think we do this
elsewhere. Why is it necessary to do it this way?

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:756: <message
name="IDS_SUBRESOURCE_FILTER_PERMISSION_EXTRA_INFO" desc="The extra information
at the top of the Site Details page when subresource filtering is activated" 
translateable="false">
nit: redundant space before translateable="false"

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:757: Subresource
filtering activated on this page
As discussed before, we need better strings to explain this, but I don't see it
as a blocker for this CL.

[Charlie Harrison, 2017-05-23 17:12:19]

raymes: there is a question for you in the comments (whether it's alright to
move forward with implementing another subtitle instead of the informational
banner).

finnur: I responded to most of the comments but have not implemented the new
subtitle or the replacement for the Preference banner. I'm waiting till we have
consensus of which option we want to move forward with.

Thanks for the very detailed review, I appreciate it.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
File chrome/android/java/res/xml/single_website_preferences.xml (right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
chrome/android/java/res/xml/single_website_preferences.xml:19:
android:layout="@layout/divider_preference" />
On 2017/05/23 15:37:13, Finnur wrote:
> I think it would lessen the cognitive overload for the user (and potentially
> code complexity for us devs also) if this was a simple subtitle under the
> subresource_filter_permission_list preference. I think that was the original
> intent of the designer and we should explore that option, unless someone
> objects.
I'm fine with adding a second subtitle which we did discuss before. Not
implemented in the current PS though.

raymes: Can you confirm that this is an OK solution?
> 
> A secondary, and perhaps moot concern, is that a preference might not be the
> best widget to use to show a static information message, as it has certain
> behavior on-click, which indicate that an action takes place on clicking
(which
> would then need to be disabled to avoid confusion). We should either find a
way
> to disable these behaviors or find a replacement. I'm not sure what is a good
> replacement is, if we end up going that route, but maybe twellington@ knows.
Acknowledged.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
chrome/android/java/res/xml/single_website_preferences.xml:55:
android:key="subresource_filter_permission_list" />
On 2017/05/23 15:37:13, Finnur wrote:
> Friendly reminder to replace the use of the... I think... popups icon (if not
> done already). :)

Got it. The icons should be ready but we aren't ready to land them quite yet.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:68:
public static final String PREF_EXTRA_INFO_DIVIDER =
"website_settings_extra_info_divider";
On 2017/05/23 15:37:13, Finnur wrote:
> These key names are a bit generic, as if to support future reuse for other
info
> items. I doubt that is useful, though, because you could have these two and
any
> future info item we might add present on screen at the same time. It would
make
> more sense to programmatically add them, but for now I would just rename it to
> be more specific.

Renamed to subresource_filter_permission_extra_info.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:275:
// gating the extra info header for subresource filtering.
On 2017/05/23 15:37:13, Finnur wrote:
> nit: "gating a header" is language that I (as a non-native speaker) struggle
> with... :)
> 
> But taking a step back, maybe it would be better to explain what it means for
a
> SubresourceFilter to be activated and how these can be at odds with one
another,
> as it seems. At first I confused it with the SubresourceFilter content setting
> value, which this UI is toggling.

I tried to rephrase the comment.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:518:
private void setUpSubresourceFilterPreference(Preference preference, boolean
activated) {
On 2017/05/23 15:37:13, Finnur wrote:
> This function has some non-obvious functionality that would benefit from a
> comment, including the param |activated| and how it affects this all.

Added a comment.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:521:
// The subresource filter permission shows sometimes even if there is no
permission set.
On 2017/05/23 15:37:13, Finnur wrote:
> I don't understand this comment, nor it's placement (above the permission ==
> null block)... 
> 
> Is this a hack to work around a bug or describing how the implementation
works?

It's describing how the implementation works. I've expanded the comment.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:532:
permission, ""));
On 2017/05/23 15:37:13, Finnur wrote:
> This feels a bit hacky... I'm a little concerned that we're modifying the
mSite
> object in order to, AFAICT, show some string in the UI. I don't think we do
this
> elsewhere. Why is it necessary to do it this way?

Is is necessary due to how we've implemented setSubresourceFilterPermission in
Website.

I've updated the comment to explain this, but would appreciate any pointers for
a better way to do this since I agree it is kind of hacky.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:756: <message
name="IDS_SUBRESOURCE_FILTER_PERMISSION_EXTRA_INFO" desc="The extra information
at the top of the Site Details page when subresource filtering is activated" 
translateable="false">
On 2017/05/23 15:37:13, Finnur wrote:
> nit: redundant space before translateable="false"

Done.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:757: Subresource
filtering activated on this page
On 2017/05/23 15:37:13, Finnur wrote:
> As discussed before, we need better strings to explain this, but I don't see
it
> as a blocker for this CL.

Acknowledged.

[raymes, 2017-05-23 22:29:01]

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
File chrome/android/java/res/xml/single_website_preferences.xml (right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/res...
chrome/android/java/res/xml/single_website_preferences.xml:19:
android:layout="@layout/divider_preference" />
I chatted with Charlie offline. I'm ok not showing a banner (I don't think
that's great either) but my concern was that afaik nothing else shows a subtitle
here, and I'm not sure that it fits in well with the existing design of the UI.
In the long term I think we want to show subtitles but we should come up with a
good design.

My preference as a short term, simple solution would be just to modify the text
that's displayed in the option for the setting. There's a precedent for doing
this with the default search engine geolocation setting. 

In any case, it may be good to land a simple solution and iron out the details
with a UX designer.

[Finnur, 2017-05-24 10:37:23]

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:532:
permission, ""));
If the problem you are facing is only while changing the value then we could
modify setSubresourceFilterPermission to add the exception when it is missing
(and have a comment explaining why). Would make this code simpler, I think.

https://codereview.chromium.org/2888473003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:529:
* 1. If the site is activated, the permission should show up even if it is set
as the default
s/site/filtering/ ?

[Charlie Harrison, 2017-05-25 15:28:04]

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

[commit-bot: I haz the power, 2017-05-25 15:28:32]

Dry run: CQ is trying da patch.

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

[Charlie Harrison, 2017-05-25 15:53:42]

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

[commit-bot: I haz the power, 2017-05-25 15:54:03]

Dry run: CQ is trying da patch.

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

[Charlie Harrison, 2017-05-25 15:54:18]

Description was changed from

==========
[subresource_filter] Implement the Site Details UI to match mocks

This patch changes the subresource filter Site Details page to
 - Include a header row of extra information if the site is activated
 - Have a custom block string

BUG=689487
==========

to

==========
[subresource_filter] Implement the Site Details UI to match mocks

This patch changes the subresource filter Site Details page to show up
even if the setting is in the default state. Additionally it shows a
custom block string (a placeholder for now).

BUG=689487
==========

[Charlie Harrison, 2017-05-25 15:54:43]

Description was changed from

==========
[subresource_filter] Implement the Site Details UI to match mocks

This patch changes the subresource filter Site Details page to show up
even if the setting is in the default state. Additionally it shows a
custom block string (a placeholder for now).

BUG=689487
==========

to

==========
[subresource_filter] Site Details UI for default state

This patch changes the subresource filter Site Details page to show up
even if the setting is in the default state. Additionally it shows a
custom block string (a placeholder for now).

BUG=689487
==========

[Charlie Harrison, 2017-05-25 15:57:20]

finnur, raymes I have updated the CL to be simpler (including the description).

There is no longer an informational banner while we figure out which Site
Details UI we want for the feature.

Thanks finnur for the useful feedback.

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:532:
permission, ""));
On 2017/05/24 10:37:22, Finnur wrote:
> If the problem you are facing is only while changing the value then we could
> modify setSubresourceFilterPermission to add the exception when it is missing
> (and have a comment explaining why). Would make this code simpler, I think.

Yeah that works for me. Done.

https://codereview.chromium.org/2888473003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
(right):

https://codereview.chromium.org/2888473003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java:529:
* 1. If the site is activated, the permission should show up even if it is set
as the default
On 2017/05/24 10:37:22, Finnur wrote:
> s/site/filtering/ ?

Done.

[commit-bot: I haz the power, 2017-05-25 17:10:04]

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

[commit-bot: I haz the power, 2017-05-25 17:10:06]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-05-29 04:58:07]

Seems good to me as a first cut. I defer to Finnur for implementation though :)

[Finnur, 2017-05-30 13:29:26]

LGTM, one nit

https://codereview.chromium.org/2888473003/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java
(right):

https://codereview.chromium.org/2888473003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java:243:
public static boolean getSubresourceFilterActivated(String origin) {
nit: document.

[Charlie Harrison, 2017-06-04 03:57:39]

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

[commit-bot: I haz the power, 2017-06-04 03:57:50]

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-06-04 04:35:38]

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

[commit-bot: I haz the power, 2017-06-04 04:35:40]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-06-05 12:11:06]

csharrison@chromium.org changed reviewers:
+ dtrainor@chromium.org

[Charlie Harrison, 2017-06-05 12:11:08]

Thanks, we've mostly agreed on going back to the info banner implementation but
let's land this first since it is already reviewed (and will still be used).

dtrainor would you look at the preference bridge + the grd file?

[David Trainor- moved to gerrit, 2017-06-07 05:54:34]

lgtm

[Charlie Harrison, 2017-06-07 15:20:25]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2017-06-07 15:20:26]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org,
finnur@chromium.org
Link to the patchset: https://codereview.chromium.org/2888473003/#ps120001
(title: "finnur review")

[commit-bot: I haz the power, 2017-06-07 15:20:52]

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-06-07 16:14:50]

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

[commit-bot: I haz the power, 2017-06-07 16:14:51]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

This could be a Rietveld flake (see http://crbug.com/401833), so if this CL has
no binary files, please re-check commit box.
Thanks for your patience.

Transient error: timed out

[Charlie Harrison, 2017-06-07 16:18:33]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2017-06-07 16:18:53]

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-06-07 16:24:28]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1496852313726020,
"parent_rev": "6416ba5d192d3d123b9ba75ff76fd1fa40900b37", "commit_rev":
"9f6c7fe99a9bd92bce38ebc9c629c89ccacd07eb"}

[commit-bot: I haz the power, 2017-06-07 16:24:43]

Description was changed from

==========
[subresource_filter] Site Details UI for default state

This patch changes the subresource filter Site Details page to show up
even if the setting is in the default state. Additionally it shows a
custom block string (a placeholder for now).

BUG=689487
==========

to

==========
[subresource_filter] Site Details UI for default state

This patch changes the subresource filter Site Details page to show up
even if the setting is in the default state. Additionally it shows a
custom block string (a placeholder for now).

BUG=689487

Review-Url: https://codereview.chromium.org/2888473003
Cr-Commit-Position: refs/heads/master@{#477673}
Committed:
https://chromium.googlesource.com/chromium/src/+/9f6c7fe99a9bd92bce38ebc9c629...
==========

[commit-bot: I haz the power, 2017-06-07 16:24:44]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/9f6c7fe99a9bd92bce38ebc9c629...