Implement OOPIFs within Devtools Extensions

chrome/browser/devtools/devtools_sanity_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/cancelable_callback.h"
@@ skipping 20 matching lines @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension_process_policy.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/test_chrome_web_ui_controller_factory.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/app_modal/javascript_app_modal_dialog.h"
 #include "components/app_modal/native_app_modal_dialog.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/devtools_agent_host.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/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/url_data_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/browser/worker_service.h"
 #include "content/public/browser/worker_service_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
@@ skipping 14 matching lines @@
 #include "ui/gl/gl_switches.h"
 #include "url/gurl.h"
 
 using app_modal::AppModalDialog;
 using app_modal::JavaScriptAppModalDialog;
 using app_modal::NativeAppModalDialog;
 using content::BrowserThread;
 using content::DevToolsAgentHost;
 using content::NavigationController;
 using content::RenderViewHost;
+using content::RenderFrameHost;

[alexmos, 2017/02/08 21:48:00]

not needed, since you're including the .h for it above.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

 using content::WebContents;
 using content::WorkerService;
 using content::WorkerServiceObserver;
 using extensions::Extension;
 
 namespace {
 
 const char kDebuggerTestPage[] = "files/devtools/debugger_test_page.html";
 const char kPauseWhenLoadingDevTools[] =
     "files/devtools/pause_when_loading_devtools.html";
@@ skipping 801 matching lines @@
 IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
                        TestDevToolsExtensionAPI) {
   LoadExtension("devtools_extension");
   RunTest("waitForTestResultsInConsole", std::string());
 }
 
 // Tests a chrome.devtools extension panel that embeds an http:// iframe.
 IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest, DevToolsExtensionWithHttpIframe) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  // Our extension must load an URL from the test server, whose port is only
+  // Our extension must load a URL from the test server, whose port is only
   // known at runtime. So, to embed the URL, we must dynamically generate the
   // extension, rather than loading it from static content.
   std::unique_ptr<extensions::TestExtensionDir> dir(
       new extensions::TestExtensionDir());
 
   extensions::DictionaryBuilder manifest;
   dir->WriteManifest(extensions::DictionaryBuilder()
                          .Set("name", "Devtools Panel w/ HTTP Iframe")
                          .Set("version", "1")
                          .Set("manifest_version", 2)
                          .Set("devtools_page", "devtools.html")
                          .ToJSON());
 
-  dir->WriteFile(
-      FILE_PATH_LITERAL("devtools.html"),
-      "<html><head><script src='devtools.js'></script></head></html>");
+  GURL http_iframe_url =
+      embedded_test_server()->GetURL("a.com", "/popup_iframe.html");
+
+  GURL simple_page_url =
+      embedded_test_server()->GetURL("a.com", "/title1.html");
+
+  dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                 "<html><head><script "
+                 "src='devtools.js'></script></head><body><iframe src='" +

[alexmos, 2017/02/08 21:48:00]

Does this load when you install the extension, without requiring to switch to
any extension panel?  It's probably good to double-check that it loads as well
(for example, with DOMMessageQueue and waiting for "DONE", like others).

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+                     simple_page_url.spec() + "'></iframe></body></html>");
 
   dir->WriteFile(
       FILE_PATH_LITERAL("devtools.js"),
       "chrome.devtools.panels.create('iframe_panel',\n"
       "    null,\n"
       "    'panel.html',\n"
       "    function(panel) {\n"
       "      chrome.devtools.inspectedWindow.eval('console.log(\"PASS\")');\n"
       "    }\n"
-      ");\n");
+      ");\n"
+      "chrome.devtools.panels.elements.createSidebarPane('iframe_pane',\n"

[alexmos, 2017/02/08 21:48:00]

This test is getting kind of long; it's worth thinking about splitting off the
sidebar pane and the devtools.html iframe parts into separate tests.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+      "function(sidebar) {\n"
+      "sidebar.setPage('panel.html');\n"
+      "});\n");
 
