Move part of browsing data counter text util method to components

Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter, AutofillCounter and HistoryCounter.

TBRing for trivial method rename in: c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browser_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
Committed: https://crrev.com/556a0adf05225bd20e66cff7a7553838c4170b40
Cr-Commit-Position: refs/heads/master@{#412227}

[ioanap, 2016-08-09 14:43:00]

Description was changed from

==========
Move part of counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

BUG=
==========

to

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

BUG=
==========

[ioanap, 2016-08-09 14:44:59]

The CQ bit was checked by ioanap@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 14:45:17]

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

[ioanap, 2016-08-09 15:47:08]

Description was changed from

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

BUG=
==========

to

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

BUG=620317
==========

[ioanap, 2016-08-09 15:50:10]

ioanap@google.com changed reviewers:
+ msramek@chromium.org

[ioanap, 2016-08-09 15:50:10]

Please take a look!

Thanks,
Ioana

[msramek, 2016-08-09 16:28:33]

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (left):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:237: bool
GetDeletionPreferenceFromDataType(
Technically unrelated to this CL, but is there a reason why this doesn't live in
//components? This method translates between the enum and prefs, which are both
defined in //components. Is that just an oversight from
https://codereview.chromium.org/2084903002 ?

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (right):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:15: #include
"components/browsing_data/core/counters/autofill_counter.h"
Some of these includes are no longer needed.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:68: pref_name ==
browsing_data::prefs::kDeleteBrowsingHistory ||
Download history seems to have disappeared?

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:70: text =
browsing_data::GetCounterTextFromResult(result);
I would restructure this a bit, so that the last line of this method is:

return browsing_data::GetCounterTextFromResult(result);

In other words, let's process //chrome/browser-specific options first, then
defer to browsing_data::GetCounterTextFromResult().

browsing_data::GetCounterTextFromResult() will then be responsible for handling
errors if there was no match found.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc
(right):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc:80:
base::string16 output = GetChromeCounterTextFromResult(&result);
The autofill-related functionality now lives in GetCounterTextFromResult(). So
this should be changed back and instead, this entire test should be moved to
//components.

https://codereview.chromium.org/2228913003/diff/20001/components/browsing_dat...
File components/browsing_data_strings.grdp (right):

https://codereview.chromium.org/2228913003/diff/20001/components/browsing_dat...
components/browsing_data_strings.grdp:6: Calculating...
Style: Offset 2 spaces from the previous line.

[ioanap, 2016-08-10 09:33:46]

The CQ bit was checked by ioanap@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 09:34:05]

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

[ioanap, 2016-08-10 09:34:37]

Addressed comments!

Thanks,
Ioana

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (left):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:237: bool
GetDeletionPreferenceFromDataType(
On 2016/08/09 16:28:32, msramek wrote:
> Technically unrelated to this CL, but is there a reason why this doesn't live
in
> //components? This method translates between the enum and prefs, which are
both
> defined in //components. Is that just an oversight from
> https://codereview.chromium.org/2084903002 ?

By looking at that CL, I think this was not moved because the pref names were
not in components/ yet. I will move it in a separate CL.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (right):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:15: #include
"components/browsing_data/core/counters/autofill_counter.h"
On 2016/08/09 16:28:32, msramek wrote:
> Some of these includes are no longer needed.

Done.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:68: pref_name ==
browsing_data::prefs::kDeleteBrowsingHistory ||
On 2016/08/09 16:28:32, msramek wrote:
> Download history seems to have disappeared?
 
Thank you for spotting it! I moved it now to the utils in components.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:70: text =
browsing_data::GetCounterTextFromResult(result);
On 2016/08/09 16:28:32, msramek wrote:
> I would restructure this a bit, so that the last line of this method is:
> 
> return browsing_data::GetCounterTextFromResult(result);
> 
> In other words, let's process //chrome/browser-specific options first, then
> defer to browsing_data::GetCounterTextFromResult().
> 
> browsing_data::GetCounterTextFromResult() will then be responsible for
handling
> errors if there was no match found.

Done.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc
(right):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc:80:
base::string16 output = GetChromeCounterTextFromResult(&result);
On 2016/08/09 16:28:32, msramek wrote:
> The autofill-related functionality now lives in GetCounterTextFromResult(). So
> this should be changed back and instead, this entire test should be moved to
> //components.

Done.

https://codereview.chromium.org/2228913003/diff/20001/components/browsing_dat...
File components/browsing_data_strings.grdp (right):

https://codereview.chromium.org/2228913003/diff/20001/components/browsing_dat...
components/browsing_data_strings.grdp:6: Calculating...
On 2016/08/09 16:28:32, msramek wrote:
> Style: Offset 2 spaces from the previous line.

Done.

[commit-bot: I haz the power, 2016-08-10 09:37:09]

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

[commit-bot: I haz the power, 2016-08-10 09:37:11]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[ioanap, 2016-08-10 09:46:23]

The CQ bit was checked by ioanap@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 09:46:35]

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

