Call recordPublishResult in Download Home during permission granting (if denied) |
||||||
Issue descriptionWhen sharing a page from Download Home and the page needs to be published, we should also record the metric if the process of granting storage permission is denied by the user. We need to call OfflinePageUtils.recordPublishPageResult(SavePageResult.PERMISSION_DENIED) from the place where the permission is granted in the 'sharing page from Download Home' code path.
,
Apr 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe8878d123f6a0f19c4e7ad82c1572ac070724b1 commit fe8878d123f6a0f19c4e7ad82c1572ac070724b1 Author: Yafei Duan <romax@chromium.org> Date: Tue Apr 24 01:16:01 2018 [Offline Pages] Adding logging for permission denied in download home. We also need to record the permission denied when asking for storage permission before publishing an offline page for sharing in Download Home. Bug: 836011 Change-Id: Ief294b69a88054a7201ba6d0ecf8bc53d77b7bc7 Reviewed-on: https://chromium-review.googlesource.com/1024741 Reviewed-by: Joy Ming <jming@chromium.org> Commit-Queue: Yafei Duan <romax@chromium.org> Cr-Commit-Position: refs/heads/master@{#552939} [modify] https://crrev.com/fe8878d123f6a0f19c4e7ad82c1572ac070724b1/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
,
Apr 24 2018
+petewil@ Do you think this needs to be merged into M-67 as well? Since it's part of user metrics, I think it should go with the shipping of P2P sharing feature.
,
Apr 24 2018
adding label for merging into M-67. This is part of a metric collection needed for the P2P sharing feature shipping in M-67. Verified in ToT that the metric is collected correctly.
,
Apr 25 2018
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 30 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 1 2018
Sorry, since the CL that this patch depend on will not be merged into M67, we decided not to merge this one either. I'm removing the flags related with merging.
,
May 1 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by romax@chromium.org
, Apr 23 2018