-  GURL http_iframe =
-      embedded_test_server()->GetURL("a.com", "/popup_iframe.html");
   dir->WriteFile(FILE_PATH_LITERAL("panel.html"),
                  "<html><body>Extension panel.<iframe src='" +
-                     http_iframe.spec() + "'></iframe>");
+                     http_iframe_url.spec() + "'></iframe>");
 
   // Install the extension.
   const Extension* extension = LoadExtensionFromPath(dir->UnpackedPath());
   ASSERT_TRUE(extension);
 
   // Open a devtools window.
   OpenDevToolsWindow(kDebuggerTestPage, false);
 
   // Wait for the panel extension to finish loading -- it'll output 'PASS'
   // when it's installed. waitForTestResultsInConsole waits until that 'PASS'.
   RunTestFunction(window_, "waitForTestResultsInConsole");
 
   // Now that we know the panel is loaded, switch to it. We'll wait until we
   // see a 'DONE' message sent from popup_iframe.html, indicating that it
   // loaded successfully.
+  SwitchToExtensionPanel(window_, extension, "iframe_panel");
   content::DOMMessageQueue message_queue;

[alexmos, 2017/02/08 21:48:00]

DOMMessageQueue has to be instantiated before the call that can trigger the
"DONE" message, in this case SwitchToExtensionPanel (like it was in the original
code), otherwise your test might be flaky due to races.  Same for all other uses
of DOMMessageQueue below.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

-  SwitchToExtensionPanel(window_, extension, "iframe_panel");
-  std::string message;
+  std::string message1;
+
   while (true) {
-    ASSERT_TRUE(message_queue.WaitForMessage(&message));
-    if (message == "\"DONE\"")
+    ASSERT_TRUE(message_queue.WaitForMessage(&message1));
+    if (message1 == "\"DONE\"")
       break;
   }
+
+  SwitchToPanel(window_, "elements");

[alexmos, 2017/02/08 21:48:00]

What is this one for?

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

added comment: "//This is a bit of a hack to switch to the sidebar pane in the
elements panel that the Iframe has been added to."

+  SwitchToPanel(window_, "iframe_pane");
+  content::DOMMessageQueue message_queue2;
+  std::string message2;
+
+  while (true) {
+    ASSERT_TRUE(message_queue2.WaitForMessage(&message2));
+    if (message2 == "\"DONE\"")
+      break;
+  }
+
+  int oopif_count = 0;
+  int frame_count = 0;
+  std::vector<RenderFrameHost*> rfhs = main_web_contents()->GetAllFrames();
+  for (std::vector<RenderFrameHost*>::size_type i = 0; i != rfhs.size(); i++) {
+    frame_count++;
+    if (rfhs[i]->IsCrossProcessSubframe()) {
+      oopif_count++;
+    }
+  }
+  ASSERT_EQ(frame_count, 7);

[alexmos, 2017/02/08 21:48:00]

I think this kind of checking might not be sufficient -- it doesn't verify that
the right frames are OOPIFs.  E.g., if the extension iframes became OOPIFs but
the web iframes didn't, this wouldn't catch it.  It's probably better to
explicitly look up a RFH for each extension iframe, and then for each web
iframe, and verify that the SiteInstance for each extension RFH is the same as
SiteInstance of the main devtools RFH, and that the SiteInstance of each web
iframe RFH is different.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    ASSERT_EQ(oopif_count, 3);
+  } else {
+    ASSERT_EQ(oopif_count, 0);
+  }
+}
+
+// Tests a chrome.devtools extension panel that embeds an http:// iframe.

[alexmos, 2017/02/08 21:48:00]

Is this true?  Looks like test.html is a iframe from a second, non-devtools
extension, not a http:// iframe.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Sorry, comments have been corrected.

