Make ExternalDataUseReporter independent of other data use classes

Make ExternalDataUseReporter independent of other data use classes

Removes DataUseTabModel and ExternalDataUseObserverBridge dependency on
ExternalDataUseReporter by passing necessary callbacks.

BUG=570884
Committed: https://crrev.com/55ffa569a94c166734ef7beef707714f366d4f3b
Cr-Commit-Position: refs/heads/master@{#406926}

[Raj, 2016-07-20 23:28:41]

rajendrant@chromium.org changed reviewers:
+ tbansal@chromium.org

[Raj, 2016-07-20 23:28:41]

ptal

[tbansal1, 2016-07-21 00:19:30]

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/data_use_tab_model.h (right):

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/data_use_tab_model.h:154: const
base::TimeTicks& timestamp,
Passing by value is preferable.
See https://cs.chromium.org/chromium/src/base/time/time.h?rcl=0&l=24

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/external_data_use_reporter.cc (right):

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.cc:101: const
base::Callback<bool(SessionID::id_type,
You can use typedef here too.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/external_data_use_reporter.h (right):

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.h:74: const
base::Time&,
Pass by value.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.h:205: const
base::Callback<bool(SessionID::id_type,
Can this be typedef'ed so there is no need to type this long list multiple
times.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.h:211: const
base::Callback<void(const std::string&,
Same here, typedefing may make it more readable.

[Raj, 2016-07-21 04:08:49]

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

[commit-bot: I haz the power, 2016-07-21 04:09:08]

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

[Raj, 2016-07-21 04:09:17]

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/data_use_tab_model.h (right):

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/data_use_tab_model.h:154: const
base::TimeTicks& timestamp,
On 2016/07/21 00:19:30, tbansal1 wrote:
> Passing by value is preferable.
> See https://cs.chromium.org/chromium/src/base/time/time.h?rcl=0&l=24

Done.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/external_data_use_reporter.cc (right):

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.cc:101: const
base::Callback<bool(SessionID::id_type,
On 2016/07/21 00:19:30, tbansal1 wrote:
> You can use typedef here too.

Done.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/external_data_use_reporter.h (right):

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.h:74: const
base::Time&,
On 2016/07/21 00:19:30, tbansal1 wrote:
> Pass by value.

Done.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.h:205: const
base::Callback<bool(SessionID::id_type,
On 2016/07/21 00:19:30, tbansal1 wrote:
> Can this be typedef'ed so there is no need to type this long list multiple
> times.

Done.

https://codereview.chromium.org/2165123002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_reporter.h:211: const
base::Callback<void(const std::string&,
On 2016/07/21 00:19:30, tbansal1 wrote:
> Same here, typedefing may make it more readable.

Done.

[commit-bot: I haz the power, 2016-07-21 04:11:06]

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

[commit-bot: I haz the power, 2016-07-21 04:11:07]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2016-07-21 15:45:02]

lgtm % nits.

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
File chrome/browser/android/data_usage/data_use_tab_model.h (right):

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
chrome/browser/android/data_usage/data_use_tab_model.h:154: const
base::TimeTicks timestamp,
Typically, const is not added to variables passed by value.

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.h
(right):

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
chrome/browser/android/data_usage/external_data_use_observer_bridge.h:78: const
base::Time start_time,
const not needed (although it is unrelated to this CL).

[Raj, 2016-07-21 17:55:30]

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

[commit-bot: I haz the power, 2016-07-21 17:56:14]

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

[Raj, 2016-07-21 17:56:44]

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

[Raj, 2016-07-21 17:56:56]

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
File chrome/browser/android/data_usage/data_use_tab_model.h (right):

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
chrome/browser/android/data_usage/data_use_tab_model.h:154: const
base::TimeTicks timestamp,
On 2016/07/21 15:45:01, tbansal1 wrote:
> Typically, const is not added to variables passed by value.

Done.

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.h
(right):

https://codereview.chromium.org/2165123002/diff/20001/chrome/browser/android/...
chrome/browser/android/data_usage/external_data_use_observer_bridge.h:78: const
base::Time start_time,
On 2016/07/21 15:45:01, tbansal1 wrote:
> const not needed (although it is unrelated to this CL).

Done.

[commit-bot: I haz the power, 2016-07-21 17:57:06]

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

[Raj, 2016-07-21 18:51:44]

The CQ bit was unchecked by rajendrant@chromium.org

[Raj, 2016-07-21 18:51:49]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-07-21 18:51:50]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2165123002/#ps40001
(title: "Addressed nits")

[commit-bot: I haz the power, 2016-07-21 18:52:19]

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-07-21 19:10:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-21 19:11:58]

Description was changed from

==========
Make ExternalDataUseReporter independent of other data use classes

Removes DataUseTabModel and ExternalDataUseObserverBridge dependency on
ExternalDataUseReporter by passing necessary callbacks.

BUG=570884
==========

to

==========
Make ExternalDataUseReporter independent of other data use classes

Removes DataUseTabModel and ExternalDataUseObserverBridge dependency on
ExternalDataUseReporter by passing necessary callbacks.

BUG=570884

Committed: https://crrev.com/55ffa569a94c166734ef7beef707714f366d4f3b
Cr-Commit-Position: refs/heads/master@{#406926}
==========

[commit-bot: I haz the power, 2016-07-21 19:11:59]

Patchset 3 (id:??) landed as
https://crrev.com/55ffa569a94c166734ef7beef707714f366d4f3b
Cr-Commit-Position: refs/heads/master@{#406926}