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/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/common/switches.h"
 #include "extensions/common/value_builder.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "net/test/url_request/url_request_mock_http_job.h"
@@ skipping 432 matching lines @@
     size_t num_after = registry->enabled_extensions().size();
     if (num_after != (num_before + 1))
       return nullptr;
 
     if (!WaitForExtensionViewsToLoad())
       return nullptr;
 
     return GetExtensionByPath(registry->enabled_extensions(), path);
   }
 
+  const Extension* LoadExtensionForTest(std::string name,

[alexmos, 2017/02/15 01:23:41]

nit: const std::string& name

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

Done.

+                                        extensions::TestExtensionDir* dir,
+                                        bool has_test_page,
+                                        bool devtools,

[alexmos, 2017/02/15 01:23:41]

nit: is_devtools, or is_devtools_extension

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

Done.

+                                        bool devtools_script) {

[alexmos, 2017/02/15 01:23:41]

similarly, I think this actually means should_create_panel (this is what the
script actually does)

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

Done. Kinda.  Renamed to |should_create_panel_and_pane| to adjust for new
functionality in the script.

+    extensions::DictionaryBuilder manifest;
+    manifest.Set("name", name).Set("version", "1").Set("manifest_version", 2);
+    if (devtools) {
+      manifest.Set("devtools_page", "devtools.html");
+    }
+    if (has_test_page) {

[alexmos, 2017/02/15 01:23:41]

Do you need to pass this in?  What if you just always included test.html in the
extension, but it would just be used in some but not all tests?  Might be a bit
of extra work but it'll make the code easier to understand.

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

Done.  I hesitated to do this initially, because I was afraid there would be
collisions between one devtools extension trying to ask another extension's test
file, but I can see now that it's definitely not an issue.

+      manifest.Set("web_accessible_resources",
+                   extensions::ListBuilder().Append("test.html").Build());
+    }
+    dir->WriteManifest(manifest.ToJSON());
+
+    if (devtools) {
+      if (devtools_script) {
+        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>");
+      } else {
+        dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                       "<html><head></head><body></body></html>");
+      }
+    }
+
+    if (has_test_page) {
+      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());
+    return extension;
+  }
+
  private:
   const Extension* GetExtensionByPath(
       const extensions::ExtensionSet& extensions,
       const base::FilePath& path) {
     base::FilePath extension_path = base::MakeAbsoluteFilePath(path);
     EXPECT_TRUE(!extension_path.empty());
     for (const scoped_refptr<const Extension>& extension : extensions) {
       if (extension->path() == extension_path) {
         return extension.get();
       }
@@ skipping 370 matching lines @@
           kDebuggerTestPage);
 }
 
 // Tests that chrome.devtools extension is correctly exposed.
 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) {
+// Tests that http Iframes within the three "devtools pages" (the visible
+// devtools panel for the devtools extension, the sidebar pane page for the
+// devtools extension that is visible in the elemanets panel, and the devtools
+// background page, which is different from the extension's background
+// page)(https://developer.chrome.com/extensions/devtools) are rendered in their
+// own processes and not in the devtools process or the extension's process.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       HttpIframeInDevToolsExtensionPanel) {
   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()

[alexmos, 2017/02/15 01:23:41]

Can this test (and the next two) also use your new LoadExtensionForTest helper? 
(I realize the scripts to run differ; perhaps you could pass a flag to toggle
between using a panel or a sidebar?)

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

The first two use this now, but not the third.  Let me know how you feel about
this.  I think it would be fine to finaggle LoadExtensionForTest to work for the
third as well, but it might make the method more distorted and confusing to the
point that it's not worth it?

                          .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");

[alexmos, 2017/02/15 01:23:41]

This can just stay at its old location in the file, next to its use -- there's
no reason to move it up here, right?

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

The code has changed quite a bit, so I'm not sure this is a relevant comment any
more.  It is still at the beginning now so that the URL can be passed into the
helper method.

+
+  dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                 "<html><head><script "
+                 "src='devtools.js'></script></head><body></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");
 
-  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>");
+                 "<html><body><iframe src='" + http_iframe_url.spec() +
+                     "'></iframe></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");
 
   // 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.
   content::DOMMessageQueue message_queue;
   SwitchToExtensionPanel(window_, extension, "iframe_panel");
   std::string message;