+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       DevToolsExtensionWithRegExtension) {

[alexmos, 2017/02/08 21:48:00]

What does RegExtension stand for?

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

All occurrences have been renamed to non-devtools extension.

+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Our extension must load a URL from the test server, whose port is only
+  // known at runtime. So, to embed the URL, we must dynamically generate the
+  // extension, rather than loading it from static content.
+  std::unique_ptr<extensions::TestExtensionDir> devtools_dir(
+      new extensions::TestExtensionDir());
+
+  devtools_dir->WriteManifest(extensions::DictionaryBuilder()
+                                  .Set("name", "Devtools Extension")
+                                  .Set("version", "1")
+                                  .Set("manifest_version", 2)
+                                  .Set("devtools_page", "devtools.html")
+                                  .ToJSON());
+
+  devtools_dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                          "<html><head><script "
+                          "src='devtools.js'></script></head><body><iframe></"
+                          "iframe></body></html>");
+
+  devtools_dir->WriteFile(
+      FILE_PATH_LITERAL("devtools.js"),
+      "chrome.devtools.panels.create('iframe_panel',\n"
+      "    null,\n"
+      "    'panel.html',\n"
+      "    function(panel) {\n"
+      "      chrome.devtools.inspectedWindow.eval('console.log(\"PASS\")');\n"
+      "    }\n"
+      ");\n");
+
+  devtools_dir->WriteFile(FILE_PATH_LITERAL("panel.html"),
+                          "<html><body>does nothing</body></html>");
+
+  // Our extension must load a URL from the test server, whose port is only
+  // known at runtime. So, to embed the URL, we must dynamically generate the
+  // extension, rather than loading it from static content.
+  std::unique_ptr<extensions::TestExtensionDir> reg_dir(
+      new extensions::TestExtensionDir());
+
+  reg_dir->WriteManifest(
+      extensions::DictionaryBuilder()
+          .Set("name", "Regular Extension")
+          .Set("version", "1")
+          .Set("manifest_version", 2)
+          .Set("web_accessible_resources",
+               extensions::ListBuilder().Append("test.html").Build())
+          .ToJSON());
+
+  reg_dir->WriteFile(FILE_PATH_LITERAL("test.html"),
+                     "<html><head></head><body>This is a test</body></html>");
+
+  // Install the extensions.
+  const Extension* devtools_extension =
+      LoadExtensionFromPath(devtools_dir->UnpackedPath());
+  ASSERT_TRUE(devtools_extension);
+
+  const Extension* reg_extension =
+      LoadExtensionFromPath(reg_dir->UnpackedPath());
+  ASSERT_TRUE(reg_extension);
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  // Wait for the panel extension to finish loading -- it'll output 'PASS'
+  // when it's installed. waitForTestResultsInConsole waits until that 'PASS'.
+  RunTestFunction(window_, "waitForTestResultsInConsole");
+
+  GURL extension_file_url = reg_extension->GetResourceURL("/test.html");
+
+  RenderFrameHost* rfh = main_web_contents()->GetAllFrames()[2];

[alexmos, 2017/02/08 21:48:00]

Which one is this one?  You can rename the var so it's clear.  

General note for all tests and all uses of GetAllFrames: it's best not to rely
on the order of frames in GetAllFrames, which might change - I don't think that
function's API isn't guaranteeing any particular order.  It'd be better to use
ChildFrameAt (for finding child frames by index) or FrameMatchesName (finding
frames by name) in browser_test_utils.h, which would be more reliable for what
you want.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+
+  base::string16 javascript =
+      base::UTF8ToUTF16("location.href='" + extension_file_url.spec() + "';");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  rfh->ExecuteJavaScript(javascript);

[alexmos, 2017/02/08 21:48:00]

Why use this instead of EXPECT_TRUE(ExecuteScript())?  The latter is the
standard thing we use in all tests.  Same for all other uses.

[davidsac (gone - try alexmos), 2017/02/16 20:29:01]

Done.

+
+  manager.WaitForNavigationFinished();
+
+  std::vector<RenderFrameHost*> rfhs = main_web_contents()->GetAllFrames();
+
+  ASSERT_EQ(rfhs.size(), (unsigned)3);