[msramek, 2016-08-10 09:59:50]

LGTM with 2 nits (sorry for nitpicking)

Please make sure to test chrome://md-settings/clearBrowserData on Desktop before
landing to make sure we handle all datatypes.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (left):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:237: bool
GetDeletionPreferenceFromDataType(
On 2016/08/10 09:34:37, ioanap wrote:
> On 2016/08/09 16:28:32, msramek wrote:
> > Technically unrelated to this CL, but is there a reason why this doesn't
live
> in
> > //components? This method translates between the enum and prefs, which are
> both
> > defined in //components. Is that just an oversight from
> > https://codereview.chromium.org/2084903002 ?
> 
> By looking at that CL, I think this was not moved because the pref names were
> not in components/ yet. I will move it in a separate CL.

Acknowledged.

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc
(right):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc:80:
base::string16 output = GetChromeCounterTextFromResult(&result);
On 2016/08/10 09:34:37, ioanap wrote:
> On 2016/08/09 16:28:32, msramek wrote:
> > The autofill-related functionality now lives in GetCounterTextFromResult().
So
> > this should be changed back and instead, this entire test should be moved to
> > //components.
> 
> Done.

Just for the record - for maximum pedantness, this whole file should be only
compiled if ENABLE_EXTENSIONS are defined, since that is the only functionality
left. But I think that would make things very confusing as
browsing_data_counter_utils.cc is compiled always, so let's just leave it as it
is.

https://codereview.chromium.org/2228913003/diff/60001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (right):

https://codereview.chromium.org/2228913003/diff/60001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:137: return
browsing_data::GetCounterTextFromResult(result);
nit: Empty line above this line please :)

https://codereview.chromium.org/2228913003/diff/60001/components/browsing_dat...
File components/browsing_data/core/browsing_data_utils.cc (right):

https://codereview.chromium.org/2228913003/diff/60001/components/browsing_dat...
components/browsing_data/core/browsing_data_utils.cc:140: return text;
nit: Please also add an empty line above this return.

[commit-bot: I haz the power, 2016-08-10 10:40:00]

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

[commit-bot: I haz the power, 2016-08-10 10:40:01]

Dry run: This issue passed the CQ dry run.

[ioanap, 2016-08-10 11:12:32]

Checked md-settings too. They're all in place now!

Thanks,
Ioana

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc
(right):

https://codereview.chromium.org/2228913003/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc:80:
base::string16 output = GetChromeCounterTextFromResult(&result);
On 2016/08/10 09:59:50, msramek wrote:
> On 2016/08/10 09:34:37, ioanap wrote:
> > On 2016/08/09 16:28:32, msramek wrote:
> > > The autofill-related functionality now lives in
GetCounterTextFromResult().
> So
> > > this should be changed back and instead, this entire test should be moved
to
> > > //components.
> > 
> > Done.
> 
> Just for the record - for maximum pedantness, this whole file should be only
> compiled if ENABLE_EXTENSIONS are defined, since that is the only
functionality
> left. But I think that would make things very confusing as
> browsing_data_counter_utils.cc is compiled always, so let's just leave it as
it
> is.

Acknowledged. Thank you!

https://codereview.chromium.org/2228913003/diff/60001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter_utils.cc (right):

