Always show counters in the material design Clear Browsing Data dialog.

Always show counters in the material design Clear Browsing Data dialog.

Previously, they were only shown when the corresponding datatype was
checked. This led to uneven spacing between checkboxes and arguably
strange visual behavior where text was appearing and disappearing.

See the linked bug for screenshots.

BUG=654812
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[msramek, 2017-01-11 17:51:34]

Description was changed from

==========
Always show counters in the material design Clear Browsing Data dialog.

Previously, they were only shown when the corresponding datatype was
checked. This led to uneven spacing between checkboxes and arguably
strange visual behavior where text was appearing and disappearing.

See the linked bug for screenshots.

BUG=654812
==========

to

==========
Always show counters in the material design Clear Browsing Data dialog.

Previously, they were only shown when the corresponding datatype was
checked. This led to uneven spacing between checkboxes and arguably
strange visual behavior where text was appearing and disappearing.

See the linked bug for screenshots.

BUG=654812
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[msramek, 2017-01-11 17:51:36]

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

[commit-bot: I haz the power, 2017-01-11 17:51:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-01-11 17:53:28]

msramek@chromium.org changed reviewers:
+ dschuyler@chromium.org

[msramek, 2017-01-11 17:53:29]

Hi Dave!

Please have a look! We got a go from the UI review to finally fix this :)

Thanks,
Martin

[commit-bot: I haz the power, 2017-01-11 18:21:22]

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

[commit-bot: I haz the power, 2017-01-11 18:21:23]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Dan Beam, 2017-01-11 20:29:28]

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

[Dan Beam, 2017-01-11 20:29:28]

https://codereview.chromium.org/2623033003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html
(left):

https://codereview.chromium.org/2623033003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html:90:
--settings-secondary-unchecked: {
this is the only use of this mixin, afaict

https://cs.chromium.org/search/?q=settings-secondary-unchecked&sq=package:chr...

please also remove it from
chrome/browser/resources/controls/settings_{checkbox,toggle_button}.html

[dschuyler, 2017-01-11 20:30:23]

lgtm

[msramek, 2017-01-12 10:41:19]

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

[commit-bot: I haz the power, 2017-01-12 10:41:36]

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-01-12 10:43:57]

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

[commit-bot: I haz the power, 2017-01-12 10:43:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[msramek, 2017-01-12 10:50:09]

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

[commit-bot: I haz the power, 2017-01-12 10:50:21]

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-01-12 11:24:35]

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

[commit-bot: I haz the power, 2017-01-12 11:24:36]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[msramek, 2017-01-12 12:35:23]

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

[msramek, 2017-01-12 12:35:32]

Patchset #4 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-01-12 12:35:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-01-12 12:37:23]

Thanks Dave!

Dan - I removed the mixin and also updated the failing PrivacyPageTest. PTAL!

https://codereview.chromium.org/2623033003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html
(left):

https://codereview.chromium.org/2623033003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html:90:
--settings-secondary-unchecked: {
On 2017/01/11 20:29:28, Dan Beam wrote:
> this is the only use of this mixin, afaict
> 
>
https://cs.chromium.org/search/?q=settings-secondary-unchecked&sq=package:chr...
> 
> please also remove it from
> chrome/browser/resources/controls/settings_{checkbox,toggle_button}.html

Yep, I remember that when I added the mixin this was the only usage, but I
wasn't sure about toggle_button.html which was added in the meantime.

The only settings-toggle-button with a subLabel seems to be the one for third
party cookies which doesn't actually hide the sublabel anyway, so I confirm that
we can remove it.

Done.

[msramek, 2017-01-12 12:38:56]

msramek@chromium.org changed reviewers:
+ dullweber@chromium.org

[msramek, 2017-01-12 12:38:57]

And since the assumption that the counter doesn't run for unchecked datatypes
was baked into all counter tests, I removed it from there and simplified the
counters where possible.

+Christian, can you please take a pass at all the browsing_data/ stuff?

[commit-bot: I haz the power, 2017-01-12 12:42:08]

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

[commit-bot: I haz the power, 2017-01-12 12:42:09]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[dullweber, 2017-01-12 12:50:55]

On 2017/01/12 12:38:57, msramek wrote:
> And since the assumption that the counter doesn't run for unchecked datatypes
> was baked into all counter tests, I removed it from there and simplified the
> counters where possible.
> 
> +Christian, can you please take a pass at all the browsing_data/ stuff?

The trybot is complaining about remaining pending_ attributes, otherwise lgtm

[msramek, 2017-01-12 14:16:23]

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

[commit-bot: I haz the power, 2017-01-12 14:16:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-01-12 14:19:40]

msramek@chromium.org changed reviewers:
+ droger@chromium.org

[msramek, 2017-01-12 14:19:41]

Thanks!

I fixed the iOS compilation now.

+David can you PTAL at ios/c/b/browsing_data? It's equivalent to
c/b/browsing_data changes which are already reviewed, the CacheCounter class is
just duplicated because of platform dependencies.

[commit-bot: I haz the power, 2017-01-12 14:22:14]

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

[commit-bot: I haz the power, 2017-01-12 14:22:15]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[msramek, 2017-01-12 14:46:27]

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

[commit-bot: I haz the power, 2017-01-12 14:46:45]

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-01-12 15:40:01]

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

[commit-bot: I haz the power, 2017-01-12 15:40:02]

Dry run: This issue passed the CQ dry run.

[droger, 2017-01-12 15:43:15]

lgtm

[Dan Beam, 2017-01-12 18:40:39]

lgtm

[msramek, 2017-01-12 21:20:34]

Thanks! Landing this now.

[msramek, 2017-01-12 21:20:43]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-01-12 21:20:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org, dullweber@chromium.org
Link to the patchset: https://codereview.chromium.org/2623033003/#ps120001
(title: "iOS fix #2")

[commit-bot: I haz the power, 2017-01-12 21:21:34]

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-01-12 21:27:43]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1484256043839020,
"parent_rev": "abe52699a2afc8416c810c73d5dbff94f60b1561", "commit_rev":
"e36f41845d3f6232aaeddf68fcde9576c7446344"}

[commit-bot: I haz the power, 2017-01-12 21:28:15]

Description was changed from

==========
Always show counters in the material design Clear Browsing Data dialog.

Previously, they were only shown when the corresponding datatype was
checked. This led to uneven spacing between checkboxes and arguably
strange visual behavior where text was appearing and disappearing.

See the linked bug for screenshots.

BUG=654812
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Always show counters in the material design Clear Browsing Data dialog.

Previously, they were only shown when the corresponding datatype was
checked. This led to uneven spacing between checkboxes and arguably
strange visual behavior where text was appearing and disappearing.

See the linked bug for screenshots.

BUG=654812
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2017-01-12 21:28:17]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/e36f41845d3f6232aaeddf68fcde...