[alexmos, 2017/02/08 21:48:01]

Reverse order.  It should be (expected, actual) everywhere.  Also (unsigned)3 ->
3U.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+  ASSERT_TRUE(rfhs[0]->GetLastCommittedURL().SchemeIs("chrome-devtools"));

[alexmos, 2017/02/08 21:48:00]

ASSERT->EXPECT.  ASSERT should be used when the test can't continue if this
isn't true, but most places where you have it don't need that and can use EXPECT
instead.  That way if things go wrong, your test can show everything that's
wrong instead of stopping here.

[alexmos, 2017/02/08 21:48:00]

It's very difficult to know what indices correspond to which RFH.  Better to
instantiate the RFHs with names like |devtools_rfh|, extension_b_rfh,
web_iframe_rfh, etc, which will make the tests more readable.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+  ASSERT_EQ(rfhs[1]->GetLastCommittedURL(),
+            devtools_extension->GetResourceURL("/devtools.html"));
+  ASSERT_EQ(rfhs[2]->GetLastCommittedURL(), extension_file_url);
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    // test.html's frame should be in extension b's
+    // process, not in devtools or extension a's process.
+    ASSERT_TRUE(
+        rfhs[0]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));

[alexmos, 2017/02/08 21:48:01]

Use kChromeDevToolsScheme instead of "chrome-devtools".

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+    ASSERT_TRUE(
+        rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_EQ(rfhs[2]->GetSiteInstance()->GetSiteURL(),
+              extension_file_url.GetOrigin());
+  } else {
+    // test.html's frame should be in the devtools process.
+    ASSERT_TRUE(
+        rfhs[0]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_TRUE(
+        rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_TRUE(
+        rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));

[alexmos, 2017/02/08 21:48:00]

rfhs[2]  (same bug copy-pasted in a couple of other places below)

[davidsac (gone - try alexmos), 2017/02/14 04:24:12]

Done.

+  }
+}
+
+// TODO(davidsac): fix comments, replace dynamic extension loading with files
+// where possible
+// Tests that if a devtools extension's devtools page has a subframe to a page
+// for another devtools extension, the subframe is rendered in either:
+// A) its own extension's process and not in devtools or the parent devtools
+// extension when in isolate-extensions or site-per-process mode.
+// B) the devtools process otherwise.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       DevToolsExtensionWithinDevToolsExtension) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Our extension must load a URL from the test server, whose port is only
+  // known at runtime. So, to embed the URL, we must dynamically generate the
+  // extension, rather than loading it from static content.
+  std::unique_ptr<extensions::TestExtensionDir> devtools_a_dir(
+      new extensions::TestExtensionDir());
+
+  devtools_a_dir->WriteManifest(extensions::DictionaryBuilder()
+                                    .Set("name", "Devtools Extension A")
+                                    .Set("version", "1")
+                                    .Set("manifest_version", 2)
+                                    .Set("devtools_page", "devtools.html")
+                                    .ToJSON());
+
+  devtools_a_dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                            "<html><head><script "
+                            "src='devtools.js'></script></head><body><iframe "
+                            "id='test-frame'></iframe></body></html>");
+
+  devtools_a_dir->WriteFile(
+      FILE_PATH_LITERAL("devtools.js"),
+      "chrome.devtools.panels.create('iframe_panel',\n"
+      "    null,\n"
+      "    'panel.html',\n"
+      "    function(panel) {\n"
+      "      chrome.devtools.inspectedWindow.eval('console.log(\"PASS\")');\n"
+      "    }\n"
+      ");\n");
+
+  devtools_a_dir->WriteFile(FILE_PATH_LITERAL("panel.html"),
+                            "<html><body>does nothing</body></html>");

[alexmos, 2017/02/08 21:48:01]

Might be better for this test to put the iframe with the second extension here
in the panel, instead of in the devtools.html page; I'd imagine that might be
slightly more likely to occur in practice?  Not a big difference though.

