|
|
DescriptionPhoto 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 #Messages
Total messages: 47 (31 generated)
Patchset #1 (id:1) has been deleted
Description was changed from ========== Photo Picker dialog: Add UMA statistics. BUG=656015 ========== to ========== Photo Picker dialog: Add UMA statistics. BUG=656015 ==========
finnur@chromium.org changed reviewers: + twellington@chromium.org
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
PTAL.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... 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... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:99: // The timestamp for when the request is being sent over for decoding. nit: ...when the request was sent.. https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:203: } It would also be interesting to record the size of the bitmap so that we can get an idea of memory usage per bitmap and the number of images decoded (maybe in the initial request after we have the file list). https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:42: private static final String HISTOGRAM_NAME = "Android.PhotoPicker.DialogAction"; optional nit: The downside to using a static final String is that our presubmit scripts can't check for missing histograms. Typically we just duplicate the string in-line in the calls to record histograms so that the presubmit checks work. https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:51: private static final int NUM_ACTIONS = 4; nit: ACTIONS_BOUNDARY is more consistent with how we usually specify histogram enum boundaries.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Patchset #2 (id:40001) has been deleted
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
All addressed. PTAL https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... 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... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:99: // The timestamp for when the request is being sent over for decoding. On 2017/05/31 18:40:02, Theresa wrote: > nit: ...when the request was sent.. Done. https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:203: } I've added metrics for CacheHits for the high-res photos as well as a counter for the decode requests. I think it is better to keep track of that over the lifetime of the dialog, instead of at some point in the middle, because we want one number to be reported for each stat. Specifically I'm trying to avoid this scenario: 1. Open dialog (UMA reports 16 decode requests, 0 cache hits). 2. Scroll down (UMA reports 16 decode requests, 0 cache hits). 3. Scroll up (UMA reports 0 decode requests, 16 cache hits). 4. Close dialog Probably better to just send one stat at step 4 (32 decode requests, 16 cache hits). https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:42: private static final String HISTOGRAM_NAME = "Android.PhotoPicker.DialogAction"; On 2017/05/31 18:40:02, Theresa wrote: > optional nit: The downside to using a static final String is that our presubmit > scripts can't check for missing histograms. Typically we just duplicate the > string in-line in the calls to record histograms so that the presubmit checks > work. Done. https://codereview.chromium.org/2915863002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:51: private static final int NUM_ACTIONS = 4; On 2017/05/31 18:40:02, Theresa wrote: > nit: ACTIONS_BOUNDARY is more consistent with how we usually specify histogram > enum boundaries. Done.
https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java (right): https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:99: public static final int BYTES_PER_GIGABYTE = 1024 * 1024 * 1024; These should move to a place that makes sense for downloads and photo_picker to share. Perhaps a new ConversionUtils.java? https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java (right): https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java:207: "Android.PhotoPicker.ImageByteCount", sizeInKB, 1, 500000, 50); 500MB seems rather high to me; can the max be lower e.g. 100MB? https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:402: * Record UMA statistics (what action was taken in the dialog and other performance stats. nit: the ( parenthesis is missing a closing parenthesis
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
finnur@chromium.org changed reviewers: + tedchoc@chromium.org
finnur@chromium.org changed reviewers: + mpearson@chromium.org
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Patchset #3 (id:80001) has been deleted
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Theresa, all addressed. PTAL. Ted, can you give OWNERS approval for moving the conversion consts into its own shared file (the two *Utils.java). Mark, can you give OWNERS approval for the *.xml files. The last of the three of you to give a green light (if I am so fortunate), feel free to turn off the lights, lock the doors and check the CQ box on your way out. ;) Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Friendly reminder as it is nearing EOW here. :)
lgtm % moving byte -> kb/mb/gb conversions to util methods in ConversionUtils.java https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java (right): https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:771: bytesInCorrectUnits = bytes / (float) ConversionUtils.BYTES_PER_KILOBYTE; Let's have ConversionUtils expose methods for #bytesToKilobytes(), #bytesToMegabytes(), and #bytesToGigabytes. https://codereview.chromium.org/2915863002/diff/100001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2915863002/diff/100001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:911: + <summary>Measures the bytecount of a decoded image.</summary> nit: s/bytecount/byte count
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm w/ small changes https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/ConversionUtils.java (right): https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/ConversionUtils.java:5: package org.chromium.chrome.browser; let's put this in the chrome.browser.util package Ideally, the top level browser package gets smaller over time and not larger https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java (right): https://codereview.chromium.org/2915863002/diff/100001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java:771: bytesInCorrectUnits = bytes / (float) ConversionUtils.BYTES_PER_KILOBYTE; On 2017/06/02 15:40:22, Theresa wrote: > Let's have ConversionUtils expose methods for #bytesToKilobytes(), > #bytesToMegabytes(), and #bytesToGigabytes. Agreed
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:42: // UMA histograms. We use a comment more like the best practices one that make it clearer you cannot change the values of items or delete them. https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histo... (The current comment mainly emphasizes "keep in sync", which is too weak an assertion.) https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:249: mListener.onPickerUserAction(PhotoPickerListener.Action.CANCEL, null); nit: if this triggers a cancel listener, does that mean the cancel action gets recorded again? (I see a cancel listener on this class also records a CANCEL action.) (I'm always nervous about the same thing being logged in multiple places, per bullet 5 on this slide: https://docs.google.com/presentation/d/10iBwg3s8iiw5uMTz9ZR1lRLCdSB9OKyDsuUok... ) https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:406: "Android.PhotoPicker.DialogAction", action, ACTION_BOUNDARY); optional nit: Are you interested in what actions happens in what orders? If so, you may want to log user actions too. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:944: + Measures the amount of time the BitmapScaler spends scaling per bitmap. nit: per -> a nit: spends -> spent https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:952: + Measures how often an image is served from the high-res cache while the see similar comments to DecodeRequests, as my same concerns apply here. (Sorry I reviewed these histograms out of order.) https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:961: + Measures how often an image is sent for decoding while the dialog is open. nit: "how often" sounds like a rate. You're simply measuring the number of something. Is a better / more accurate description // The number of images sent for decoding upon opening the photo picker dialog. Yet, that's not quite right because when this is recorded sounds odd. It looks like it's recorded on, say, cancel events too, which seems strange to me. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:970: + Records which action the user takes in the PhotoPickerDialog. Do you have a normalizing action or histogram that show how often is photo picker dialog is opened? https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:977: + <summary>Measures the byte count of a decoded image.</summary> ditto my comments on ImageDecodeTime https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:984: + Measures the amount of time it takes the decoder to decode one image. Only recorded on success or always recorded? Also, what's with the "!= -1" in the code? It's clearly not decoded under the same conditions as RequestProcessTime, yet the descriptions need as if they're emitted under the same conditions. If they're under different conditions, please be clear: "Only recorded if ..." https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:993: + end (RPC and actual decoding of the bits by utility process). Only recorded on success or always recorded? https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:1002: + cache. optional nit: explain roughly under what conditions this is emitted, e.g., every time this bitmap is viewed? Only the first time?
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
All addressed, Mark PTAL. https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... 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/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:249: mListener.onPickerUserAction(PhotoPickerListener.Action.CANCEL, null); This is listening for the user clicking the X to close the dialog. The other handler is for listening for the user pressing Back. I tried both and there's no overlap (no over-counting). https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:406: "Android.PhotoPicker.DialogAction", action, ACTION_BOUNDARY); Thanks for the tip, but the order is not noteworthy in this case. All of these are final actions, as far as I'm concerned, and mark an end in my interest of what happens (as far as metrics goes). https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:944: + Measures the amount of time the BitmapScaler spends scaling per bitmap. On 2017/06/02 17:17:58, Mark P wrote: > nit: per -> a > nit: spends -> spent Done. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:952: + Measures how often an image is served from the high-res cache while the No problem. Done (see answer below). https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:961: + Measures how often an image is sent for decoding while the dialog is open. Yeah, it's not a rate and it's not just counted while the dialog is being opened. It is how many images are decoded during the lifetime of the dialog. Example: If the dialog fits 16 images and I close it right after it populates, then 16 is the metric recorded. If I don't close it right away, but scroll down so that 8 more images are shown, then 24 is the metric recorded. If I scroll back up again, 24 is still the number recorded, because the previous images are cached and not sent for decoding again. This explains why it is recorded on Cancel (or Browse, or whatever the final action is that dismisses the dialog). I've refined the comment. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:970: + Records which action the user takes in the PhotoPickerDialog. Not sure I need it. The main purpose of this metric is to see if a large portion of users are opting for the escape hatch ('Browse' to pick a photo from the Android Stock Picker) instead of performing an action in our dialog. So, I'm purely comparing against other actions taken, and as I understand it, this metric provides that. In short, I just need to be able to see: X% of time 'Browse' was picked. Also, I believe the current list of actions is an exhaustive list of outcomes in the dialog, so I'm guessing that *if* I need the open count at some point in the future, then I can just add up all the counters in this metric (or add a separate metric for that). https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:977: + <summary>Measures the byte count of a decoded image.</summary> On 2017/06/02 17:17:58, Mark P wrote: > ditto my comments on ImageDecodeTime Done (see answer below). https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:984: + Measures the amount of time it takes the decoder to decode one image. RequestProcessTime is for measuring the length of time it takes to satisfy a single decode request, including rpc delays and all. As such, it was not intended to only be recorded on success, but I guess it could be argued either way and I could be convinced to record it only on success. Until then, I've revised the comment. ImageDecodeTime and ImageByteCount only make sense on successful decode, so I've revised those comment also. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:993: + end (RPC and actual decoding of the bits by utility process). On 2017/06/02 17:17:58, Mark P wrote: > Only recorded on success or always recorded? Done. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:1002: + cache. Revised.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Mark: Ping.
lgtm modulo a minor word substitution request below Thanks for the conscientious fixes to the descriptions. Sorry I didn't get to review it yesterday; I was slammed. --mark https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:249: mListener.onPickerUserAction(PhotoPickerListener.Action.CANCEL, null); On 2017/06/06 14:23:16, Finnur wrote: > This is listening for the user clicking the X to close the dialog. The other > handler is for listening for the user pressing Back. I tried both and there's no > overlap (no over-counting). Acknowledged, thanks for checking. https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:406: "Android.PhotoPicker.DialogAction", action, ACTION_BOUNDARY); On 2017/06/06 14:23:16, Finnur wrote: > Thanks for the tip, but the order is not noteworthy in this case. All of these > are final actions, as far as I'm concerned, and mark an end in my interest of > what happens (as far as metrics goes). Acknowledged, though I was more curious whether you wanted to know what people did *before* they closed/canceled the dialog. https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:961: + Measures how often an image is sent for decoding while the dialog is open. On 2017/06/06 14:23:16, Finnur wrote: > Yeah, it's not a rate and it's not just counted while the dialog is being > opened. It is how many images are decoded during the lifetime of the dialog. > Example: > > If the dialog fits 16 images and I close it right after it populates, then 16 is > the metric recorded. > If I don't close it right away, but scroll down so that 8 more images are shown, > then 24 is the metric recorded. > If I scroll back up again, 24 is still the number recorded, because the previous > images are cached and not sent for decoding again. > > This explains why it is recorded on Cancel (or Browse, or whatever the final > action is that dismisses the dialog). > > I've refined the comment. The revised comment reads great now! thanks! https://codereview.chromium.org/2915863002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:970: + Records which action the user takes in the PhotoPickerDialog. On 2017/06/06 14:23:16, Finnur wrote: > Not sure I need it. The main purpose of this metric is to see if a large portion > of users are opting for the escape hatch ('Browse' to pick a photo from the > Android Stock Picker) instead of performing an action in our dialog. So, I'm > purely comparing against other actions taken, and as I understand it, this > metric provides that. In short, I just need to be able to see: X% of time > 'Browse' was picked. Also, I believe the current list of actions is an > exhaustive list of outcomes in the dialog, so I'm guessing that *if* I need the > open count at some point in the future, then I can just add up all the counters > in this metric (or add a separate metric for that). Acknowledged. Good to know that the current list of actions is (or is effectively) exhaustive so you don't need a normalizing value. https://codereview.chromium.org/2915863002/diff/140001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2915863002/diff/140001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:944: + Measures the amount of time the BitmapScaler spent scaling per bitmap. nit: per -> a
> Sorry I didn't get to review it yesterday; I was slammed. No problem. Thanks for the review. All addressed, so checking in.
The CQ bit was checked by finnur@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from twellington@chromium.org, tedchoc@chromium.org, mpearson@chromium.org Link to the patchset: https://codereview.chromium.org/2915863002/#ps160001 (title: "Address feedback from Mark")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Ooops, forgot to publish my draft. https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2915863002/diff/120001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:406: "Android.PhotoPicker.DialogAction", action, ACTION_BOUNDARY); There's little left to track; it is a very simple dialog. The only remaining action (that doesn't result in ending the dialog) that is not tracked at the moment is clicking on photos to select them. I'm not sure it is interesting to have statistics on how and/or what they select, but I guess it can always be added later.
CQ is committing da patch. Bot data: {"patchset_id": 160001, "attempt_start_ts": 1496917823687170, "parent_rev": "326cef509333990933a2ab90ead4f12dc33d2098", "commit_rev": "83e34cf9274af314b3a3ebc21e13a68dbc1cbb76"}
Message was sent while issue was closed.
Description was changed from ========== Photo Picker dialog: Add UMA statistics. BUG=656015 ========== to ========== 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/+/83e34cf9274af314b3a3ebc21e13... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:160001) as https://chromium.googlesource.com/chromium/src/+/83e34cf9274af314b3a3ebc21e13... |