Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(21)

Issue 2614203002: Record data use of user traffic by different core page transition types (Closed)

Created:
3 years, 11 months ago by Raj
Modified:
3 years, 9 months ago
CC:
chromium-reviews, droger+watchlist_chromium.org, blundell+watchlist_chromium.org, sdefresne+watchlist_chromium.org, jam, darin-cc_chromium.org, asvitkine+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Record data use of user traffic by different core page transition types BUG=679026 Review-Url: https://codereview.chromium.org/2614203002 Cr-Commit-Position: refs/heads/master@{#458347} Committed: https://chromium.googlesource.com/chromium/src/+/002f07fbe7776175f47bf49a284f673e26c409b3

Patch Set 1 #

Total comments: 12

Patch Set 2 : Addressed comments #

Total comments: 3

Patch Set 3 : rebased #

Patch Set 4 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+209 lines, -4 lines) Patch
M chrome/browser/data_use_measurement/chrome_data_use_ascriber.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc View 1 2 3 1 chunk +10 lines, -0 lines 0 comments Download
M chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.cc View 1 2 3 1 chunk +21 lines, -0 lines 0 comments Download
M chrome/browser/data_use_measurement/chrome_data_use_recorder.cc View 1 chunk +4 lines, -1 line 0 comments Download
M chrome/browser/data_use_measurement/data_use_web_contents_observer.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/data_use_measurement/data_use_web_contents_observer.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M components/data_use_measurement/content/BUILD.gn View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M components/data_use_measurement/content/DEPS View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M components/data_use_measurement/content/content_url_request_classifier.h View 1 2 1 chunk +5 lines, -1 line 0 comments Download
M components/data_use_measurement/content/content_url_request_classifier.cc View 1 2 3 2 chunks +89 lines, -0 lines 0 comments Download
M components/data_use_measurement/core/data_use_measurement.h View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M components/data_use_measurement/core/data_use_measurement.cc View 1 2 3 3 chunks +14 lines, -1 line 0 comments Download
M components/data_use_measurement/core/data_use_measurement_unittest.cc View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M components/data_use_measurement/core/data_use_network_delegate_unittest.cc View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M components/data_use_measurement/core/data_use_recorder.h View 1 2 chunks +9 lines, -0 lines 0 comments Download
M components/data_use_measurement/core/url_request_classifier.h View 1 2 2 chunks +6 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 2 chunks +25 lines, -0 lines 0 comments Download

Messages

Total messages: 26 (10 generated)
Raj
ptal. Not sure if there is any easy way to add unittests for this.
3 years, 11 months ago (2017-01-06 21:05:14 UTC) #2
RyanSturm
Hey, I won't have time to fully review this today, unfortunately. I will probably have ...
3 years, 11 months ago (2017-01-06 21:56:39 UTC) #3
Raj
On 2017/01/06 21:56:39, Ryan Sturm wrote: > Hey, I won't have time to fully review ...
3 years, 11 months ago (2017-01-06 22:12:31 UTC) #4
RyanSturm
Here's a few more more nits. The design looks good aside from needing to track ...
3 years, 11 months ago (2017-01-10 18:32:24 UTC) #5
Raj
https://codereview.chromium.org/2614203002/diff/1/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h File chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h (right): https://codereview.chromium.org/2614203002/diff/1/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h#newcode66 chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h:66: void DidFinishNavigation(content::NavigationHandle* navigation_handle); On 2017/01/10 18:32:23, Ryan Sturm wrote: ...
3 years, 11 months ago (2017-01-10 22:21:58 UTC) #6
RyanSturm
lgtm % nit https://codereview.chromium.org/2614203002/diff/20001/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h File chrome/browser/data_use_measurement/chrome_data_use_ascriber.h (right): https://codereview.chromium.org/2614203002/diff/20001/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h#newcode107 chrome/browser/data_use_measurement/chrome_data_use_ascriber.h:107: void DidFinishNavigation(int render_process_id, method Comment.
3 years, 11 months ago (2017-01-10 22:56:19 UTC) #7
Raj
rkaplow: ptal histograms.xml
3 years, 11 months ago (2017-01-12 03:26:58 UTC) #9
rkaplow
https://codereview.chromium.org/2614203002/diff/20001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2614203002/diff/20001/tools/metrics/histograms/histograms.xml#newcode9531 tools/metrics/histograms/histograms.xml:9531: +<histogram name="DataUse.PageTransition.UserTraffic" I don't think this design is correct, ...
3 years, 11 months ago (2017-01-12 16:28:06 UTC) #10
Raj
rkaplow: ptal https://codereview.chromium.org/2614203002/diff/20001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2614203002/diff/20001/tools/metrics/histograms/histograms.xml#newcode9531 tools/metrics/histograms/histograms.xml:9531: +<histogram name="DataUse.PageTransition.UserTraffic" On 2017/01/12 16:28:06, rkaplow wrote: ...
3 years, 10 months ago (2017-02-14 22:52:51 UTC) #11
rkaplow
lgtm
3 years, 10 months ago (2017-02-15 15:28:06 UTC) #12
Raj
ccameron: ptal components/data_use_measurement/content/DEPS
3 years, 10 months ago (2017-02-22 19:44:39 UTC) #14
ccameron
On 2017/02/22 19:44:39, Raj wrote: > ccameron: ptal components/data_use_measurement/content/DEPS lgtm
3 years, 9 months ago (2017-03-20 18:05:56 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2614203002/40001
3 years, 9 months ago (2017-03-20 19:50:26 UTC) #18
commit-bot: I haz the power
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/173600) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, ...
3 years, 9 months ago (2017-03-20 19:59:11 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2614203002/60001
3 years, 9 months ago (2017-03-21 06:40:44 UTC) #23
commit-bot: I haz the power
3 years, 9 months ago (2017-03-21 07:42:49 UTC) #26
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/002f07fbe7776175f47bf49a284f...

Powered by Google App Engine
This is Rietveld 408576698