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

Issue 2375773002: Add data usage tracking for feedback uploader, crash uploader, dom distiller (Closed)

Created:
4 years, 2 months ago by Raj
Modified:
4 years, 2 months ago
CC:
chromium-reviews, blundell+watchlist_chromium.org, sdefresne+watchlist_chromium.org, asvitkine+watch_chromium.org, droger+watchlist_chromium.org, mdjones
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add data usage tracking for feedback uploader, trace uploader, dom distiller This CL tags the data use from Chrome services feedback uploader, trace uploader and dom distiller BUG=527304 Committed: https://crrev.com/06be1a78361e2243545ae1cce115f5bbe9739b1d Cr-Commit-Position: refs/heads/master@{#425594}

Patch Set 1 : minor fix #

Patch Set 2 : Rebased #

Patch Set 3 : Change to Tracing and Crash Uploader #

Total comments: 1

Patch Set 4 : Change to TRACING_UPLOADER #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -1 line) Patch
M chrome/browser/android/feedback/connectivity_checker.cc View 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/tracing/crash_service_uploader.cc View 1 2 3 2 chunks +4 lines, -0 lines 0 comments Download
M components/data_use_measurement/core/data_use_user_data.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M components/data_use_measurement/core/data_use_user_data.cc View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M components/dom_distiller/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M components/dom_distiller/core/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M components/dom_distiller/core/distiller_url_fetcher.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M components/feedback/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M components/feedback/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M components/feedback/feedback_uploader_chrome.cc View 2 chunks +3 lines, -1 line 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 35 (20 generated)
Raj
zork: ptal components/feedback/* yfriedman: ptal chrome/browser/android/feedback/connectivity_checker.cc wychen: ptal components/dom_distiller/*
4 years, 2 months ago (2016-09-27 23:31:51 UTC) #7
Zachary Kuznia
lgtm
4 years, 2 months ago (2016-09-27 23:56:57 UTC) #8
wychen
dom distiller lgtm A side note: In normal use cases (where the original WebContents is ...
4 years, 2 months ago (2016-09-28 07:36:02 UTC) #11
Raj
bauerb: ptal chrome/browser/android/feedback/connectivity_checker.cc primiano: ptal chrome/browser/tracing/crash_service_uploader.cc holte: ptal histograms.xml
4 years, 2 months ago (2016-10-14 02:00:48 UTC) #13
Primiano Tucci (use gerrit)
+oysteine Code itself LG, but the name you are using here is extremely misleading. What ...
4 years, 2 months ago (2016-10-14 08:53:40 UTC) #15
Bernhard Bauer
lgtm
4 years, 2 months ago (2016-10-14 11:08:12 UTC) #16
Raj
On 2016/10/14 08:53:40, Primiano Tucci wrote: > +oysteine > Code itself LG, but the name ...
4 years, 2 months ago (2016-10-14 17:23:57 UTC) #17
Primiano Tucci (use gerrit)
On 2016/10/14 17:23:57, Raj wrote: > On 2016/10/14 08:53:40, Primiano Tucci wrote: > > +oysteine ...
4 years, 2 months ago (2016-10-14 17:26:07 UTC) #18
Steven Holte
histograms lgtm
4 years, 2 months ago (2016-10-14 22:13:37 UTC) #19
oystein (OOO til 10th of July)
https://codereview.chromium.org/2375773002/diff/60001/chrome/browser/tracing/crash_service_uploader.cc File chrome/browser/tracing/crash_service_uploader.cc (right): https://codereview.chromium.org/2375773002/diff/60001/chrome/browser/tracing/crash_service_uploader.cc#newcode316 chrome/browser/tracing/crash_service_uploader.cc:316: data_use_measurement::DataUseUserData::TRACING_CRASH_UPLOADER); Yeah as primiano mentioned, I would just call ...
4 years, 2 months ago (2016-10-14 22:15:59 UTC) #20
Raj
On 2016/10/14 22:15:59, oystein wrote: > https://codereview.chromium.org/2375773002/diff/60001/chrome/browser/tracing/crash_service_uploader.cc > File chrome/browser/tracing/crash_service_uploader.cc (right): > > https://codereview.chromium.org/2375773002/diff/60001/chrome/browser/tracing/crash_service_uploader.cc#newcode316 > ...
4 years, 2 months ago (2016-10-15 18:46:23 UTC) #21
Primiano Tucci (use gerrit)
LGTM this patch, this patch LGTM ;-) PS please update your commit message which still ...
4 years, 2 months ago (2016-10-15 22:17:36 UTC) #26
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/2375773002/80001
4 years, 2 months ago (2016-10-16 19:09:10 UTC) #31
commit-bot: I haz the power
Committed patchset #4 (id:80001)
4 years, 2 months ago (2016-10-16 20:24:30 UTC) #33
commit-bot: I haz the power
4 years, 2 months ago (2016-10-16 20:27:06 UTC) #35
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/06be1a78361e2243545ae1cce115f5bbe9739b1d
Cr-Commit-Position: refs/heads/master@{#425594}

Powered by Google App Engine
This is Rietveld 408576698