Move data use measurement to DataUseNetworkDelegate

Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is more
appropriate to hook to DataUseNetworkDelegate, since getting web content
observer events other UI events will be easier (in future CLs) from
DataUseAscriber.

BUG=648808
Committed: https://crrev.com/14a5a85e424b65bcf740e9b4031d0af42f95b1b8
Cr-Commit-Position: refs/heads/master@{#432910}

[Raj, 2016-10-31 17:51:13]

Description was changed from

==========
Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is
more appropriate to hook to DataUseNetworkDelegate, since getting
callbacks from web content observers will be easier from
DataUseAscriber.

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

to

==========
Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is more
appropriate to hook to DataUseNetworkDelegate, since getting web content
observer events other UI events will be easier (in future CLs) from
DataUseAscriber.

BUG=648808
==========

[Raj, 2016-10-31 17:52:07]

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

[Raj, 2016-10-31 17:56:41]

rajendrant@chromium.org changed reviewers:
+ mattm@chromium.org, mlerman@chromium.org, rch@chromium.org

[Raj, 2016-10-31 17:56:41]

mattm: ptal chrome/browser/net/*

mlerman: ptal chrome/browser/profiles/*

rch: chrome/browser/io_thread.*

kundaji: ptal components/data_use_measurement/*

[Raj, 2016-10-31 17:58:22]

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

[commit-bot: I haz the power, 2016-10-31 17:58:49]

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-10-31 18:02:19]

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

[commit-bot: I haz the power, 2016-10-31 18:02:20]

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...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Raj, 2016-10-31 18:35:56]

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

[commit-bot: I haz the power, 2016-10-31 18:36:26]

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-10-31 18:40:24]

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

[commit-bot: I haz the power, 2016-10-31 18:40:28]

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...)

[Not at Google. Contact bengr, 2016-11-01 21:32:04]

Layering violations:

https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
File components/data_use_measurement/core/DEPS (right):

https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
components/data_use_measurement/core/DEPS:4: "+content/public/common",
data_use_measurement/core/ should not depend on content/.

I agree with the general direction of this cl that DataUseMeasurement could use
data_use_measurement/ rather than being hardcoded in ChromeNetworkDelegate.

You have a number of options:
1) Allow observers on DataUseAscriber. Then DataUseMeasurement could just be an
obsever of DataUseAscriber. This functionality will be required for other
purposes such as Page Size Pingback. You could postpone this cl until the
observer functionality is in, or add it yourself.

2) It seems the only reason DataUseMeasurement class is in content is because of
dependency on ResourceRequestInfo for IsUserInitiatedRequest().
DataUseMeasurement could be given a callback that given a URLRequest can tell
whether it is user initiated. The callback interface would be defined in core/
and the implementation under /content.

https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
File components/data_use_measurement/core/data_use_network_delegate.h (right):

https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
components/data_use_measurement/core/data_use_network_delegate.h:13: #include
"components/data_use_measurement/content/data_use_measurement.h"
data_use_measurement/core/ should not depend on data_use_measurement/content/.

[Raj, 2016-11-02 17:44:20]

rajendrant@chromium.org changed reviewers:
- mattm@chromium.org, mlerman@chromium.org, rch@chromium.org

[Raj, 2016-11-02 17:45:44]

On 2016/11/01 21:32:04, kundaji wrote:
> Layering violations:
> 
>
https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
> File components/data_use_measurement/core/DEPS (right):
> 
>
https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
> components/data_use_measurement/core/DEPS:4: "+content/public/common",
> data_use_measurement/core/ should not depend on content/.
> 
> I agree with the general direction of this cl that DataUseMeasurement could
use
> data_use_measurement/ rather than being hardcoded in ChromeNetworkDelegate.
> 
> You have a number of options:
> 1) Allow observers on DataUseAscriber. Then DataUseMeasurement could just be
an
> obsever of DataUseAscriber. This functionality will be required for other
> purposes such as Page Size Pingback. You could postpone this cl until the
> observer functionality is in, or add it yourself.
> 
> 2) It seems the only reason DataUseMeasurement class is in content is because
of
> dependency on ResourceRequestInfo for IsUserInitiatedRequest().
> DataUseMeasurement could be given a callback that given a URLRequest can tell
> whether it is user initiated. The callback interface would be defined in core/
> and the implementation under /content.
> 
>
https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
> File components/data_use_measurement/core/data_use_network_delegate.h (right):
> 
>
https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
> components/data_use_measurement/core/data_use_network_delegate.h:13: #include
> "components/data_use_measurement/content/data_use_measurement.h"
> data_use_measurement/core/ should not depend on data_use_measurement/content/.

Thanks.
I will look at the alternatives.

[Raj, 2016-11-08 22:39:31]

kundaji:

ptal. I will add other reviewers after your review.

https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
File components/data_use_measurement/core/DEPS (right):

https://codereview.chromium.org/2462983003/diff/20001/components/data_use_mea...
components/data_use_measurement/core/DEPS:4: "+content/public/common",
On 2016/11/01 21:32:04, kundaji wrote:
> data_use_measurement/core/ should not depend on content/.
> 
> I agree with the general direction of this cl that DataUseMeasurement could
use
> data_use_measurement/ rather than being hardcoded in ChromeNetworkDelegate.
> 
> You have a number of options:
> 1) Allow observers on DataUseAscriber. Then DataUseMeasurement could just be
an
> obsever of DataUseAscriber. This functionality will be required for other
> purposes such as Page Size Pingback. You could postpone this cl until the
> observer functionality is in, or add it yourself.
> 
> 2) It seems the only reason DataUseMeasurement class is in content is because
of
> dependency on ResourceRequestInfo for IsUserInitiatedRequest().
> DataUseMeasurement could be given a callback that given a URLRequest can tell
> whether it is user initiated. The callback interface would be defined in core/
> and the implementation under /content.

I am following option#2.

option#1 will not be enough for my usecase to get datause of various
combinations such as chrome app in background/foreground, chrome tab in
background/foreground, screen on/off, etc. Part of this info is obtained from
web content observers and remaining from other places. So DataUseMeasurement
needs to fuse all these info together. Observers on DataUseAscriber would be as
simple as returning datause for a tab/webcontent, and do not consider these
various combinations.

[Raj, 2016-11-09 20:31:44]

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

[Raj, 2016-11-09 20:31:45]

ryansturm: ptal
I will add other reviewers after your review.

[Not at Google. Contact bengr, 2016-11-09 22:04:10]

https://codereview.chromium.org/2462983003/diff/40001/chrome/browser/io_threa...
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/2462983003/diff/40001/chrome/browser/io_threa...
chrome/browser/io_thread.cc:536:
data_use_measurement::GetIsUserInitiatedRequestCallback(),
This method call seems awkward in this file. We should be able to avoid this
class knowing anything about the details of the data use network delegate.

Here is a possible approach:
Abstracted this to an interface, say RequestClassifier, which exposes the method
IsUserInitiatedRequest(..). A new pure virtual method in DataUseAscriber could
create instances of RequestClassifier. This method could be implemented in
ChromeDataUseAscriber to return a implementation of RequestClassifier that
depends on resource_request_info.h.

With this approach, this file will not need to know about the callback at all,
and the only change will be to pass metrics_data_use_forwarder_.

[Raj, 2016-11-10 21:08:59]

ptal

[RyanSturm, 2016-11-10 22:50:31]

A few comments, I'll need to look at this some more, but it makes sense.

https://codereview.chromium.org/2462983003/diff/60001/chrome/browser/data_use...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2462983003/diff/60001/chrome/browser/data_use...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:70:
ChromeDataUseAscriber::CreateURLRequestClassifier() {
const method

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
File components/data_use_measurement/content/chrome_url_request_classifier.cc
(right):

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/content/chrome_url_request_classifier.cc:12:
bool IsUserInitiatedRequest(const net::URLRequest& request) {
User Initiated usually means the user clicked a link and it caused a request or
a user hit enter to start the navigation request. I'd rename this method
"IsUserRequest".

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/content/chrome_url_request_classifier.cc:13: //
Having ResourceRequestInfo in the URL request is a sign that the request is
This can probably be shortened to:

"The presence of ResourecRequestInfo in |request| implies that this request was
created for a content::WebContents."

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
File components/data_use_measurement/content/chrome_url_request_classifier.h
(right):

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/content/chrome_url_request_classifier.h:19:
class ChromeURLRequestClassifier : public URLRequestClassifier {
s/ChromeURLRequestClassifier/ContentURLRequestClassifier/

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
File components/data_use_measurement/core/BUILD.gn (right):

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/core/BUILD.gn:16: static_library("ascriber") {
Why the seperate lib?

[Raj, 2016-11-10 23:20:27]

https://codereview.chromium.org/2462983003/diff/60001/chrome/browser/data_use...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2462983003/diff/60001/chrome/browser/data_use...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:70:
ChromeDataUseAscriber::CreateURLRequestClassifier() {
On 2016/11/10 22:50:30, Ryan Sturm wrote:
> const method

Done.

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
File components/data_use_measurement/content/chrome_url_request_classifier.cc
(right):

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/content/chrome_url_request_classifier.cc:12:
bool IsUserInitiatedRequest(const net::URLRequest& request) {
On 2016/11/10 22:50:30, Ryan Sturm wrote:
> User Initiated usually means the user clicked a link and it caused a request
or
> a user hit enter to start the navigation request. I'd rename this method
> "IsUserRequest".

Done.

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/content/chrome_url_request_classifier.cc:13: //
Having ResourceRequestInfo in the URL request is a sign that the request is
On 2016/11/10 22:50:30, Ryan Sturm wrote:
> This can probably be shortened to:
> 
> "The presence of ResourecRequestInfo in |request| implies that this request
was
> created for a content::WebContents."

Done.

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
File components/data_use_measurement/content/chrome_url_request_classifier.h
(right):

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/content/chrome_url_request_classifier.h:19:
class ChromeURLRequestClassifier : public URLRequestClassifier {
On 2016/11/10 22:50:30, Ryan Sturm wrote:
> s/ChromeURLRequestClassifier/ContentURLRequestClassifier/

Done.

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
File components/data_use_measurement/core/BUILD.gn (right):

https://codereview.chromium.org/2462983003/diff/60001/components/data_use_mea...
components/data_use_measurement/core/BUILD.gn:16: static_library("ascriber") {
On 2016/11/10 22:50:30, Ryan Sturm wrote:
> Why the seperate lib? 

Library with core files data_use_user_data.* is enough where AttachToFetcher()
is used to label other components/libs (search for
//components/data_use_measurement/core). Otherwise the ascriber files are added.

[Not at Google. Contact bengr, 2016-11-11 00:09:49]

lgtm

LGTM % nits.

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:70:
ChromeDataUseAscriber::CreateURLRequestClassifier() const {
Is this what you get from running "git cl format"?

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.h (right):

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.h:83: const
override;
Move above to be with the other overridden method.

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/io_thread.h
File chrome/browser/io_thread.h (right):

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/io_threa...
chrome/browser/io_thread.h:27: #include "components/metrics/data_use_tracker.h"
Not required?

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
File components/data_use_measurement/content/content_url_request_classifier.cc
(right):

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
components/data_use_measurement/content/content_url_request_classifier.cc:14: //
was
Fix formatting

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
File components/data_use_measurement/core/data_use_measurement_unittest.cc
(right):

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
components/data_use_measurement/core/data_use_measurement_unittest.cc:74:
UserRequestUserDataForTesting::MarkAsUserRequest(request.get());
Do you need this? Seems like you could use
data_use_measurement::DataUseUserData::kUserDataKey which is set in the else
below to check if it is a user request. But I might be missing something.

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
File components/data_use_measurement/core/data_use_network_delegate.h (right):

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
components/data_use_measurement/core/data_use_network_delegate.h:59: //
Component to measure data use.
Can you please make the comment more specific? It is reporting data use UMA,
right?

I think in the longer run, it will be cleaner to have this be a data use
observer rather than being hard coded here. But can put that off until data use
observers are supported by the ascriber.

[Raj, 2016-11-11 19:10:38]

rajendrant@chromium.org changed reviewers:
+ mmenke@chromium.org, sclittle@chromium.org

[Raj, 2016-11-11 19:10:39]

mmenke: ptal chrome/browser/net/* chrome/browser/io_thread.*

sclittle: ptal chrome/browser/precache/precache_util.cc

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:70:
ChromeDataUseAscriber::CreateURLRequestClassifier() const {
On 2016/11/11 00:09:48, kundaji wrote:
> Is this what you get from running "git cl format"?

Yes.

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.h (right):

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/data_use...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.h:83: const
override;
On 2016/11/11 00:09:48, kundaji wrote:
> Move above to be with the other overridden method.

Done.

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/io_thread.h
File chrome/browser/io_thread.h (right):

https://codereview.chromium.org/2462983003/diff/80001/chrome/browser/io_threa...
chrome/browser/io_thread.h:27: #include "components/metrics/data_use_tracker.h"
On 2016/11/11 00:09:48, kundaji wrote:
> Not required?

Still needed. |metrics_data_use_forwarder_| is used here.

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
File components/data_use_measurement/content/content_url_request_classifier.cc
(right):

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
components/data_use_measurement/content/content_url_request_classifier.cc:14: //
was
On 2016/11/11 00:09:49, kundaji wrote:
> Fix formatting

Done.

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
File components/data_use_measurement/core/data_use_measurement_unittest.cc
(right):

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
components/data_use_measurement/core/data_use_measurement_unittest.cc:74:
UserRequestUserDataForTesting::MarkAsUserRequest(request.get());
On 2016/11/11 00:09:49, kundaji wrote:
> Do you need this? Seems like you could use
> data_use_measurement::DataUseUserData::kUserDataKey which is set in the else
> below to check if it is a user request. But I might be missing something.

DataUseUserData::kUserDataKey only has services information. It does not
distinguish user vs service request. I would change it in other CL, so that it
is easy for testing.

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
File components/data_use_measurement/core/data_use_network_delegate.h (right):

https://codereview.chromium.org/2462983003/diff/80001/components/data_use_mea...
components/data_use_measurement/core/data_use_network_delegate.h:59: //
Component to measure data use.
On 2016/11/11 00:09:49, kundaji wrote:
> Can you please make the comment more specific? It is reporting data use UMA,
> right?
> 
> I think in the longer run, it will be cleaner to have this be a data use
> observer rather than being hard coded here. But can put that off until data
use
> observers are supported by the ascriber.

Done.

[mmenke, 2016-11-11 19:26:58]

chrome/browser/net / chrome/browser/io_thread LGTM!

[Raj, 2016-11-11 19:39:14]

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

[Raj, 2016-11-11 19:39:15]

asargent: ptal
chrome/browser/extensions/api/web_request/web_request_api_unittest.cc

[asargent_no_longer_on_chrome, 2016-11-12 00:48:29]

chrome/browser/extensions/api/web_request/web_request_api_unittest.cc lgtm

[Raj, 2016-11-12 07:39:53]

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

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

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-11-12 08:47:09]

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

[commit-bot: I haz the power, 2016-11-12 08:47:10]

Dry run: This issue passed the CQ dry run.

[Raj, 2016-11-12 18:58:20]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-11-12 18:58:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
kundaji@chromium.org, asargent@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2462983003/#ps120001
(title: "Rebased")

[commit-bot: I haz the power, 2016-11-12 18:58: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-11-12 19:05:15]

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

[commit-bot: I haz the power, 2016-11-12 19:05:17]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sclittle, 2016-11-15 19:23:27]

components/data_use_measurement and chrome/browser/precache LGTM

[Raj, 2016-11-15 19:23:46]

The CQ bit was checked by rajendrant@chromium.org

[commit-bot: I haz the power, 2016-11-15 19:24:40]

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-11-15 19:34:30]

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

[commit-bot: I haz the power, 2016-11-15 19:34:32]

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...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Raj, 2016-11-15 19:43:55]

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

[Raj, 2016-11-15 19:43:56]

blundell: ptal components/BUILD.gn

[blundell, 2016-11-16 10:13:18]

//components/BUILD.gn lgtm

https://codereview.chromium.org/2462983003/diff/120001/components/data_use_me...
File components/data_use_measurement/core/BUILD.gn (right):

https://codereview.chromium.org/2462983003/diff/120001/components/data_use_me...
components/data_use_measurement/core/BUILD.gn:32:
"//components/metrics:metrics",
nit: you don't need the ":metrics" at the end here.

[Raj, 2016-11-16 17:44:29]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-11-16 17:44:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
sclittle@chromium.org, kundaji@chromium.org, asargent@chromium.org,
mmenke@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2462983003/#ps140001
(title: "Rebased")

[commit-bot: I haz the power, 2016-11-16 17:45:25]

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

[Raj, 2016-11-16 17:47:33]

Patchset #8 (id:140001) has been deleted

[Raj, 2016-11-16 17:47:51]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-11-16 17:47:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
sclittle@chromium.org, kundaji@chromium.org, asargent@chromium.org,
mmenke@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2462983003/#ps160001
(title: "Rebased and fixed nits")

[Raj, 2016-11-16 17:48:13]

https://codereview.chromium.org/2462983003/diff/120001/components/data_use_me...
File components/data_use_measurement/core/BUILD.gn (right):

https://codereview.chromium.org/2462983003/diff/120001/components/data_use_me...
components/data_use_measurement/core/BUILD.gn:32:
"//components/metrics:metrics",
On 2016/11/16 10:13:18, blundell wrote:
> nit: you don't need the ":metrics" at the end here.

Done.

[commit-bot: I haz the power, 2016-11-16 17:48:36]

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-11-16 21:57:07]

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

[commit-bot: I haz the power, 2016-11-16 21:57:08]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raj, 2016-11-17 07:42:51]

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

[Raj, 2016-11-17 07:42:58]

Patchset #8 (id:160001) has been deleted

[commit-bot: I haz the power, 2016-11-17 07:43:10]

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-11-17 10:04:29]

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

[commit-bot: I haz the power, 2016-11-17 10:04:30]

Dry run: This issue passed the CQ dry run.

[Raj, 2016-11-17 17:47:13]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-11-17 17:47:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
sclittle@chromium.org, kundaji@chromium.org, asargent@chromium.org,
mmenke@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2462983003/#ps180001
(title: "Rebased and fixed nits")

[commit-bot: I haz the power, 2016-11-17 17:47:59]

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-11-17 17:59:20]

Description was changed from

==========
Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is more
appropriate to hook to DataUseNetworkDelegate, since getting web content
observer events other UI events will be easier (in future CLs) from
DataUseAscriber.

BUG=648808
==========

to

==========
Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is more
appropriate to hook to DataUseNetworkDelegate, since getting web content
observer events other UI events will be easier (in future CLs) from
DataUseAscriber.

BUG=648808
==========

[commit-bot: I haz the power, 2016-11-17 17:59:22]

Committed patchset #8 (id:180001)

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

Description was changed from

==========
Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is more
appropriate to hook to DataUseNetworkDelegate, since getting web content
observer events other UI events will be easier (in future CLs) from
DataUseAscriber.

BUG=648808
==========

to

==========
Move data use measurement to DataUseNetworkDelegate

Currently data_use_measurement hooks to ChromeNetworkDelegate. It is more
appropriate to hook to DataUseNetworkDelegate, since getting web content
observer events other UI events will be easier (in future CLs) from
DataUseAscriber.

BUG=648808

Committed: https://crrev.com/14a5a85e424b65bcf740e9b4031d0af42f95b1b8
Cr-Commit-Position: refs/heads/master@{#432910}
==========

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

Patchset 8 (id:??) landed as
https://crrev.com/14a5a85e424b65bcf740e9b4031d0af42f95b1b8
Cr-Commit-Position: refs/heads/master@{#432910}