+
   while (true) {
     ASSERT_TRUE(message_queue.WaitForMessage(&message));
     if (message == "\"DONE\"")
       break;
   }
+
+  std::vector<content::RenderFrameHost*> rfhs =
+      main_web_contents()->GetAllFrames();
+  EXPECT_EQ(4U, rfhs.size());
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+  content::RenderFrameHost* devtools_devtools_extension_rfh =
+      ChildFrameAt(main_devtools_rfh, 0);
+  content::RenderFrameHost* devtools_panel_extension_rfh =
+      ChildFrameAt(main_devtools_rfh, 1);
+  content::RenderFrameHost* http_iframe_rfh =
+      ChildFrameAt(devtools_panel_extension_rfh, 0);
+
+  EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_EQ(extension->GetResourceURL("/devtools.html"),
+            devtools_devtools_extension_rfh->GetLastCommittedURL());
+  EXPECT_EQ(extension->GetResourceURL("/panel.html"),
+            devtools_panel_extension_rfh->GetLastCommittedURL());
+  EXPECT_EQ(http_iframe_url, http_iframe_rfh->GetLastCommittedURL());
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()

[alexmos, 2017/02/15 01:23:41]

Instead of this, let's just expect that
devtools_devtools_extension_rfh->GetSiteInstance() is equal to
main_devtools_rfh->GetSiteInstance().  Same everywhere below.

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

Hmm I feel like it might still be helpful to check the scheme or origin for each
expected site instance to make sure that they are getting put in the right
process though?  Like, to make sure that they don't both have the wrong origin? 
Is that still reasonable?

+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(
+        devtools_panel_extension_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+            content::kChromeDevToolsScheme));
+    EXPECT_EQ(http_iframe_url.host(),
+              http_iframe_rfh->GetSiteInstance()->GetSiteURL().host());

[alexmos, 2017/02/15 01:23:41]

Similarly, for this kind of check, let's ensure that
http_iframe_rfh->GetSiteInstance() is not equal to the devtools SiteInstance.

[davidsac (gone - try alexmos), 2017/03/09 20:21:22]

Are you sure?  I can change it, but I felt like it might not be better to have a
more specific check.  Are we thinking the result for this might change for
things like TDI mode?

