Cleanup a bit WebUI tests in render_frame_host_manager_unittest.cc

The WebUI tests in render_frame_host_manager_unittest.cc have some
code that is no longer necessary or can be streamlined a bit. There is
also a test that no longer matches reality, so I'm removing it as well
as some accidental copy/paste duplicate expectations.

Bug: 713313, 1002276
Change-Id: I29a0b3c0dea69bbef3eaff5df1dce08127333a00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849439
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705106}

content/browser/frame_host/render_frame_host_manager_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/hash/hash.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
@@ -128,34 +129,16 @@ bool FindMessageForRoutingId(const IPC::TestSink& sink,
 class RenderFrameHostManagerTestWebUIControllerFactory
     : public WebUIControllerFactory {
  public:
-  RenderFrameHostManagerTestWebUIControllerFactory()
-      : should_create_webui_(false), type_(1) {
-    CHECK_NE(reinterpret_cast<WebUI::TypeID>(type_), WebUI::kNoWebUI);
-  }
+  RenderFrameHostManagerTestWebUIControllerFactory() {}
   ~RenderFrameHostManagerTestWebUIControllerFactory() override {}
 
-  void set_should_create_webui(bool should_create_webui) {
-    should_create_webui_ = should_create_webui;
-  }
-
-  // This method simulates the expectation that different WebUI instance types
-  // would be created. The |type| value will be returned by GetWebUIType casted
-  // to WebUI::TypeID.
-  // As WebUI::TypeID is a typedef to void pointer, factory implementations
-  // return values that they know to be unique to their respective cases. So
-  // values set here should be safe if kept very low (just above zero).
-  void set_webui_type(uintptr_t type) {
-    CHECK_NE(reinterpret_cast<WebUI::TypeID>(type), WebUI::kNoWebUI);
-    type_ = type;
-  }
-
   // WebUIFactory implementation.
   std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
       WebUI* web_ui,
       const GURL& url) override {
     // If WebUI creation is enabled for the test and this is a WebUI URL,
     // returns a new instance.
-    if (should_create_webui_ && HasWebUIScheme(url))
+    if (HasWebUIScheme(url))
       return std::make_unique<WebUIController>(web_ui);
     return nullptr;
   }
@@ -164,8 +147,8 @@ class RenderFrameHostManagerTestWebUIControllerFactory
                              const GURL& url) override {
     // If WebUI creation is enabled for the test and this is a WebUI URL,
     // returns a mock WebUI type.
-    if (should_create_webui_ && HasWebUIScheme(url)) {
-      return reinterpret_cast<WebUI::TypeID>(type_);
+    if (HasWebUIScheme(url)) {
+      return reinterpret_cast<WebUI::TypeID>(base::Hash(url.host()));
     }
     return WebUI::kNoWebUI;
   }
@@ -181,9 +164,6 @@ class RenderFrameHostManagerTestWebUIControllerFactory
   }
 
  private:
-  bool should_create_webui_;
-  uintptr_t type_;
-
   DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManagerTestWebUIControllerFactory);
 };
 
@@ -352,12 +332,6 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
     WebUIControllerFactory::UnregisterFactoryForTesting(&factory_);
   }
 
-  void set_should_create_webui(bool should_create_webui) {
-    factory_.set_should_create_webui(should_create_webui);
-  }
-
-  void set_webui_type(int type) { factory_.set_webui_type(type); }
-
   GURL isolated_cross_site_url() const {
     return GURL("http://isolated-cross-site.com");
   }