[davidsac (gone - try alexmos), 2017/02/16 20:29:01]

Done.

+
+  // Install the extension.
+  const Extension* devtools_a_extension =
+      LoadExtensionFromPath(devtools_a_dir->UnpackedPath());
+  ASSERT_TRUE(devtools_a_extension);
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  // Wait for the panel extension to finish loading -- it'll output 'PASS'
+  // when it is. waitForTestResultsInConsole waits until that 'PASS'.
+  RunTestFunction(window_, "waitForTestResultsInConsole");
+
+  // Our extension must load a URL from the test server, whose port is only
+  // known at runtime. So, to embed the URL, we must dynamically generate the
+  // extension, rather than loading it from static content.
+  std::unique_ptr<extensions::TestExtensionDir> devtools_b_dir(
+      new extensions::TestExtensionDir());
+
+  devtools_b_dir->WriteManifest(
+      extensions::DictionaryBuilder()
+          .Set("name", "Devtools Extension B")
+          .Set("version", "1")
+          .Set("manifest_version", 2)
+          .Set("devtools_page", "devtools.html")
+          .Set("web_accessible_resources",
+               extensions::ListBuilder().Append("test.html").Build())
+          .ToJSON());
+
+  devtools_b_dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                            "<html><head></head><body></body></html>");
+
+  devtools_b_dir->WriteFile(
+      FILE_PATH_LITERAL("test.html"),
+      "<html><head></head><body>This is a test</body></html>");
+
+  const Extension* devtools_b_extension =
+      LoadExtensionFromPath(devtools_b_dir->UnpackedPath());
+  ASSERT_TRUE(devtools_b_extension);
+
+  GURL extension_file_url = devtools_b_extension->GetResourceURL("/test.html");
+
+  LOG(INFO) << extension_file_url.spec();
+
+  RenderFrameHost* rfh = main_web_contents()->GetAllFrames()[2];
+
+  base::string16 javascript =
+      base::UTF8ToUTF16("location.href='" + extension_file_url.spec() + "';");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  rfh->ExecuteJavaScript(javascript);
+
+  manager.WaitForNavigationFinished();
+
+  std::vector<RenderFrameHost*> rfhs = main_web_contents()->GetAllFrames();
+
+  // TODO the expected result is the exact same as a reg extension in devtools
+  ASSERT_EQ(rfhs.size(), (unsigned)3);
+  ASSERT_TRUE(rfhs[0]->GetLastCommittedURL().SchemeIs("chrome-devtools"));
+  ASSERT_EQ(rfhs[1]->GetLastCommittedURL(),
+            devtools_extension->GetResourceURL("/devtools.html"));
+  ASSERT_EQ(rfhs[2]->GetLastCommittedURL(), extension_file_url);
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    // test.html's frame should be in extension b's
+    // process, not in devtools or extension a's process.
+    ASSERT_TRUE(
+        rfhs[0]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_TRUE(
+        rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_EQ(rfhs[2]->GetSiteInstance()->GetSiteURL(),
+              extension_file_url.GetOrigin());
+  } else {
+    // test.html's frame should be in the devtools process.
+    ASSERT_TRUE(
+        rfhs[0]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_TRUE(
+        rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+    ASSERT_TRUE(
+        rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+  }
+}
+
+// Tests a chrome.devtools extension panel that embeds an http:// iframe.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest, DevToolsExtensionWithinItself) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Our extension must load a URL from the test server, whose port is only
+  // known at runtime. So, to embed the URL, we must dynamically generate the
+  // extension, rather than loading it from static content.
+  std::unique_ptr<extensions::TestExtensionDir> dir(
+      new extensions::TestExtensionDir());
+
+  extensions::DictionaryBuilder manifest;
+  dir->WriteManifest(
+      extensions::DictionaryBuilder()
+          .Set("name", "Devtools Extension")
+          .Set("version", "1")
+          .Set("manifest_version", 2)
+          .Set("devtools_page", "devtools.html")
+          .Set("web_accessible_resources",
+               extensions::ListBuilder().Append("test.html").Build())
+          .ToJSON());
+
+  dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                 "<html><head><script "
+                 "src='devtools.js'></script></head><body><iframe></iframe></"
+                 "body></html>");
+
+  dir->WriteFile(
+      FILE_PATH_LITERAL("devtools.js"),
+      "chrome.devtools.panels.create('iframe_panel',\n"
+      "    null,\n"
+      "    'panel.html',\n"
+      "    function(panel) {\n"
+      "      chrome.devtools.inspectedWindow.eval('console.log(\"PASS\")');\n"
+      "    }\n"
+      ");\n");
+
+  dir->WriteFile(FILE_PATH_LITERAL("panel.html"),
+                 "<html><body>does nothing</body></html>");
+
+  dir->WriteFile(FILE_PATH_LITERAL("test.html"),
+                 "<html><head></head><body>This is a test</body></html>");
+
+  // Install the extension.
+  const Extension* extension = LoadExtensionFromPath(dir->UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  // Wait for the panel extension to finish loading -- it'll output 'PASS'
+  // when it's installed. waitForTestResultsInConsole waits until that 'PASS'.
+  RunTestFunction(window_, "waitForTestResultsInConsole");
+
+  GURL extension_file_url = extension->GetResourceURL("/test.html");
+
+  LOG(INFO) << extension_file_url.spec();
+
+  RenderFrameHost* rfh = main_web_contents()->GetAllFrames()[2];
+
+  base::string16 javascript =
+      base::UTF8ToUTF16("location.href='" + extension_file_url.spec() + "';");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  rfh->ExecuteJavaScript(javascript);
+
+  manager.WaitForNavigationFinished();
+
+  std::vector<RenderFrameHost*> rfhs = main_web_contents()->GetAllFrames();
+
+  // all frames should be in the devtools process.
+  ASSERT_EQ(rfhs.size(), (unsigned)3);
+  ASSERT_TRUE(rfhs[0]->GetLastCommittedURL().SchemeIs("chrome-devtools"));
+  ASSERT_EQ(rfhs[1]->GetLastCommittedURL(),
+            devtools_extension->GetResourceURL("/devtools.html"));
+  ASSERT_EQ(rfhs[2]->GetLastCommittedURL(), extension_file_url);
+  ASSERT_TRUE(
+      rfhs[0]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+  ASSERT_TRUE(
+      rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+  ASSERT_TRUE(
+      rfhs[1]->GetSiteInstance()->GetSiteURL().SchemeIs("chrome-devtools"));
+}
+
+// Tests that a regular extension without devtools permissions cannot be
+// injected into devtools when isolate-extensions or site-per-process mode.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest, RegularExtensionInDevtools) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // TODO:(davidsac) Get rid of this dynamic loading and opt into static
+  // loading? Our extension must load a URL from the test server, whose port is
+  // only
+  // known at runtime. So, to embed the URL, we must dynamically generate the
+  // extension, rather than loading it from static content.
+  std::unique_ptr<extensions::TestExtensionDir> dir(
+      new extensions::TestExtensionDir());
+
+  extensions::DictionaryBuilder manifest;
+  dir->WriteManifest(
+      extensions::DictionaryBuilder()
+          .Set("name", "Regular Extension")
+          .Set("version", "1")
+          .Set("manifest_version", 2)
+          .Set("web_accessible_resources",
+               extensions::ListBuilder().Append("test.html").Build())
+          .ToJSON());
+
+  dir->WriteFile(FILE_PATH_LITERAL("test.html"),
+                 "<html><head></head><body>This is a test</body></html>");
+
+  // Install the extension.
+  const Extension* extension = LoadExtensionFromPath(dir->UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  GURL extension_file_url = extension->GetResourceURL("/test.html");
+
+  LOG(INFO) << extension_file_url.spec();
+
+  RenderFrameHost* rfh = main_web_contents()->GetMainFrame();
+
+  base::string16 javascript = base::UTF8ToUTF16(
+      "var extensionFrame = document.createElement('iframe');"
+      "document.body.appendChild(extensionFrame);"
+      "extensionFrame.setAttribute('src', '" +
+      extension_file_url.spec() + "');");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  rfh->ExecuteJavaScript(javascript);
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    ASSERT_FALSE(manager.WaitForRequestStart());
+    // TODO(davidsac):  check to make sure that the rfh is about:blank or
+    // something

[alexmos, 2017/02/08 21:48:00]

Can you investigate how the request is actually blocked?  Then we will know how
to check for the proper error here.

[davidsac (gone - try alexmos), 2017/03/09 19:36:47]

Done.  Irrelevant.  This (and later tests) have been deleted due to the change
in the navigation policy.

+  } else {
+    manager.WaitForNavigationFinished();
+    std::vector<RenderFrameHost*> rfhs = main_web_contents()->GetAllFrames();
+    ASSERT_EQ(rfhs[1]->GetSiteInstance()->GetSiteURL(),
+              rfhs[0]->GetSiteInstance()->GetSiteURL());
+    ASSERT_EQ(rfhs[1]->GetLastCommittedURL(), extension_file_url);
+  }
+}
+
+// Tests that http iframes cannot be injected into devtools outside of devtools
+// extensions when isolate-extensions or site-per-process mode.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       DevToolsHttpIframeOutsideExtension) {

[alexmos, 2017/02/08 21:48:00]

Feels like you can combine this one with the previous one and just re-navigate
the same iframe.  It's basically about disallowing anything other than devtools
extensions in devtools iframes.

[davidsac (gone - try alexmos), 2017/02/16 20:29:01]

That's definitely a possibility.  In either case, it might be better to wait on
this until all of the tests are finalized.  I personally think it might be a bit
cleaner to keep them separate end easier to make sure that the later navigations
aren't effected by previous ones.  Maybe we can come back to this?

[davidsac (gone - try alexmos), 2017/03/09 19:36:47]

As mentioned earlier, this is no longer relevant as these tests have been
deleted now.

+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL simple_page_url =
+      embedded_test_server()->GetURL("a.com", "/title1.html");
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  base::string16 javascript = base::UTF8ToUTF16(
+      "var httpFrame = document.createElement('iframe');"
+      "document.body.appendChild(httpFrame);"
+      "httpFrame.setAttribute('src', '" +
+      simple_page_url.spec() + "');");
+
+  RenderFrameHost* rfh = main_web_contents()->GetMainFrame();
+
+  content::TestNavigationManager manager(main_web_contents(), simple_page_url);
+
+  rfh->ExecuteJavaScript(javascript);
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    ASSERT_FALSE(manager.WaitForRequestStart());
+    // TODO(davidsac):  check to make sure that the rfh is about:blank or
+    // something
+  } else {
+    manager.WaitForNavigationFinished();
+    std::vector<RenderFrameHost*> rfhs = main_web_contents()->GetAllFrames();
+    ASSERT_EQ(rfhs[1]->GetSiteInstance()->GetSiteURL(),
+              rfhs[0]->GetSiteInstance()->GetSiteURL());
+    ASSERT_EQ(rfhs[1]->GetLastCommittedURL(), simple_page_url);
+  }
 }
 
 // Some web features, when used from an extension, are subject to browser-side
 // security policy enforcement. Make sure they work properly from inside a
 // devtools extension.
 IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
                        DevToolsExtensionSecurityPolicyGrants) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
   std::unique_ptr<extensions::TestExtensionDir> dir(
@@ skipping 591 matching lines @@
 
   DevToolsWindowTesting::CloseDevToolsWindowSync(window);
   content::WebUIControllerFactory::UnregisterFactoryForTesting(&test_factory);
 }
 
 // Tests scripts panel showing.
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestDevToolsSharedWorker) {
   RunTest("testDevToolsSharedWorker", url::kAboutBlankURL);
 }