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

Issue 2915863002: Photo Picker dialog: Add UMA statistics. (Closed)

Created:
3 years, 6 months ago by Finnur
Modified:
3 years, 6 months ago
Reviewers:
Ted C, Theresa, Mark P
CC:
chromium-reviews, asvitkine+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Photo Picker dialog: Add UMA statistics. BUG=656015 Review-Url: https://codereview.chromium.org/2915863002 Cr-Commit-Position: refs/heads/master@{#477943} Committed: https://chromium.googlesource.com/chromium/src/+/83e34cf9274af314b3a3ebc21e13a68dbc1cbb76

Patch Set 1 #

Total comments: 8

Patch Set 2 : Address feedback from Theresa #

Total comments: 3

Patch Set 3 : Address feedback from Theresa #

Total comments: 4

Patch Set 4 : Address feedback from Theresa #

Total comments: 27

Patch Set 5 : Address Mark's (plus nits from T and T) #

Total comments: 1

Patch Set 6 : Address feedback from Mark #

Unified diffs Side-by-side diffs Delta from patch set Stats (+240 lines, -25 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java View 1 2 3 4 3 chunks +6 lines, -9 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/photo_picker/BitmapScalerTask.java View 2 chunks +10 lines, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java View 4 chunks +7 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java View 1 2 3 4 8 chunks +28 lines, -5 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerAdapter.java View 1 4 chunks +36 lines, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerBitmapViewHolder.java View 1 3 chunks +14 lines, -3 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java View 1 2 3 4 8 chunks +32 lines, -6 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/util/ConversionUtils.java View 1 2 3 4 1 chunk +26 lines, -0 lines 0 comments Download
M chrome/android/java_sources.gni View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M tools/metrics/histograms/enums.xml View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 1 chunk +73 lines, -0 lines 0 comments Download

Messages

Total messages: 47 (31 generated)
Finnur
PTAL.
3 years, 6 months ago (2017-05-31 17:01:51 UTC) #5
Theresa
https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java (right): https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java#newcode99 chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:99: // The timestamp for when the request is being ...
3 years, 6 months ago (2017-05-31 18:40:02 UTC) #7
Finnur
All addressed. PTAL https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java (right): https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java#newcode99 chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:99: // The timestamp for when the ...
3 years, 6 months ago (2017-06-01 15:13:55 UTC) #13
Theresa
https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java File chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java (right): https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java#newcode99 chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:99: public static final int BYTES_PER_GIGABYTE = 1024 * 1024 ...
3 years, 6 months ago (2017-06-01 15:58:04 UTC) #14
Finnur
Theresa, all addressed. PTAL. Ted, can you give OWNERS approval for moving the conversion consts ...
3 years, 6 months ago (2017-06-01 23:18:54 UTC) #22
Finnur
Friendly reminder as it is nearing EOW here. :)
3 years, 6 months ago (2017-06-02 15:17:57 UTC) #25
Theresa
lgtm % moving byte -> kb/mb/gb conversions to util methods in ConversionUtils.java https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java File chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java ...
3 years, 6 months ago (2017-06-02 15:40:22 UTC) #26
Ted C
lgtm w/ small changes https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/ConversionUtils.java File chrome/android/java/src/org/chromium/chrome/browser/ConversionUtils.java (right): https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/ConversionUtils.java#newcode5 chrome/android/java/src/org/chromium/chrome/browser/ConversionUtils.java:5: package org.chromium.chrome.browser; let's put this ...
3 years, 6 months ago (2017-06-02 16:16:40 UTC) #29
Mark P
https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java#newcode42 chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:42: // UMA histograms. We use a comment more like ...
3 years, 6 months ago (2017-06-02 17:17:58 UTC) #32
Finnur
All addressed, Mark PTAL. https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java#newcode42 chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:42: // UMA histograms. Done. https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java#newcode249 ...
3 years, 6 months ago (2017-06-06 14:23:17 UTC) #35
Finnur
Mark: Ping.
3 years, 6 months ago (2017-06-07 15:29:26 UTC) #38
Mark P
lgtm modulo a minor word substitution request below Thanks for the conscientious fixes to the ...
3 years, 6 months ago (2017-06-07 17:19:31 UTC) #39
Finnur
> Sorry I didn't get to review it yesterday; I was slammed. No problem. Thanks ...
3 years, 6 months ago (2017-06-08 10:30:16 UTC) #40
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/2915863002/160001
3 years, 6 months ago (2017-06-08 10:30:35 UTC) #43
Finnur
Ooops, forgot to publish my draft. https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java#newcode406 chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:406: "Android.PhotoPicker.DialogAction", action, ACTION_BOUNDARY); ...
3 years, 6 months ago (2017-06-08 10:39:31 UTC) #44
commit-bot: I haz the power
3 years, 6 months ago (2017-06-08 11:57:49 UTC) #47
Message was sent while issue was closed.
Committed patchset #6 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/83e34cf9274af314b3a3ebc21e13...

Powered by Google App Engine
This is Rietveld 408576698