Make PermissionRequestManager::requests_ correspond to the active prompt

chrome/browser/permissions/permission_context_base_unittest.cc

Index: chrome/browser/permissions/permission_context_base_unittest.cc
diff --git a/chrome/browser/permissions/permission_context_base_unittest.cc b/chrome/browser/permissions/permission_context_base_unittest.cc
index 4bd7650387c98c5181c9bcb487b7a8e2d16dd6fd..f177f562f801a67ee3716892a27ae920e51ba4ff 100644
--- a/chrome/browser/permissions/permission_context_base_unittest.cc
+++ b/chrome/browser/permissions/permission_context_base_unittest.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/permissions/permission_uma_util.h"
 #include "chrome/browser/permissions/permission_util.h"
+#include "chrome/browser/ui/permission_bubble/mock_permission_prompt_factory.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -247,19 +248,13 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     PermissionRequestManager* manager =
         PermissionRequestManager::FromWebContents(web_contents());
     manager->TogglePersist(persist);
-    switch (response) {
-      case CONTENT_SETTING_ALLOW:
-        manager->Accept();
-        break;
-      case CONTENT_SETTING_BLOCK:
-        manager->Deny();
-        break;
-      case CONTENT_SETTING_ASK:
-        manager->Closing();
-        break;
-      default:
-        NOTREACHED();
-    }
+    using AutoResponseType = PermissionRequestManager::AutoResponseType;
+    AutoResponseType decision = AutoResponseType::DISMISS;
+    if (response == CONTENT_SETTING_ALLOW)
+      decision = AutoResponseType::ACCEPT_ALL;
+    else if (response == CONTENT_SETTING_BLOCK)
+      decision = AutoResponseType::DENY_ALL;

[raymes, 2017/05/04 05:00:04]

perhaps add
else
  NOTREACHED();

to maintain the invariant?

[Timothy Loh, 2017/05/04 08:22:30]

The value of response is DCHECK'd above.

+    prompt_factory_->set_response_type(decision);
 #endif
   }
 
@@ -269,6 +264,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     TestPermissionContext permission_context(profile(), content_settings_type);
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
+    LoadCompleted();

[raymes, 2017/05/04 05:00:04]

Can this just go in NavigateAndCommit?

[Timothy Loh, 2017/05/04 08:22:30]

It's defined in RenderViewHostTestHarness so no, but I could wrap it up into a
single function (SetUpUrl or something?)

[raymes, 2017/05/08 03:14:26]

That sounds good. Or maybe NavigateToUrl

[Timothy Loh, 2017/05/08 04:06:51]

Done (called it SetUpUrl)

     base::HistogramTester histograms;
 
     const PermissionRequestID id(
@@ -421,6 +417,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
   void TestBlockOnSeveralDismissals_TestContent() {
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
+    LoadCompleted();
     base::HistogramTester histograms;
 
     // First, ensure that > 3 dismissals behaves correctly.
@@ -489,6 +486,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
   void TestVariationBlockOnSeveralDismissals_TestContent() {
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
+    LoadCompleted();
     base::HistogramTester histograms;
 
     // Set up the custom parameter and custom value.
@@ -596,6 +594,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     GURL url;
     ASSERT_FALSE(url.is_valid());
     NavigateAndCommit(url);
+    LoadCompleted();
 
     const PermissionRequestID id(
         web_contents()->GetRenderProcessHost()->GetID(),
@@ -619,6 +618,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     TestPermissionContext permission_context(profile(), content_settings_type);
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
+    LoadCompleted();
 
     const PermissionRequestID id(
         web_contents()->GetRenderProcessHost()->GetID(),
@@ -674,6 +674,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
         profile(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
     GURL url("http://www.google.com");
     NavigateAndCommit(url);
+    LoadCompleted();
 
     const PermissionRequestID id0(
         web_contents()->GetRenderProcessHost()->GetID(),
@@ -719,6 +720,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
       ContentSetting expected_permission_status,
       PermissionEmbargoStatus expected_embargo_reason) {
     NavigateAndCommit(url);
+    LoadCompleted();
     base::HistogramTester histograms;
     base::test::ScopedFeatureList scoped_feature_list;
     scoped_feature_list.InitAndEnableFeature(features::kPermissionsBlacklist);
@@ -761,6 +763,12 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
                                   static_cast<int>(expected_embargo_reason), 1);
   }
 
+  void LoadCompleted() {
+#if !defined(OS_ANDROID)
+    prompt_factory_->DocumentOnLoadCompletedInMainFrame();
+#endif
+  }
+
  private:
   // ChromeRenderViewHostTestHarness:
   void SetUp() override {
@@ -769,10 +777,20 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     InfoBarService::CreateForWebContents(web_contents());
 #else
     PermissionRequestManager::CreateForWebContents(web_contents());
+    PermissionRequestManager* manager =
+        PermissionRequestManager::FromWebContents(web_contents());
+    prompt_factory_.reset(new MockPermissionPromptFactory(manager));
+    manager->DisplayPendingRequests();
 #endif
   }
 
+  void TearDown() override {
+    prompt_factory_.reset();
+    ChromeRenderViewHostTestHarness::TearDown();
+  }
+
   DISALLOW_COPY_AND_ASSIGN(PermissionContextBaseTests);

[raymes, 2017/05/04 05:00:04]

This usually goes after all member variables.

[Timothy Loh, 2017/05/04 08:22:30]

Fixed

+  std::unique_ptr<MockPermissionPromptFactory> prompt_factory_;
 };
 
 // Simulates clicking Accept. The permission should be granted and