@@ -477,7 +451,6 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
 // different SiteInstances, BrowsingInstances, and RenderProcessHosts. This is
 // a regression test for bug 9364.
 TEST_F(RenderFrameHostManagerTest, ChromeSchemeProcesses) {
-  set_should_create_webui(true);
   const GURL kChromeUrl(GetWebUIURL("foo"));
   const GURL kDestUrl("http://www.google.com/");
 
@@ -970,7 +943,6 @@ TEST_F(RenderFrameHostManagerTest, Navigate) {
 
 // Tests WebUI creation.
 TEST_F(RenderFrameHostManagerTest, WebUI) {
-  set_should_create_webui(true);
   scoped_refptr<SiteInstance> instance =
       SiteInstance::Create(browser_context());
 
@@ -1021,7 +993,6 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
 // Tests that we can open a WebUI link in a new tab from a WebUI page and still
 // grant the correct bindings.  http://crbug.com/189101.
 TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) {
-  set_should_create_webui(true);
   scoped_refptr<SiteInstance> blank_instance =
       SiteInstance::Create(browser_context());
   blank_instance->GetProcess()->Init();
@@ -1093,8 +1064,6 @@ TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) {
 
 // Tests that a WebUI is correctly reused between chrome:// pages.
 TEST_F(RenderFrameHostManagerTest, WebUIWasReused) {
-  set_should_create_webui(true);
-
   // Navigate to a WebUI page.
   const GURL kUrl1(GetWebUIURL("foo"));
   contents()->NavigateAndCommit(kUrl1);
@@ -1111,8 +1080,6 @@ TEST_F(RenderFrameHostManagerTest, WebUIWasReused) {
 // Tests that a WebUI is correctly cleaned up when navigating from a chrome://
 // page to a non-chrome:// page.
 TEST_F(RenderFrameHostManagerTest, WebUIWasCleared) {
-  set_should_create_webui(true);
-
   // Navigate to a WebUI page.
   const GURL kUrl1(GetWebUIURL("foo"));
   contents()->NavigateAndCommit(kUrl1);
@@ -1438,50 +1405,6 @@ TEST_F(RenderFrameHostManagerTest, CleanUpSwappedOutRVHOnProcessCrash) {
       rfh2->GetRenderViewHost()->opener_frame_route_id());
 }
 
-// Test that RenderViewHosts created for WebUI navigations are properly
-// granted WebUI bindings even if an unprivileged swapped out RenderViewHost
-// is in the same process (http://crbug.com/79918).
-TEST_F(RenderFrameHostManagerTest, EnableWebUIWithSwappedOutOpener) {
-  set_should_create_webui(true);
-  const GURL kSettingsUrl(GetWebUIURL("chrome/settings"));
-  const GURL kPluginUrl(GetWebUIURL("plugins"));
-
-  // Navigate to an initial WebUI URL.
-  contents()->NavigateAndCommit(kSettingsUrl);
-
-  // Ensure the RVH has WebUI bindings.
-  TestRenderViewHost* rvh1 = test_rvh();
-  EXPECT_TRUE(rvh1->GetMainFrame()->GetEnabledBindings() &
-              BINDINGS_POLICY_WEB_UI);
-
-  // Create a new tab and simulate it being the opener for the main
-  // tab.  It should be in the same SiteInstance.
-  std::unique_ptr<TestWebContents> opener1(
-      TestWebContents::Create(browser_context(), rvh1->GetSiteInstance()));
-  RenderFrameHostManager* opener1_manager =
-      opener1->GetRenderManagerForTesting();
-  contents()->SetOpener(opener1.get());
-
-  // Navigate to a different WebUI URL (different SiteInstance, same
-  // BrowsingInstance).
-  contents()->NavigateAndCommit(kPluginUrl);
-  TestRenderViewHost* rvh2 = test_rvh();
-  EXPECT_NE(rvh1->GetSiteInstance(), rvh2->GetSiteInstance());
-  EXPECT_TRUE(
-      rvh1->GetSiteInstance()->IsRelatedSiteInstance(rvh2->GetSiteInstance()));
-
-  // Ensure a proxy and swapped out RVH are created in the first opener tab.
-  EXPECT_TRUE(
-      opener1_manager->GetRenderFrameProxyHost(rvh2->GetSiteInstance()));
-  TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
-      opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
-  EXPECT_FALSE(opener1_rvh->is_active());
-
-  // Ensure the new RVH has WebUI bindings.
-  EXPECT_TRUE(rvh2->GetMainFrame()->GetEnabledBindings() &
-              BINDINGS_POLICY_WEB_UI);
-}
-
 // Test that we reuse the same guest SiteInstance if we navigate across sites.
 TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
   GURL guest_url(std::string(kGuestScheme).append("://abc123"));
@@ -2057,8 +1980,6 @@ TEST_F(RenderFrameHostManagerTestWithSiteIsolation,
 // See https://crbug.com/536145.
 TEST_F(RenderFrameHostManagerTestWithSiteIsolation,
        DontGrantPendingWebUIToSubframe) {
-  set_should_create_webui(true);
-
   // Make sure the initial process is live so that the pending WebUI navigation
   // does not commit immediately.  Give the page a subframe as well.
   const GURL kUrl1("http://foo.com");
@@ -2516,8 +2437,6 @@ TEST_F(RenderFrameHostManagerTest,
 
 // Checks that a restore navigation to a WebUI works.
 TEST_F(RenderFrameHostManagerTest, RestoreNavigationToWebUI) {
-  set_should_create_webui(true);
-
   const GURL kInitUrl(GetWebUIURL("foo"));
   scoped_refptr<SiteInstanceImpl> initial_instance =
       SiteInstanceImpl::Create(browser_context());
@@ -2576,7 +2495,6 @@ TEST_F(RenderFrameHostManagerTest, RestoreNavigationToWebUI) {
 // Simulates two simultaneous navigations involving one WebUI where the current
 // RenderFrameHost commits.
 TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithOneWebUI1) {
-  set_should_create_webui(true);
   NavigationSimulator::NavigateAndCommitFromBrowser(contents(),
                                                     GetWebUIURL("foo/"));
 
@@ -2638,7 +2556,6 @@ TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithOneWebUI1) {
 // Simulates two simultaneous navigations involving one WebUI where the new,
 // cross-site RenderFrameHost commits.
 TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithOneWebUI2) {
-  set_should_create_webui(true);
   NavigationSimulator::NavigateAndCommitFromBrowser(contents(),
                                                     GetWebUIURL("foo/"));
 
@@ -2696,8 +2613,6 @@ TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithOneWebUI2) {
 // Simulates two simultaneous navigations involving two WebUIs where the current
 // RenderFrameHost commits.
 TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithTwoWebUIs1) {
-  set_should_create_webui(true);
-  set_webui_type(1);
   NavigationSimulator::NavigateAndCommitFromBrowser(contents(),
                                                     GetWebUIURL("foo"));
 
@@ -2718,8 +2633,7 @@ TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithTwoWebUIs1) {
   EXPECT_EQ(web_ui1, host1->pending_web_ui());
   EXPECT_FALSE(GetPendingFrameHost(manager));
 
-  // Navigation another WebUI page, with a different type.
-  set_webui_type(2);
+  // Navigate to another WebUI page.
   const GURL kUrl(GetWebUIURL("bar"));
   auto navigation =
       NavigationSimulator::CreateBrowserInitiated(kUrl, contents());
@@ -2762,8 +2676,6 @@ TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithTwoWebUIs1) {
 // Simulates two simultaneous navigations involving two WebUIs where the new,
 // cross-site RenderFrameHost commits.
 TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithTwoWebUIs2) {
-  set_should_create_webui(true);
-  set_webui_type(1);
   NavigationSimulator::NavigateAndCommitFromBrowser(contents(),
                                                     GetWebUIURL("foo/"));
 
@@ -2784,8 +2696,7 @@ TEST_F(RenderFrameHostManagerTest, SimultaneousNavigationWithTwoWebUIs2) {
   EXPECT_EQ(web_ui1, host1->pending_web_ui());
   EXPECT_FALSE(GetPendingFrameHost(manager));
 
-  // Navigation another WebUI page, with a different type.
-  set_webui_type(2);
+  // Navigate to another WebUI page.
   const GURL kUrl(GetWebUIURL("bar/"));
   auto navigation =
       NavigationSimulator::CreateBrowserInitiated(kUrl, contents());
@@ -2872,7 +2783,6 @@ TEST_F(RenderFrameHostManagerTest, CanCommitOrigin) {
 // Tests that the correct intermediary and final navigation states are reached
 // when navigating from a renderer that is not live to a WebUI URL.
 TEST_F(RenderFrameHostManagerTest, NavigateFromDeadRendererToWebUI) {
-  set_should_create_webui(true);
   RenderFrameHostManager* manager = contents()->GetRenderManagerForTesting();
 
   RenderFrameHostImpl* initial_host = manager->current_frame_host();
@@ -2948,7 +2858,6 @@ TEST_F(RenderFrameHostManagerTest, NavigateFromDeadRendererToWebUI) {
 // Tests that the correct intermediary and final navigation states are reached
 // when navigating same-site between two WebUIs of the same type.
 TEST_F(RenderFrameHostManagerTest, NavigateSameSiteBetweenWebUIs) {
-  set_should_create_webui(true);
   NavigationSimulator::NavigateAndCommitFromBrowser(contents(),
                                                     GetWebUIURL("foo"));
 
@@ -2991,8 +2900,6 @@ TEST_F(RenderFrameHostManagerTest, NavigateSameSiteBetweenWebUIs) {
 TEST_F(RenderFrameHostManagerTest, NavigateCrossSiteBetweenWebUIs) {
   // Cross-site navigations will always cause the change of the WebUI instance
   // but for consistency sake different types will be set for each navigation.
-  set_should_create_webui(true);
-  set_webui_type(1);
   NavigationSimulator::NavigateAndCommitFromBrowser(contents(),
                                                     GetWebUIURL("foo"));
 
@@ -3001,12 +2908,9 @@ TEST_F(RenderFrameHostManagerTest, NavigateCrossSiteBetweenWebUIs) {
   EXPECT_TRUE(host->IsRenderFrameLive());
   EXPECT_TRUE(host->web_ui());
 
-  // Set the WebUI controller to return a different WebUIType value. This will
-  // cause the next navigation to "chrome://bar" to require a different WebUI
-  // than the current one, forcing it to be treated as cross-site.
-  set_webui_type(2);
-
-  // Navigation request.
+  // Navigate to different WebUI. This will cause the next navigation to
+  // "chrome://bar" to require a different WebUI than the current one,
+  // forcing it to be treated as cross-site.
   const GURL kUrl(GetWebUIURL("bar"));
   auto web_ui_navigation =
       NavigationSimulator::CreateBrowserInitiated(kUrl, contents());
@@ -3021,15 +2925,8 @@ TEST_F(RenderFrameHostManagerTest, NavigateCrossSiteBetweenWebUIs) {
   WebUIImpl* next_web_ui = manager->GetNavigatingWebUI();
   EXPECT_TRUE(next_web_ui);
   EXPECT_EQ(next_web_ui, speculative_host->web_ui());
-  EXPECT_NE(next_web_ui, manager->current_frame_host()->web_ui());
-  EXPECT_FALSE(speculative_host->pending_web_ui());
-
-  EXPECT_TRUE(manager->current_frame_host()->web_ui());
-  EXPECT_FALSE(manager->current_frame_host()->pending_web_ui());
-  EXPECT_EQ(speculative_host, GetPendingFrameHost(manager));
-  EXPECT_NE(next_web_ui, manager->current_frame_host()->web_ui());
-  EXPECT_EQ(next_web_ui, speculative_host->web_ui());
   EXPECT_EQ(next_web_ui, manager->GetNavigatingWebUI());
+  EXPECT_NE(next_web_ui, manager->current_frame_host()->web_ui());
   EXPECT_FALSE(speculative_host->pending_web_ui());
 
   // The RenderFrameHost committed.