+
+  } else {
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(
+        devtools_panel_extension_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+            content::kChromeDevToolsScheme));
+    EXPECT_TRUE(http_iframe_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+  }
+}
+
+// Tests that http Iframes within the three "devtools pages" (the visible
+// devtools panel for the devtools extension, the sidebar pane page for the
+// devtools extension that is visible in the elemanets panel, and the devtools
+// background page, which is different from the extension's background
+// page)(https://developer.chrome.com/extensions/devtools) are rendered in their
+// own processes and not in the devtools process or the extension's process.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       HttpIframeInDevToolsExtensionSideBarPane) {
+  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());
+
+  dir->WriteManifest(extensions::DictionaryBuilder()
+                         .Set("name", "Devtools Panel w/ HTTP Iframe")
+                         .Set("version", "1")
+                         .Set("manifest_version", 2)
+                         .Set("devtools_page", "devtools.html")
+                         .ToJSON());
+
+  GURL http_iframe_url =
+      embedded_test_server()->GetURL("a.com", "/popup_iframe.html");
+
+  dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                 "<html><head><script "
+                 "src='devtools.js'></script></head><body></body></html>");
+
+  dir->WriteFile(
+      FILE_PATH_LITERAL("devtools.js"),
+      "chrome.devtools.panels.elements.createSidebarPane('iframe_pane',\n"
+      "function(sidebar) {\n"
+      "chrome.devtools.inspectedWindow.eval('console.log(\"PASS\")');\n"
+      "sidebar.setPage('panel.html');\n"
+      "});\n");
+
+  dir->WriteFile(FILE_PATH_LITERAL("panel.html"),
+                 "<html><body><iframe src='" + http_iframe_url.spec() +
+                     "'></iframe></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");
+
+  content::DOMMessageQueue message_queue;
+  SwitchToPanel(window_, "elements");
+  // 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");
+
+  std::string message;
+
+  while (true) {
+    ASSERT_TRUE(message_queue.WaitForMessage(&message));
+    if (message == "\"DONE\"")
+      break;
+  }
+
+  std::vector<content::RenderFrameHost*> rfhs =
+      main_web_contents()->GetAllFrames();
+  EXPECT_EQ(4U, rfhs.size());
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+  content::RenderFrameHost* devtools_devtools_extension_rfh =
+      ChildFrameAt(main_devtools_rfh, 0);
+  content::RenderFrameHost* devtools_sidebar_pane_extension_rfh =
+      ChildFrameAt(main_devtools_rfh, 1);
+  content::RenderFrameHost* http_iframe_rfh =
+      ChildFrameAt(devtools_sidebar_pane_extension_rfh, 0);
+
+  EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_EQ(extension->GetResourceURL("/devtools.html"),
+            devtools_devtools_extension_rfh->GetLastCommittedURL());
+  EXPECT_EQ(extension->GetResourceURL("/panel.html"),
+            devtools_sidebar_pane_extension_rfh->GetLastCommittedURL());
+  EXPECT_EQ(http_iframe_url, http_iframe_rfh->GetLastCommittedURL());
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_sidebar_pane_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_EQ(http_iframe_url.host(),
+              http_iframe_rfh->GetSiteInstance()->GetSiteURL().host());
+
+  } else {
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_sidebar_pane_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(http_iframe_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+  }
+}
+
+// Tests that http Iframes within the three "devtools pages" (the visible
+// devtools panel for the devtools extension, the sidebar pane page for the
+// devtools extension that is visible in the elemanets panel, and the devtools
+// background page, which is different from the extension's background
+// page)(https://developer.chrome.com/extensions/devtools) are rendered in their
+// own processes and not in the devtools process or the extension's process.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       HttpIframeInDevToolsExtensionDevtools) {
+  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());
+
+  GURL http_iframe_url =
+      embedded_test_server()->GetURL("a.com", "/popup_iframe.html");
+
+  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></head><body><iframe src='" +
+                     http_iframe_url.spec() + "'></iframe></body></html>");
+
+  // Install the extension.
+  const Extension* extension = LoadExtensionFromPath(dir->UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  // 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.
+  content::DOMMessageQueue message_queue;
+  std::string message;
+
+  while (true) {
+    ASSERT_TRUE(message_queue.WaitForMessage(&message));
+    if (message == "\"DONE\"")
+      break;
+  }
+
+  std::vector<content::RenderFrameHost*> rfhs =
+      main_web_contents()->GetAllFrames();
+  EXPECT_EQ(3U, rfhs.size());
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+  content::RenderFrameHost* devtools_devtools_extension_rfh =
+      ChildFrameAt(main_devtools_rfh, 0);
+  content::RenderFrameHost* http_iframe_rfh =
+      ChildFrameAt(devtools_devtools_extension_rfh, 0);
+
+  EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_EQ(extension->GetResourceURL("/devtools.html"),
+            devtools_devtools_extension_rfh->GetLastCommittedURL());
+  EXPECT_EQ(http_iframe_url, http_iframe_rfh->GetLastCommittedURL());
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_EQ(http_iframe_url.host(),
+              http_iframe_rfh->GetSiteInstance()->GetSiteURL().host());
+
+  } else {
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(http_iframe_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+  }
+}
+
+// tests that an iframe to a non-devtools extension embedded in a devtools
+// extension will be isolated from devtools and the devtools extension.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       NonDevToolsExtensionInDevToolsExtension) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<extensions::TestExtensionDir> dir(
+      new extensions::TestExtensionDir());
+
+  // Install the extensions.
+  const Extension* devtools_extension =
+      LoadExtensionForTest("Devtools Extension", dir.get(), false, true, true);

[alexmos, 2017/02/15 01:23:41]

To make the bools more readable (someone reading the test should know what they
refer to without needing to look up the helper definition), we typically either
include /* param_name */ right after the bool, or use an enum instead. 

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

Done.  Tell me if that looks good to you, though.

