Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2153133002:
Add gesture type value from desktop prompt to permission report (Closed)
|
Created:
4 years, 5 months ago by stefanocs Modified:
4 years, 4 months ago CC:
chromium-reviews, mlamouri+watch-geolocation_chromium.org, toyoshim+midi_chromium.org, Peter Beverloo, mlamouri+watch-notifications_chromium.org, feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org, Michael van Ouwerkerk, mlamouri+watch-permissions_chromium.org, harkness+watch_chromium.org, johnme+watch_chromium.org, miu+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@add-user-gesture-to-reporting-part Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd gesture type value from desktop prompt to permission report
We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable.
To record PermissionIgnored, gesture type obtained from PermissionRequestImpl.
For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations are not possible.
To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses.
For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon.
BUG=613883
Committed: https://crrev.com/d8726709c5a13788275fbbea9770db0c67f4fb7e
Cr-Commit-Position: refs/heads/master@{#408329}
Patch Set 1 #Patch Set 2 #Patch Set 3 #Patch Set 4 #Patch Set 5 #Patch Set 6 #Patch Set 7 #Patch Set 8 #Patch Set 9 #Patch Set 10 #Patch Set 11 #Patch Set 12 #Patch Set 13 #Patch Set 14 #Patch Set 15 #Patch Set 16 #Patch Set 17 #Patch Set 18 #Patch Set 19 #
Total comments: 21
Patch Set 20 : Remove push messaging add comment fix nits #
Total comments: 11
Patch Set 21 : Update comments + change permission revoked user gesture to true #Patch Set 22 : nit: #
Total comments: 6
Patch Set 23 : fix nits #Patch Set 24 : Remove push messaging + change revoked to false user gesture #Patch Set 25 : rebase #Patch Set 26 : rebase #Patch Set 27 : merge rebase #Patch Set 28 : rebase again #Patch Set 29 : merge #
Total comments: 4
Patch Set 30 : Fix nits #Patch Set 31 : Rebase #Patch Set 32 : rebase #
Total comments: 5
Patch Set 33 : Send user_gesture to permission reporter #Patch Set 34 : Rebased with the new cl #Patch Set 35 : Use permission request gesture type #Patch Set 36 : Change back mistake #
Total comments: 2
Patch Set 37 : Add comment #Patch Set 38 : add include #Patch Set 39 : nit #Dependent Patchsets: Messages
Total messages: 130 (107 generated)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
The CQ bit was checked by stefanocs@google.com 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...
Description was changed from ========== Add user gesture value from permission layer to permission report BUG=613883 ========== to ========== Add user gesture value from permission layer to permission report User gesture will be added to permission report for prompt source ui. The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl and PermissionQueueController (and PermissionInfobarDelegate + its subclasses) To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false. BUG=613883 ==========
The CQ bit was checked by stefanocs@google.com 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...
Description was changed from ========== Add user gesture value from permission layer to permission report User gesture will be added to permission report for prompt source ui. The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl and PermissionQueueController (and PermissionInfobarDelegate + its subclasses) To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false. BUG=613883 ========== to ========== Add user gesture value from permission layer to permission report User gesture is added to permission report for only prompt source ui. The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl and PermissionQueueController (and PermissionInfobarDelegate + its subclasses) To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false. BUG=613883 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
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 stefanocs@google.com 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...
Description was changed from ========== Add user gesture value from permission layer to permission report User gesture is added to permission report for only prompt source ui. The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl and PermissionQueueController (and PermissionInfobarDelegate + its subclasses) To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false. BUG=613883 ========== to ========== Add user gesture value from desktop prompt to permission report The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl. To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false. BUG=613883 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
stefanocs@google.com changed reviewers: + kcarattini@chromium.org, raymes@chromium.org
https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:82: callback.Run(permission_type, false /* user_gesture */, Do we need to add a comment that this needs to be fixed at some point? https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_bubble_request_impl.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_bubble_request_impl.cc:35: if (!action_taken_) nit: add {} https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_context_base.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:194: // TODO(stefanocs): Get user gesture from infobar. Hmm, what does this mean? https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:220: if (content_setting == CONTENT_SETTING_ALLOW) nit: add {} https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:223: else nit: add {} https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_infobar_delegate.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_infobar_delegate.cc:14: // TODO(stefanocs): Pass the actual user_gesture value to PermissionUmaUtil. |user_gesture| https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_infobar_delegate.cc:15: if (!action_taken_) nit: add {} https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_queue_controller.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_queue_controller.cc:212: if (allowed) nit: add {} https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_queue_controller.cc:216: else nit: add {} https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/push_me... File chrome/browser/push_messaging/push_messaging_permission_context.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/push_me... chrome/browser/push_messaging/push_messaging_permission_context.cc:108: bool user_gesture, I would say don't bother passing this for now since those metrics are wrong anyway and I'm making some changes to the code. Perhaps just pass false and add a TODO below to pass the correct value.
https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:82: callback.Run(permission_type, false /* user_gesture */, On 2016/07/18 00:58:43, raymes wrote: > Do we need to add a comment that this needs to be fixed at some point? Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_bubble_request_impl.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_bubble_request_impl.cc:35: if (!action_taken_) On 2016/07/18 00:58:43, raymes wrote: > nit: add {} Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_context_base.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:194: // TODO(stefanocs): Get user gesture from infobar. On 2016/07/18 00:58:43, raymes wrote: > Hmm, what does this mean? I edited the comment, does it make sense for you or is it even needed? https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:220: if (content_setting == CONTENT_SETTING_ALLOW) On 2016/07/18 00:58:43, raymes wrote: > nit: add {} Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:223: else On 2016/07/18 00:58:43, raymes wrote: > nit: add {} Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_infobar_delegate.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_infobar_delegate.cc:14: // TODO(stefanocs): Pass the actual user_gesture value to PermissionUmaUtil. On 2016/07/18 00:58:43, raymes wrote: > |user_gesture| Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_infobar_delegate.cc:15: if (!action_taken_) On 2016/07/18 00:58:43, raymes wrote: > nit: add {} Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_queue_controller.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_queue_controller.cc:212: if (allowed) On 2016/07/18 00:58:43, raymes wrote: > nit: add {} Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_queue_controller.cc:216: else On 2016/07/18 00:58:43, raymes wrote: > nit: add {} Done. https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/push_me... File chrome/browser/push_messaging/push_messaging_permission_context.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/push_me... chrome/browser/push_messaging/push_messaging_permission_context.cc:108: bool user_gesture, On 2016/07/18 00:58:43, raymes wrote: > I would say don't bother passing this for now since those metrics are wrong > anyway and I'm making some changes to the code. Perhaps just pass false and add > a TODO below to pass the correct value. Done.
https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:82: // The actual |user_gesture| will be passed once this file has been This is a TODO https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); I wonder if this should be true for revocations? It's impossible to revoke with _some_ action by the user. Not that it should make a difference to our metrics. https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... File chrome/browser/push_messaging/push_messaging_permission_context.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... chrome/browser/push_messaging/push_messaging_permission_context.cc:123: PermissionUmaUtil::PermissionDenied(permission_type(), Can you specify what you mean by "push messaging metrics has been fixed"? Is there a specific bug you are referring to? When do you estimate it will be fixed?
https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:82: // The actual |user_gesture| will be passed once this file has been On 2016/07/18 07:04:33, kcarattini wrote: > This is a TODO Done. https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); On 2016/07/18 07:04:33, kcarattini wrote: > I wonder if this should be true for revocations? It's impossible to revoke with > _some_ action by the user. Not that it should make a difference to our metrics. Done. https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... File chrome/browser/push_messaging/push_messaging_permission_context.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... chrome/browser/push_messaging/push_messaging_permission_context.cc:123: PermissionUmaUtil::PermissionDenied(permission_type(), On 2016/07/18 07:04:33, kcarattini wrote: > Can you specify what you mean by "push messaging metrics has been fixed"? Is > there a specific bug you are referring to? When do you estimate it will be > fixed? Done. I think Raymes is working on this one. Raymes, is there a specific bug tracker for this one?
lgtm https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... File chrome/browser/permissions/permission_context_base.cc (right): https://codereview.chromium.org/2153133002/diff/360001/chrome/browser/permiss... chrome/browser/permissions/permission_context_base.cc:194: // TODO(stefanocs): Get user gesture from infobar. Looks good, thanks! https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); I think it is good to note that they are very different kinds of user gestures. The one we're recording for prompts is to do with what triggered the prompt (a user gesture from blink). The user may also take an action on the prompt, such as clicking allow, but we don't record anything about that "user gesture" in the report. I'd lean toward keeping it as false but I don't feel strongly :) https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... File chrome/browser/push_messaging/push_messaging_permission_context.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... chrome/browser/push_messaging/push_messaging_permission_context.cc:123: PermissionUmaUtil::PermissionDenied(permission_type(), On 2016/07/18 07:19:33, stefanocs wrote: > On 2016/07/18 07:04:33, kcarattini wrote: > > Can you specify what you mean by "push messaging metrics has been fixed"? Is > > there a specific bug you are referring to? When do you estimate it will be > > fixed? > Done. I think Raymes is working on this one. Raymes, is there a specific bug > tracker for this one? Once this CL is landed it will happen automatically: https://codereview.chromium.org/2149883002/ https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:75: base::Callback<void(content::PermissionType, bool, const GURL&, Profile*)>; nit: bool /* user_gesture */ https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... File chrome/browser/permissions/permission_bubble_request_impl.h (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... chrome/browser/permissions/permission_bubble_request_impl.h:55: bool user_gesture_; nit: perhaps add a comment that this is specifying whether there was a user gesture which triggered this request. https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... File chrome/browser/permissions/permission_queue_controller.cc (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... chrome/browser/permissions/permission_queue_controller.cc:212: if (allowed) nit add {}
https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... File chrome/browser/push_messaging/push_messaging_permission_context.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/push_me... chrome/browser/push_messaging/push_messaging_permission_context.cc:123: PermissionUmaUtil::PermissionDenied(permission_type(), On 2016/07/18 07:31:59, raymes wrote: > On 2016/07/18 07:19:33, stefanocs wrote: > > On 2016/07/18 07:04:33, kcarattini wrote: > > > Can you specify what you mean by "push messaging metrics has been fixed"? Is > > > there a specific bug you are referring to? When do you estimate it will be > > > fixed? > > Done. I think Raymes is working on this one. Raymes, is there a specific bug > > tracker for this one? > > Once this CL is landed it will happen automatically: > https://codereview.chromium.org/2149883002/ In that case, I will probably just remove the comment. https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:75: base::Callback<void(content::PermissionType, bool, const GURL&, Profile*)>; On 2016/07/18 07:31:59, raymes wrote: > nit: bool /* user_gesture */ Done. https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... File chrome/browser/permissions/permission_queue_controller.cc (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... chrome/browser/permissions/permission_queue_controller.cc:212: if (allowed) On 2016/07/18 07:31:59, raymes wrote: > nit add {} Done.
lgtm https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); On 2016/07/18 07:31:59, raymes wrote: > I think it is good to note that they are very different kinds of user gestures. > The one we're recording for prompts is to do with what triggered the prompt (a > user gesture from blink). The user may also take an action on the prompt, such > as clicking allow, but we don't record anything about that "user gesture" in the > report. I'd lean toward keeping it as false but I don't feel strongly :) I'm fine with it being false, because it is the same value for all revocations. Revocations are entirely different beasts from prompts anyway. It looks like we aren't going to be able to tell the difference between "no user gesture" and "unspecified" anyway.
stefanocs@google.com changed reviewers: + sergeyu@chromium.org, timvolodine@chromium.org
stefanocs@google.com changed reviewers: + sergeyu@chromium.org, timvolodine@chromium.org
Thanks! sergeyu@chromium.org: Please review changes in chrome/browser/media/media_stream_devices_controller.cc timvolodine@chromium.org: Please review changes in chrome/browser/geolocation/geolocation_permission_context_android.cc https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); On 2016/07/18 08:03:20, kcarattini wrote: > On 2016/07/18 07:31:59, raymes wrote: > > I think it is good to note that they are very different kinds of user > gestures. > > The one we're recording for prompts is to do with what triggered the prompt (a > > user gesture from blink). The user may also take an action on the prompt, such > > as clicking allow, but we don't record anything about that "user gesture" in > the > > report. I'd lean toward keeping it as false but I don't feel strongly :) > > I'm fine with it being false, because it is the same value for all revocations. > Revocations are entirely different beasts from prompts anyway. > > It looks like we aren't going to be able to tell the difference between "no user > gesture" and "unspecified" anyway. Done. Changed back to false. https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... File chrome/browser/permissions/permission_bubble_request_impl.h (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... chrome/browser/permissions/permission_bubble_request_impl.h:55: bool user_gesture_; On 2016/07/18 07:31:59, raymes wrote: > nit: perhaps add a comment that this is specifying whether there was a user > gesture which triggered this request. Done.
Thanks! sergeyu@chromium.org: Please review changes in chrome/browser/media/media_stream_devices_controller.cc timvolodine@chromium.org: Please review changes in chrome/browser/geolocation/geolocation_permission_context_android.cc https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); On 2016/07/18 08:03:20, kcarattini wrote: > On 2016/07/18 07:31:59, raymes wrote: > > I think it is good to note that they are very different kinds of user > gestures. > > The one we're recording for prompts is to do with what triggered the prompt (a > > user gesture from blink). The user may also take an action on the prompt, such > > as clicking allow, but we don't record anything about that "user gesture" in > the > > report. I'd lean toward keeping it as false but I don't feel strongly :) > > I'm fine with it being false, because it is the same value for all revocations. > Revocations are entirely different beasts from prompts anyway. > > It looks like we aren't going to be able to tell the difference between "no user > gesture" and "unspecified" anyway. Done. Changed back to false. https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... File chrome/browser/permissions/permission_bubble_request_impl.h (right): https://codereview.chromium.org/2153133002/diff/420001/chrome/browser/permiss... chrome/browser/permissions/permission_bubble_request_impl.h:55: bool user_gesture_; On 2016/07/18 07:31:59, raymes wrote: > nit: perhaps add a comment that this is specifying whether there was a user > gesture which triggered this request. Done.
The CQ bit was checked by stefanocs@google.com 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...
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 stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:75: bool /* user_gesture */, nit: don't need to have the name commented (and can add names for other args as well) https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... File chrome/browser/media/midi_permission_infobar_delegate_android.cc (right): https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... chrome/browser/media/midi_permission_infobar_delegate_android.cc:21: profile, callback)))); nit: don't need this formatting change, and I think clang-format would prefer the previous version.
Thanks! ping timvolodine https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... File chrome/browser/media/media_stream_devices_controller.cc (right): https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... chrome/browser/media/media_stream_devices_controller.cc:75: bool /* user_gesture */, On 2016/07/21 19:32:47, Sergey Ulanov wrote: > nit: don't need to have the name commented (and can add names for other args as > well) Done. https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... File chrome/browser/media/midi_permission_infobar_delegate_android.cc (right): https://codereview.chromium.org/2153133002/diff/560001/chrome/browser/media/m... chrome/browser/media/midi_permission_infobar_delegate_android.cc:21: profile, callback)))); On 2016/07/21 19:32:47, Sergey Ulanov wrote: > nit: don't need this formatting change, and I think clang-format would prefer > the previous version. Done. Sorry I changed this and then decided to move the change to the other cl, forgot to do cl format after that.
The CQ bit was checked by stefanocs@google.com 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...
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 stefanocs@google.com 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...
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
could you pls add in the description why user_gesture in media_stream_devices_controller is set to false https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); is this because we don't have user gestures for revoke? https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:344: bool user_gesture, so this is passed but not used? am I missing something?
Description was changed from ========== Add user gesture value from desktop prompt to permission report The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl. To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false. BUG=613883 ========== to ========== Add user gesture value from desktop prompt to permission report The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl. To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false since it will be refactored into PermissionContext soon. BUG=613883 ==========
Thanks. Could you lgtm this cl if everything else seem ok? https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:240: false /* user_gesture */, revoked_origin, profile); On 2016/07/25 13:58:58, timvolodine wrote: > is this because we don't have user gestures for revoke? Yes, revocations can only be triggered when user changed the setting, not when prompted. This is a different kind of user gesture that we don't need to record. You can see the discussion here https://codereview.chromium.org/2153133002/diff/380001/chrome/browser/permiss... https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:344: bool user_gesture, On 2016/07/25 13:58:58, timvolodine wrote: > so this is passed but not used? am I missing something? Oh right, I missed this one, this should be passed to PermissionReporter below. I will change this tomorrow morning.
https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/620001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:344: bool user_gesture, On 2016/07/25 13:58:58, timvolodine wrote: > so this is passed but not used? am I missing something? Done.
Hi, I have updated the cl to pass in a PermissionRequestGestureType instead of a boolean. Could you re-review the cl? Thanks.
lgtm
ok thanks, chrome/browser/geolocation/ -- lgtm However, please update the description I think it's a bit hard to read. Please explain in what cases the user gesture is passed and where it is treated as 'unknown'. Does PermissionBubbleRequestImpl even exist? https://codereview.chromium.org/2153133002/diff/700001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/700001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:267: PermissionRequestGestureType::UNKNOWN, maybe add a comment explaining why this has to be 'unknown'..
Description was changed from ========== Add user gesture value from desktop prompt to permission report The value of user gesture can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, user_gesture is passed to PermissionBubbleRequestImpl. To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false since it will be refactored into PermissionContext soon. BUG=613883 ========== to ========== Add user gesture value from desktop prompt to permission report The value of gesture type can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, gesture type is passed to PermissionBubbleRequestImpl. To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false since it will be refactored into PermissionContext soon. BUG=613883 ==========
lgtm
Description was changed from ========== Add user gesture value from desktop prompt to permission report The value of gesture type can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, gesture type is passed to PermissionBubbleRequestImpl. To record other permission actions, user_gesture can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, user_gesture is set to false since it will be refactored into PermissionContext soon. BUG=613883 ========== to ========== Add gesture type value from desktop prompt to permission report The value of gesture type can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations is not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. We also use BUG=613883 ==========
Description was changed from ========== Add gesture type value from desktop prompt to permission report The value of gesture type can be obtained from PermissionContextBase::DecidePermission. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations is not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. We also use BUG=613883 ========== to ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations is not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 ==========
Description was changed from ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations is not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 ========== to ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations are not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 ==========
Thanks! @timvolodine: Sorry the desc was outdated, I have updated the desc. https://codereview.chromium.org/2153133002/diff/700001/chrome/browser/permiss... File chrome/browser/permissions/permission_uma_util.cc (right): https://codereview.chromium.org/2153133002/diff/700001/chrome/browser/permiss... chrome/browser/permissions/permission_uma_util.cc:267: PermissionRequestGestureType::UNKNOWN, On 2016/07/26 14:47:40, timvolodine wrote: > maybe add a comment explaining why this has to be 'unknown'.. Done.
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by stefanocs@google.com 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...
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 stefanocs@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org, kcarattini@chromium.org, raymes@chromium.org, timvolodine@chromium.org Link to the patchset: https://codereview.chromium.org/2153133002/#ps760001 (title: "nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations are not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 ========== to ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations are not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 ==========
Message was sent while issue was closed.
Committed patchset #39 (id:760001)
Message was sent while issue was closed.
Description was changed from ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations are not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 ========== to ========== Add gesture type value from desktop prompt to permission report We are using the value of user_gesture obtained from PermissionContextBase::DecidePermission to obtain the PermissionRequestGestureType value. If user_gesture is true, we record PermissionRequestGestureType::GESTURE, otherwise PermissionRequestGestureType::NO_GESTURE. PermissionRequestGestureType::UNKNOWN is used when it is not applicable. To record PermissionIgnored, gesture type obtained from PermissionRequestImpl. For PermissionRevoked, an UNKNOWN gesture type will be passed in since gesture type only applicable in the prompt UIs where revocations are not possible. To record other permission actions, gesture type can be obtained from PermissionContextBase and its subclasses. For call from media_stream_devices_controller, gesture type is set to UNKNOWN since it will be refactored into PermissionContext soon. BUG=613883 Committed: https://crrev.com/d8726709c5a13788275fbbea9770db0c67f4fb7e Cr-Commit-Position: refs/heads/master@{#408329} ==========
Message was sent while issue was closed.
Patchset 39 (id:??) landed as https://crrev.com/d8726709c5a13788275fbbea9770db0c67f4fb7e Cr-Commit-Position: refs/heads/master@{#408329} |
