Move browsing data counters helper functions to an 'utils' file.

Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on; since the next step is to introduce the experiment on Android, the function now has two defaults: true on Desktop and false on Android
  - counter text output; it's necessary to compute this in C++, since on Android side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android (https://codereview.chromium.org/1530123002/)

BUG=569870
Committed: https://crrev.com/f686e096aa1518fecc9e538618fde61f506e03bb
Cr-Commit-Position: refs/heads/master@{#367564}

[msramek, 2015-12-16 13:12:12]

Description was changed from

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

BUG=569870
==========

to

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on
  - counter text output -> it's necessary to compute this in C++, since on
Android side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870
==========

[msramek, 2015-12-16 13:17:14]

Description was changed from

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on
  - counter text output -> it's necessary to compute this in C++, since on
Android side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870
==========

to

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on; since the next step is to
introduce the experiment on Android, the function now has two defaults: true on
Desktop and false on Android
  - counter text output; it's necessary to compute this in C++, since on Android
side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870
==========

[msramek, 2015-12-16 13:19:14]

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

[msramek, 2015-12-16 13:19:15]

Hi Bernhard,

can you please have a look at this?

Thanks,
Martin

[Bernhard Bauer, 2015-12-16 13:35:32]

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

https://codereview.chromium.org/1527653004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.h:10: namespace
browsing_data {
I think the general guideline now is against using namespaces in top-level
chrome/ code. If anything, the namespace should be named after the file name,
i.e. browsing_data_counter_utils? That does seem a bit unwieldy, but maybe you
can import these with `using` where they are used.

https://codereview.chromium.org/1527653004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.h:16: base::string16
GetCounterTextFromResult(
#include "base/string16.h"

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

https://codereview.chromium.org/1527653004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc:16:
TEST(BrowsingDataHelpersTest, AutofillCounterResult) {
Should this be called BrowsingDataCounterUtilsTest?

[msramek, 2016-01-05 16:22:50]

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

https://codereview.chromium.org/1527653004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.h:10: namespace
browsing_data {
On 2015/12/16 13:35:32, Bernhard Bauer wrote:
> I think the general guideline now is against using namespaces in top-level
> chrome/ code. If anything, the namespace should be named after the file name,
> i.e. browsing_data_counter_utils? That does seem a bit unwieldy, but maybe you
> can import these with `using` where they are used.

I removed the namespace; it's true that other files in this directory don't use
one either.

I found this thread 
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/namesp...
on chromium-dev, which is probably what you're referring to. But the style guide
itself pretty much encourages using namespaces everywhere.

https://codereview.chromium.org/1527653004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils.h:16: base::string16
GetCounterTextFromResult(
On 2015/12/16 13:35:32, Bernhard Bauer wrote:
> #include "base/string16.h"

Done.

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

https://codereview.chromium.org/1527653004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter_utils_unittest.cc:16:
TEST(BrowsingDataHelpersTest, AutofillCounterResult) {
On 2015/12/16 13:35:32, Bernhard Bauer wrote:
> Should this be called BrowsingDataCounterUtilsTest?

Yes! I originally had the file named helpers :)

[Bernhard Bauer, 2016-01-05 16:33:06]

lgtm

[msramek, 2016-01-05 16:51:24]

Patchset #4 (id:60001) has been deleted

[msramek, 2016-01-05 16:54:05]

Patchset #4 (id:80001) has been deleted

[msramek, 2016-01-05 17:17:44]

The CQ bit was checked by msramek@chromium.org

[msramek, 2016-01-05 17:17:45]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1527653004/#ps100001
(title: "Rebase, solved conflict in #includes")

[commit-bot: I haz the power, 2016-01-05 17:17:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1527653004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1527653004/100001

[commit-bot: I haz the power, 2016-01-05 17:49:04]

Description was changed from

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on; since the next step is to
introduce the experiment on Android, the function now has two defaults: true on
Desktop and false on Android
  - counter text output; it's necessary to compute this in C++, since on Android
side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870
==========

to

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on; since the next step is to
introduce the experiment on Android, the function now has two defaults: true on
Desktop and false on Android
  - counter text output; it's necessary to compute this in C++, since on Android
side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870
==========

[commit-bot: I haz the power, 2016-01-05 17:49:05]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-01-05 17:50:09]

Description was changed from

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on; since the next step is to
introduce the experiment on Android, the function now has two defaults: true on
Desktop and false on Android
  - counter text output; it's necessary to compute this in C++, since on Android
side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870
==========

to

==========
Move browsing data counters helper functions to an 'utils' file.

This cleans up the code a bit and makes it easier to share this functionality
between the Desktop and Android implementation.

Currently, the 'utils' file contains:
  - the method to check if the experiment is on; since the next step is to
introduce the experiment on Android, the function now has two defaults: true on
Desktop and false on Android
  - counter text output; it's necessary to compute this in C++, since on Android
side we don't have good ICU string support
  - pref->counter factory; this is currently only to be used on Android
(https://codereview.chromium.org/1530123002/)

BUG=569870

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

[commit-bot: I haz the power, 2016-01-05 17:50:10]

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