+  ASSERT_TRUE(devtools_extension);
+
+  std::unique_ptr<extensions::TestExtensionDir> dir2(
+      new extensions::TestExtensionDir());
+
+  const Extension* non_devtools_extension = LoadExtensionForTest(
+      "Non-DevTools Extension", dir2.get(), true, false, false);
+  ASSERT_TRUE(non_devtools_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 =
+      non_devtools_extension->GetResourceURL("/test.html");
+
+  std::vector<content::RenderFrameHost*> rfhs =
+      main_web_contents()->GetAllFrames();
+  EXPECT_EQ(3U, rfhs.size());
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+  content::RenderFrameHost* devtools_devtools_extension_rfh =
+      ChildFrameAt(main_devtools_rfh, 0);
+  content::RenderFrameHost* non_devtools_extension_rfh =
+      ChildFrameAt(devtools_devtools_extension_rfh, 0);
+
+  base::string16 javascript =
+      base::UTF8ToUTF16("location.href='" + extension_file_url.spec() + "';");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  non_devtools_extension_rfh->ExecuteJavaScript(javascript);

[alexmos, 2017/02/15 01:23:41]

Still wondering if there are any reasons to use ExecuteJavaScript?
Seems like 
  EXPECT_TRUE(ExecuteScript(non_devtools_extension_rfh, "your script"));
would work just as well, and it would also verify that the script executed
properly.

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

Done.  Sounds great.  It was on my todo list, it was just a temporarily
low-priority fix for me.

+
+  manager.WaitForNavigationFinished();
+
+  rfhs = main_web_contents()->GetAllFrames();
+  EXPECT_EQ(3U, rfhs.size());
+
+  EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_EQ(devtools_extension->GetResourceURL("/devtools.html"),
+            devtools_devtools_extension_rfh->GetLastCommittedURL());
+  EXPECT_EQ(extension_file_url,
+            non_devtools_extension_rfh->GetLastCommittedURL());
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    // test.html's frame should be in extension b's
+    // process, not in devtools or extension a's process.
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_EQ(extension_file_url.GetOrigin(),
+              non_devtools_extension_rfh->GetSiteInstance()->GetSiteURL());
+  } else {
+    // test.html's frame should be in the devtools process.
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(devtools_devtools_extension_rfh->GetSiteInstance()
+                    ->GetSiteURL()
+                    .SchemeIs(content::kChromeDevToolsScheme));
+    EXPECT_TRUE(
+        non_devtools_extension_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+            content::kChromeDevToolsScheme));
+  }
+}
+
+// Tests that if a devtools extension's devtools page has a subframe to a
+// page for another devtools extension, the subframe is rendered in the devtools
+// process as well.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       DevToolsExtensionInDevToolsExtension) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<extensions::TestExtensionDir> dir(
+      new extensions::TestExtensionDir());
+
+  // Install the extension.
+  const Extension* devtools_a_extension = LoadExtensionForTest(
+      "Devtools Extension A", dir.get(), false, true, true);
+  ASSERT_TRUE(devtools_a_extension);
+
+  std::unique_ptr<extensions::TestExtensionDir> dir2(
+      new extensions::TestExtensionDir());
+
+  const Extension* devtools_b_extension = LoadExtensionForTest(
+      "Devtools Extension B", dir2.get(), true, true, false);
+  ASSERT_TRUE(devtools_b_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");
+
+  GURL extension_file_url = devtools_b_extension->GetResourceURL("/test.html");

[alexmos, 2017/02/15 01:23:41]

I'd name this extension_b_url, to make its uses more readable.

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

Done.

+
+  std::vector<content::RenderFrameHost*> rfhs =
+      main_web_contents()->GetAllFrames();
+  EXPECT_EQ(3U, rfhs.size());
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+  content::RenderFrameHost* devtools_extension_a_devtools_rfh =

[alexmos, 2017/02/15 01:23:41]

no need for second "devtools" in the names here

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

We might want some other name differentiation then.  I agree that the name is
confusing and kinda ridiculous-sounding, but I was kinda planning ahead for the
current version, where there is both a devtools_rfh and panel_rfh for certain
extensions.

+      ChildFrameAt(main_devtools_rfh, 0);
+  content::RenderFrameHost* devtools_extension_b_devtools_rfh =
+      ChildFrameAt(devtools_extension_a_devtools_rfh, 0);
+
+  base::string16 javascript =
+      base::UTF8ToUTF16("location.href='" + extension_file_url.spec() + "';");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  devtools_extension_b_devtools_rfh->ExecuteJavaScript(javascript);
+
+  manager.WaitForNavigationFinished();
+
+  rfhs = main_web_contents()->GetAllFrames();
+  EXPECT_EQ(3U, rfhs.size());
+
+  // TODO the expected result is the exact same as a non-devtools extension in
+  // devtools
+  EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_EQ(devtools_a_extension->GetResourceURL("/devtools.html"),
+            devtools_extension_a_devtools_rfh->GetLastCommittedURL());
+  EXPECT_EQ(extension_file_url,
+            devtools_extension_b_devtools_rfh->GetLastCommittedURL());

[alexmos, 2017/02/15 01:23:42]

So, technically, at this point, if we ever regressed and started putting this
into a separate process, devtools_extension_b_devtools_rfh would be destroyed at
this point, and a new RFH would be created for that frame.  This test would then
crash here, due to the use-after-free on |devtools_extension_b_devtools_rfh|. 
Might be nice for the test to fail in a nicer way, but this is probably fine for
now.

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

Done. As part of my refactor, I got rid of the calls to executeScript, so I was
able to move the naming of all of the renderframehosts to the last part of the
test before the expect statements.

+  EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_TRUE(devtools_extension_a_devtools_rfh->GetSiteInstance()
+                  ->GetSiteURL()
+                  .SchemeIs(content::kChromeDevToolsScheme));
+  // test.html's frame should be in the devtools process.
+  EXPECT_TRUE(devtools_extension_b_devtools_rfh->GetSiteInstance()
+                  ->GetSiteURL()
+                  .SchemeIs(content::kChromeDevToolsScheme));
+}
+
+// Tests that a devtools extension can still have subframes to itself in a
+// "devtools page" and that they will be rendered within the devtools process as
+// well, not in the extension process.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest, DevToolsExtensionInItself) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+#if 0
+    std::unique_ptr<extensions::TestExtensionDir> dir(
+        new extensions::TestExtensionDir());
+  // Install the extension.
+  const Extension* extension =
+      LoadExtensionForTest("Devtools Extension", dir.get(), true, true, true);
+  ASSERT_TRUE(extension);
+
+
+
+  GURL extension_file_url = extension->GetResourceURL("/test.html");
+
+  // 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");
+
+  //SwitchToExtensionPanel(window_, extension, "iframe_panel");
+
+  base::string16 javascript =
+      base::UTF8ToUTF16("location.href='" + extension_file_url.spec() + "';");
+
+  content::TestNavigationManager manager(main_web_contents(),
+                                         extension_file_url);
+
+  content::RenderFrameHost* rfh = main_web_contents()->GetAllFrames()[2];
+  rfh->ExecuteJavaScript(javascript);
+
+  manager.WaitForNavigationFinished();
+
+#else
+  std::unique_ptr<extensions::TestExtensionDir> dir(
+      new extensions::TestExtensionDir());
+
+  extensions::DictionaryBuilder manifest;
+  manifest.Set("name", "Devtools Extension")
+      .Set("version", "1")
+      .Set("manifest_version", 2);
+  manifest.Set("devtools_page", "devtools.html");
+
+  manifest.Set("web_accessible_resources",
+               extensions::ListBuilder().Append("test.html").Build());
+
+  dir->WriteManifest(manifest.ToJSON());
+
+  dir->WriteFile(FILE_PATH_LITERAL("devtools.html"),
+                 "<html><head><script "
+                 "src='devtools.js'></script></head><body></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><iframe src='test.html'></iframe></body></html>");
+
+  dir->WriteFile(FILE_PATH_LITERAL("test.html"),
+                 "<html><head><script src='test.js'></script></head><body>This "
+                 "is a test</body></html>");
+
+  dir->WriteFile(FILE_PATH_LITERAL("test.js"),
+                 "window.onload = function() {\n"
+                 "  domAutomationController.setAutomationId(0);\n"
+                 "  domAutomationController.send('DONE');\n"
+                 "}\n");
+
+  // 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.
+  content::DOMMessageQueue message_queue;
+  SwitchToExtensionPanel(window_, extension, "iframe_panel");
+  std::string message1;
+
+  while (true) {
+    ASSERT_TRUE(message_queue.WaitForMessage(&message1));
+    if (message1 == "\"DONE\"")
+      break;
+  }
+
+  GURL extension_file_url = extension->GetResourceURL("/test.html");
+
+#endif
+
+  std::vector<content::RenderFrameHost*> rfhs =
+      main_web_contents()->GetAllFrames();
+  EXPECT_EQ(3U, rfhs.size());
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+  content::RenderFrameHost* devtools_extension_devtools_rfh =
+      ChildFrameAt(main_devtools_rfh, 0);
+  content::RenderFrameHost* devtools_extension_test_rfh =
+      ChildFrameAt(devtools_extension_devtools_rfh, 0);
+
+  // all frames should be in the devtools process.
+  EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_EQ(extension->GetResourceURL("/devtools.html"),
+            devtools_extension_devtools_rfh->GetLastCommittedURL());
+  EXPECT_EQ(extension_file_url,
+            devtools_extension_test_rfh->GetLastCommittedURL());
+  EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+      content::kChromeDevToolsScheme));
+  EXPECT_TRUE(
+      devtools_extension_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+          content::kChromeDevToolsScheme));
+  // test.html's frame should be in the devtools process.
+  EXPECT_TRUE(
+      devtools_extension_test_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+          content::kChromeDevToolsScheme));
+}
+
+// Tests that an Iframe to a non-devtools extension cannot be injected into
+// devtools when not contained in a devtools extension.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       NonDevToolsExtensionInDevToolsOutsideDevToolsExtension) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<extensions::TestExtensionDir> dir(
+      new extensions::TestExtensionDir());
+
+  // Install the extension.
+  const Extension* extension = LoadExtensionForTest(
+      "Non-DevTools Extension", dir.get(), true, false, false);
+  ASSERT_TRUE(extension);
+
+  // Open a devtools window.
+  OpenDevToolsWindow(kDebuggerTestPage, false);
+
+  GURL extension_file_url = extension->GetResourceURL("/test.html");
+
+  content::RenderFrameHost* main_devtools_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);
+
+  main_devtools_rfh->ExecuteJavaScript(javascript);
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    EXPECT_FALSE(manager.WaitForRequestStart());
+
+    std::vector<content::RenderFrameHost*> rfhs =
+        main_web_contents()->GetAllFrames();
+
+    EXPECT_EQ(1U, rfhs.size());
+
+    // TODO(davidsac):  check to make sure that the rfh is about:blank or
+    // something
+  } else {
+    manager.WaitForNavigationFinished();
+    std::vector<content::RenderFrameHost*> rfhs =
+        main_web_contents()->GetAllFrames();
+
+    EXPECT_EQ(2U, rfhs.size());
+
+    content::RenderFrameHost* extension_test_rfh =
+        ChildFrameAt(main_devtools_rfh, 0);
+    EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(extension_test_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_EQ(extension_file_url, extension_test_rfh->GetLastCommittedURL());
+  }
+}
+
+// Tests that a web Iframe cannot be injected into devtools when not contained
+// in a devtools extension.
+IN_PROC_BROWSER_TEST_F(DevToolsExtensionTest,
+                       HttpIframeInDevToolsOutsideDevToolsExtension) {
+  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() + "');");
+
+  content::RenderFrameHost* main_devtools_rfh =
+      main_web_contents()->GetMainFrame();
+
+  content::TestNavigationManager manager(main_web_contents(), simple_page_url);
+
+  main_devtools_rfh->ExecuteJavaScript(javascript);
+
+  if (content::AreAllSitesIsolatedForTesting() ||
+      extensions::IsIsolateExtensionsEnabled()) {
+    EXPECT_FALSE(manager.WaitForRequestStart());
+
+    std::vector<content::RenderFrameHost*> rfhs =
+        main_web_contents()->GetAllFrames();
+
+    EXPECT_EQ(1U, rfhs.size());
+    // TODO(davidsac):  check to make sure that the rfh is about:blank or
+    // something
+  } else {
+    manager.WaitForNavigationFinished();
+    std::vector<content::RenderFrameHost*> rfhs =
+        main_web_contents()->GetAllFrames();
+    EXPECT_EQ(2U, rfhs.size());
+    content::RenderFrameHost* http_iframe_rfh =
+        ChildFrameAt(main_devtools_rfh, 0);
+    EXPECT_TRUE(main_devtools_rfh->GetLastCommittedURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(main_devtools_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_TRUE(http_iframe_rfh->GetSiteInstance()->GetSiteURL().SchemeIs(
+        content::kChromeDevToolsScheme));
+    EXPECT_EQ(simple_page_url, http_iframe_rfh->GetLastCommittedURL());
+  }
 }
 
 // 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);
 }