Permission Metrics: Switch the order of RecordIgnore and PermissionIgnored

chrome/browser/permissions/permission_request_impl.cc

Index: chrome/browser/permissions/permission_request_impl.cc
diff --git a/chrome/browser/permissions/permission_request_impl.cc b/chrome/browser/permissions/permission_request_impl.cc
index feb02d3af6353ba849f244936d76bc8f6602fbd6..ccddd4082cd02e8f2daa3e0b1cd250dd816a3500 100644
--- a/chrome/browser/permissions/permission_request_impl.cc
+++ b/chrome/browser/permissions/permission_request_impl.cc
@@ -33,11 +33,14 @@ PermissionRequestImpl::PermissionRequestImpl(
 PermissionRequestImpl::~PermissionRequestImpl() {
   DCHECK(is_finished_);
   if (!action_taken_) {
-    PermissionDecisionAutoBlocker(profile_).RecordIgnore(request_origin_,
-                                                         permission_type_);
-
+    // PermissionIgnored needs to be called before RecordIgnore in the blocker
+    // because it gets the number of prior ignore and dismiss values from the
+    // blocker and we don't want to include the current ignore.
     PermissionUmaUtil::PermissionIgnored(permission_type_, GetGestureType(),
                                          request_origin_, profile_);
+
+    PermissionDecisionAutoBlocker(profile_).RecordIgnore(request_origin_,
+                                                         permission_type_);

[raymes, 2016/08/23 06:01:48]

Could we instead move this into PermissionUmaUtil::PermissionIgnored?

[kcarattini, 2016/08/23 07:19:50]

Done. I like this, but I don't like that of symmetry in the recording of prior
dismisses. Maybe we could think about separating that from the test of whether
or not to block after n dismissals? I'd like to hear what dominickn@ thinks.

[raymes, 2016/08/23 08:14:58]

Were you thinking we should record the dismiss in the
PermissionUmaUtil::PermissionPromptDismissed and check the number of dismisses
separately? I like that idea too, I'd be interested in what Dom thinks too.

[kcarattini, 2016/08/23 10:00:58]

Yes, that's what I meant :).

   }
 }