Refactor of ExtensionNavigationThrottle

chrome/browser/extensions/extension_browsertest.cc

Index: chrome/browser/extensions/extension_browsertest.cc
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 35e284e25e9fdf71fa20c33f3bac09372dd8106f..c11954eae609c691042abea3b6eb16b82a620ec2 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -47,6 +47,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
@@ -535,26 +536,20 @@ void ExtensionBrowserTest::OpenWindow(content::WebContents* contents,
                                       const GURL& url,
                                       bool newtab_process_should_equal_opener,
                                       content::WebContents** newtab_result) {
-  content::WindowedNotificationObserver windowed_observer(
-      content::NOTIFICATION_LOAD_STOP,
-      content::NotificationService::AllSources());
+  content::WebContentsAddedObserver tab_added_observer;
   ASSERT_TRUE(content::ExecuteScript(contents,
                                      "window.open('" + url.spec() + "');"));
-
-  // The above window.open call is not user-initiated, so it will create
-  // a popup window instead of a new tab in current window.
-  // The stop notification will come from the new tab.
-  windowed_observer.Wait();
-  content::NavigationController* controller =
-      content::Source<content::NavigationController>(
-          windowed_observer.source()).ptr();
-  content::WebContents* newtab = controller->GetWebContents();
+  content::WebContents* newtab = tab_added_observer.GetWebContents();
   ASSERT_TRUE(newtab);
-  EXPECT_EQ(url, controller->GetLastCommittedEntry()->GetURL());
-  if (newtab_process_should_equal_opener)
-    EXPECT_EQ(contents->GetRenderProcessHost(), newtab->GetRenderProcessHost());
-  else
-    EXPECT_NE(contents->GetRenderProcessHost(), newtab->GetRenderProcessHost());
+  WaitForLoadStop(newtab);
+  EXPECT_EQ(url, newtab->GetLastCommittedURL());
+  if (newtab_process_should_equal_opener) {
+    EXPECT_EQ(contents->GetMainFrame()->GetSiteInstance(),

[ncarter (slow), 2017/05/01 21:50:37]

This is incidental cleanup, not actually necessary. Looking for a SiteInstance
swap is technically more correct, since two extensions might be assigned the
same a renderer. However, this didn't turn out to be the reason the test failed.

+              newtab->GetMainFrame()->GetSiteInstance());
+  } else {
+    EXPECT_NE(contents->GetMainFrame()->GetSiteInstance(),
+              newtab->GetMainFrame()->GetSiteInstance());
+  }
 
   if (newtab_result)
     *newtab_result = newtab;