https://codereview.chromium.org/2228913003/diff/60001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.cc:137: return
browsing_data::GetCounterTextFromResult(result);
On 2016/08/10 09:59:50, msramek wrote:
> nit: Empty line above this line please :)

Done.

https://codereview.chromium.org/2228913003/diff/60001/components/browsing_dat...
File components/browsing_data/core/browsing_data_utils.cc (right):

https://codereview.chromium.org/2228913003/diff/60001/components/browsing_dat...
components/browsing_data/core/browsing_data_utils.cc:140: return text;
On 2016/08/10 09:59:50, msramek wrote:
> nit: Please also add an empty line above this return.

Done.

[ioanap, 2016-08-10 11:20:08]

ioanap@google.com changed reviewers:
+ sdefresne@chromium.org, sky@chromium.org

[ioanap, 2016-08-10 11:20:09]

+ sdefresne@: For changes in top level components/

+ sky@: For deps on "ui/base" in components/browsing_data/core/DEPS

Please take a look!

Thanks,
Ioana

[sky, 2016-08-10 15:13:15]

LGTM

[ioanap, 2016-08-11 09:23:58]

The CQ bit was checked by ioanap@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 09:24:04]

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

[ioanap, 2016-08-11 09:35:40]

Description was changed from

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

BUG=620317
==========

to

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browsing_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
==========

[ioanap, 2016-08-11 09:37:09]

Description was changed from

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browsing_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
==========

to

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browser_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
==========

[commit-bot: I haz the power, 2016-08-11 11:09:17]

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

[commit-bot: I haz the power, 2016-08-11 11:09:18]

Dry run: This issue passed the CQ dry run.

[ioanap, 2016-08-16 09:43:19]

ioanap@google.com changed reviewers:
+ blundell@chromium.org
- sdefresne@chromium.org

[ioanap, 2016-08-16 09:43:19]

-sdefresne@ (OOO)

+blundell@ for changes in top level components/

Please take a look!

Thanks,
Ioana

[blundell, 2016-08-16 09:50:01]

lgtm

[ioanap, 2016-08-16 11:13:16]

The CQ bit was checked by ioanap@google.com

[ioanap, 2016-08-16 11:13:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2228913003/#ps140001
(title: "Fixed call site in Android code")

[commit-bot: I haz the power, 2016-08-16 11:13:29]

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-08-16 11:15:43]

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

[commit-bot: I haz the power, 2016-08-16 11:15:45]

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

[ioanap, 2016-08-16 12:39:07]

The CQ bit was checked by ioanap@google.com

[ioanap, 2016-08-16 12:39:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, sky@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2228913003/#ps160001
(title: "Rebase")

[ioanap, 2016-08-16 12:39:07]

The CQ bit was unchecked by ioanap@google.com

[ioanap, 2016-08-16 12:39:09]

The CQ bit was checked by ioanap@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 12:39:17]

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-08-16 13:33:33]

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

[commit-bot: I haz the power, 2016-08-16 13:33:34]

Dry run: This issue passed the CQ dry run.

[ioanap, 2016-08-16 13:36:15]

The CQ bit was checked by ioanap@google.com

[commit-bot: I haz the power, 2016-08-16 13:36:26]

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-08-16 13:40:09]

Description was changed from

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browser_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
==========

to

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browser_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
==========

[commit-bot: I haz the power, 2016-08-16 13:40:11]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-08-16 13:41:58]

Description was changed from

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browser_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317
==========

to

==========
Move part of browsing data counter text util method to components

Extract part of the method that constructs the text for browsing data counters
into the utils in components/browsing_data/core/. This will return the text
for the counters that have been moved to components: PasswordsCounter,
AutofillCounter and HistoryCounter.

TBRing for trivial method rename in:
c/b/android/browsing_data/browsing_data_counter_bridge.cc
c/b/ui/webui/settings/settings_clear_browsing_data_handler.cc
c/b/ui/webui/options/clear_browser_data_handler.cc

TBR=bauerb@chromium.org, dbeam@chromium.org
BUG=620317

Committed: https://crrev.com/556a0adf05225bd20e66cff7a7553838c4170b40
Cr-Commit-Position: refs/heads/master@{#412227}
==========

[commit-bot: I haz the power, 2016-08-16 13:42:00]

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