Only record the prediction service response if the selector was considered

Bug: 1138595
Change-Id: I7b5ab3739842122e5b71f208b4e0bb237636ee3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2624608
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844171}

components/permissions/permission_request_manager.cc

@@ -796,12 +796,6 @@ void PermissionRequestManager::OnNotificationPermissionUiSelectorDone(
     }
   }
 
-  if (!prediction_grant_likelihood_.has_value()) {
-    prediction_grant_likelihood_ =
-        notification_permission_ui_selectors_[selector_index]
-            ->PredictedGrantLikelihoodForUKM();
-  }
-
   // We have already made a decision because of a higher priority selector
   // therefore this selector's decision can be discarded.
   if (current_request_ui_to_use_.has_value())
@@ -814,12 +808,20 @@ void PermissionRequestManager::OnNotificationPermissionUiSelectorDone(
   while (decision_index < selector_decisions_.size() &&
          selector_decisions_[decision_index].has_value()) {
     const UiDecision& current_decision =
-        selector_decisions_[decision_index++].value();
+        selector_decisions_[decision_index].value();
+
+    if (!prediction_grant_likelihood_.has_value()) {
+      prediction_grant_likelihood_ =
+          notification_permission_ui_selectors_[decision_index]
+              ->PredictedGrantLikelihoodForUKM();
+    }
 
     if (current_decision.quiet_ui_reason.has_value()) {
       current_request_ui_to_use_ = current_decision;
       break;
     }
+
+    ++decision_index;
   }
 
   // All decisions have been considered and none was conclusive.

components/permissions/permission_request_manager.h

@@ -175,6 +175,11 @@ class PermissionRequestManager
 
   PermissionPrompt* view_for_testing() { return view_.get(); }
 
+  base::Optional<PermissionUmaUtil::PredictionGrantLikelihood>
+  prediction_grant_likelihood_for_testing() {
+    return prediction_grant_likelihood_;
+  }
+
  private:
   friend class test::PermissionRequestManagerTestApi;
   friend class content::WebContentsUserData<PermissionRequestManager>;

components/permissions/permission_request_manager_unittest.cc

@@ -687,10 +687,12 @@ class MockNotificationPermissionUiSelector
  public:
   explicit MockNotificationPermissionUiSelector(
       base::Optional<QuietUiReason> quiet_ui_reason,
-      bool async) {
-    quiet_ui_reason_ = quiet_ui_reason;
-    async_ = async;
-  }
+      base::Optional<PermissionUmaUtil::PredictionGrantLikelihood>
+          prediction_likelihood,
+      bool async)
+      : quiet_ui_reason_(quiet_ui_reason),
+        prediction_likelihood_(prediction_likelihood),
+        async_(async) {}
 
   void SelectUiToUse(PermissionRequest* request,
                      DecisionMadeCallback callback) override {
@@ -703,16 +705,26 @@ class MockNotificationPermissionUiSelector
     }
   }
 
-  static void CreateForManager(PermissionRequestManager* manager,
+  base::Optional<PermissionUmaUtil::PredictionGrantLikelihood>
+  PredictedGrantLikelihoodForUKM() override {
+    return prediction_likelihood_;
+  }
+
+  static void CreateForManager(
+      PermissionRequestManager* manager,
       base::Optional<QuietUiReason> quiet_ui_reason,
-                               bool async) {
+      bool async,
+      base::Optional<PermissionUmaUtil::PredictionGrantLikelihood>
+          prediction_likelihood = base::nullopt) {
     manager->add_notification_permission_ui_selector_for_testing(
-        std::make_unique<MockNotificationPermissionUiSelector>(quiet_ui_reason,
-                                                               async));
+        std::make_unique<MockNotificationPermissionUiSelector>(
+            quiet_ui_reason, prediction_likelihood, async));
   }
 
  private:
   base::Optional<QuietUiReason> quiet_ui_reason_;
+  base::Optional<PermissionUmaUtil::PredictionGrantLikelihood>
+      prediction_likelihood_;
   bool async_;
 };
 
@@ -867,6 +879,61 @@ TEST_P(PermissionRequestManagerTest, MultipleUiSelectors) {
   }
 }
 
+TEST_P(PermissionRequestManagerTest, SelectorsPredictionLikelihood) {
+  using QuietUiReason = NotificationPermissionUiSelector::QuietUiReason;
+  using PredictionLikelihood = PermissionUmaUtil::PredictionGrantLikelihood;
+  const auto VeryLikely = PredictionLikelihood::
+      PermissionSuggestion_Likelihood_DiscretizedLikelihood_VERY_LIKELY;
+  const auto Neutral = PredictionLikelihood::
+      PermissionSuggestion_Likelihood_DiscretizedLikelihood_NEUTRAL;
+
+  const struct {
+    std::vector<bool> enable_quiet_uis;
+    std::vector<base::Optional<PredictionLikelihood>> prediction_likelihoods;
+    base::Optional<PredictionLikelihood> expected_prediction_likelihood;
+  } kTests[] = {
+      // Sanity check: prediction likelihood is populated correctly.
+      {{true}, {VeryLikely}, VeryLikely},
+      {{false}, {Neutral}, Neutral},
+
+      // Prediction likelihood is populated only if the selector was considered.
+      {{true, true}, {base::nullopt, VeryLikely}, base::nullopt},
+      {{false, true}, {base::nullopt, VeryLikely}, VeryLikely},
+      {{false, false}, {base::nullopt, VeryLikely}, VeryLikely},
+
+      // First considered selector is preserved.
+      {{true, true}, {Neutral, VeryLikely}, Neutral},
+      {{false, true}, {Neutral, VeryLikely}, Neutral},
+      {{false, false}, {Neutral, VeryLikely}, Neutral},
+  };
+
+  for (const auto& test : kTests) {
+    manager_->clear_notification_permission_ui_selector_for_testing();
+    for (size_t i = 0; i < test.enable_quiet_uis.size(); ++i) {
+      MockNotificationPermissionUiSelector::CreateForManager(
+          manager_,
+          test.enable_quiet_uis[i]
+              ? base::Optional<QuietUiReason>(QuietUiReason::kEnabledInPrefs)
+              : base::nullopt,
+          false /* async */, test.prediction_likelihoods[i]);
+    }
+
+    MockPermissionRequest request("foo", RequestType::kNotifications,
+                                  PermissionRequestGestureType::GESTURE);
+
+    manager_->AddRequest(web_contents()->GetMainFrame(), &request);
+    WaitForBubbleToBeShown();
+
+    EXPECT_TRUE(prompt_factory_->is_visible());
+    EXPECT_TRUE(prompt_factory_->RequestTypeSeen(request.GetRequestType()));
+    EXPECT_EQ(test.expected_prediction_likelihood,
+              manager_->prediction_grant_likelihood_for_testing());
+
+    Accept();
+    EXPECT_TRUE(request.granted());
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          PermissionRequestManagerTest,
                          ::testing::Values(false, true));