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

Issue 2507293006: Report redirect UMA in offline interceptor (Closed)

Created:
4 years, 1 month ago by jianli
Modified:
4 years, 1 month ago
Reviewers:
Mark P, fgorski
CC:
chromium-reviews, romax+watch_chromium.org, fgorski+watch_chromium.org, dewittj+watch_chromium.org, petewil+watch_chromium.org, chili+watch_chromium.org, asvitkine+watch_chromium.org, dimich+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Report redirect UMA in offline interceptor BUG=666963 Committed: https://crrev.com/48ea10d8835e13336cf03c243b938bcb133dba49 Cr-Commit-Position: refs/heads/master@{#434029}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Address feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+77 lines, -18 lines) Patch
M chrome/browser/android/offline_pages/offline_page_request_job.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/android/offline_pages/offline_page_request_job.cc View 1 3 chunks +31 lines, -0 lines 0 comments Download
M chrome/browser/android/offline_pages/offline_page_request_job_unittest.cc View 1 16 chunks +38 lines, -18 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 23 (13 generated)
jianli
4 years, 1 month ago (2016-11-18 23:44:38 UTC) #2
fgorski
lgtm with comments (mostly naming and docs) https://codereview.chromium.org/2507293006/diff/1/chrome/browser/android/offline_pages/offline_page_request_job.cc File chrome/browser/android/offline_pages/offline_page_request_job.cc (right): https://codereview.chromium.org/2507293006/diff/1/chrome/browser/android/offline_pages/offline_page_request_job.cc#newcode62 chrome/browser/android/offline_pages/offline_page_request_job.cc:62: REDIRECTED, add ...
4 years, 1 month ago (2016-11-18 23:52:22 UTC) #5
jianli
https://codereview.chromium.org/2507293006/diff/1/chrome/browser/android/offline_pages/offline_page_request_job.cc File chrome/browser/android/offline_pages/offline_page_request_job.cc (right): https://codereview.chromium.org/2507293006/diff/1/chrome/browser/android/offline_pages/offline_page_request_job.cc#newcode62 chrome/browser/android/offline_pages/offline_page_request_job.cc:62: REDIRECTED, On 2016/11/18 23:52:22, fgorski wrote: > add a ...
4 years, 1 month ago (2016-11-19 00:54:05 UTC) #7
jianli
holte for histograms
4 years, 1 month ago (2016-11-19 00:54:21 UTC) #9
jianli
4 years, 1 month ago (2016-11-22 22:00:16 UTC) #14
jianli
mpearson for histograms
4 years, 1 month ago (2016-11-22 22:00:42 UTC) #15
Mark P
histograms.xml lgtm
4 years, 1 month ago (2016-11-22 22:35:25 UTC) #16
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/2507293006/20001
4 years, 1 month ago (2016-11-22 22:39:56 UTC) #19
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-11-22 23:56:46 UTC) #21
commit-bot: I haz the power
4 years, 1 month ago (2016-11-23 00:00:42 UTC) #23
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/48ea10d8835e13336cf03c243b938bcb133dba49
Cr-Commit-Position: refs/heads/master@{#434029}

Powered by Google App Engine
This is Rietveld 408576698