Track all extension frames in ProcessManager, inspect extensionoptions

extensions/browser/extension_web_contents_observer.cc

Index: extensions/browser/extension_web_contents_observer.cc
diff --git a/extensions/browser/extension_web_contents_observer.cc b/extensions/browser/extension_web_contents_observer.cc
index 665fc0809cca1b3ebcfa428ae79d762d9c8c5caa..bd69dfaa23164c47cae7aa33978a0018d5cabe81 100644
--- a/extensions/browser/extension_web_contents_observer.cc
+++ b/extensions/browser/extension_web_contents_observer.cc
@@ -4,12 +4,16 @@
 
 #include "extensions/browser/extension_web_contents_observer.h"
 
+#include "base/command_line.h"
+#include "components/guest_view/browser/guest_view_base.h"
 #include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -50,18 +54,38 @@ void ExtensionWebContentsObserver::InitializeRenderFrame(
   DCHECK(render_frame_host);
   DCHECK(render_frame_host->IsRenderFrameLive());
 
+  ViewType view_type = GetViewType(web_contents());
+  if (view_type == VIEW_TYPE_INVALID) {
+    // This is expected to happen only for non-extension pages (e.g. devtools)
+    // and GuestViews.
+    DCHECK(guest_view::GuestViewBase::FromWebContents(web_contents()) ||
+           !render_frame_host->GetSiteInstance()->GetSiteURL().SchemeIs(
+               kExtensionScheme));
+    return;
+  }
+
+  // At the initialization of the render frame, the last committed URL is not
+  // reliable, so do not take it into account in determining whether it is an
+  // extension frame.
+  const Extension* frame_extension =
+      GetExtensionFromFrame(render_frame_host, false);
+  // Only extension frames should be reported if site isolation is enabled, so
+  // |frame_extension| should exist at this point.
+  DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kSitePerProcess) ||
+         frame_extension);
+  if (!frame_extension)
+    return;
+
   // Notify the render frame of the view type.
   render_frame_host->Send(new ExtensionMsg_NotifyRenderViewType(
-      render_frame_host->GetRoutingID(), GetViewType(web_contents())));
-
-  const Extension* frame_extension = GetExtensionFromFrame(render_frame_host);
-  if (frame_extension) {
-    ExtensionsBrowserClient::Get()->RegisterMojoServices(render_frame_host,
-                                                         frame_extension);
-    ProcessManager::Get(browser_context_)
-        ->RegisterRenderFrameHost(web_contents(), render_frame_host,
-                                  frame_extension);
-  }
+      render_frame_host->GetRoutingID(), view_type));
+
+  ExtensionsBrowserClient::Get()->RegisterMojoServices(render_frame_host,
+                                                       frame_extension);
+  ProcessManager::Get(browser_context_)
+      ->RegisterRenderFrameHost(web_contents(), render_frame_host,
+                                frame_extension);
 }
 
 content::WebContents* ExtensionWebContentsObserver::GetAssociatedWebContents()
@@ -110,6 +134,49 @@ void ExtensionWebContentsObserver::RenderFrameDeleted(
       ->UnregisterRenderFrameHost(render_frame_host);
 }
 
+void ExtensionWebContentsObserver::DidCommitProvisionalLoadForFrame(
+    content::RenderFrameHost* render_frame_host,
+    const GURL& url,
+    ui::PageTransition transition_type) {
+  ProcessManager* pm = ProcessManager::Get(browser_context_);
+
+  if (pm->IsRenderFrameHostRegistered(render_frame_host)) {
+    const Extension* frame_extension =
+        GetExtensionFromFrame(render_frame_host, true);
+
+    if (!frame_extension)
+      pm->UnregisterRenderFrameHost(render_frame_host);
+  }
+}
+
+void ExtensionWebContentsObserver::DidNavigateAnyFrame(
+    content::RenderFrameHost* render_frame_host,
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  if (details.is_in_page)
+    return;
+
+  const Extension* frame_extension =
+      GetExtensionFromFrame(render_frame_host, true);
+  ProcessManager* pm = ProcessManager::Get(browser_context_);
+
+  if (!frame_extension) {
+    // Should have been unregistered by DidCommitProvisionalLoadForFrame.
+    DCHECK(!pm->IsRenderFrameHostRegistered(render_frame_host));
+    return;
+  }
+
+  if (pm->IsRenderFrameHostRegistered(render_frame_host)) {
+    // Notify ProcessManager, because some clients do not just want to know
+    // whether the frame is in an extension process, but also whether the frame
+    // was navigated.
+    pm->DidNavigateRenderFrameHost(render_frame_host);
+  } else {
+    pm->RegisterRenderFrameHost(web_contents(), render_frame_host,
+                                frame_extension);
+  }
+}
+
 bool ExtensionWebContentsObserver::OnMessageReceived(
     const IPC::Message& message,
     content::RenderFrameHost* render_frame_host) {
@@ -146,24 +213,39 @@ void ExtensionWebContentsObserver::PepperInstanceDeleted() {
 
 std::string ExtensionWebContentsObserver::GetExtensionIdFromFrame(
     content::RenderFrameHost* render_frame_host) const {
-  content::SiteInstance* site_instance = render_frame_host->GetSiteInstance();
-  GURL url = render_frame_host->GetLastCommittedURL();
-  if (!url.is_empty()) {
-    if (site_instance->GetSiteURL().GetOrigin() != url.GetOrigin())
-      return std::string();
-  } else {
-    url = site_instance->GetSiteURL();
-  }
-
-  return url.SchemeIs(kExtensionScheme) ? url.host() : std::string();
+  const Extension* extension = GetExtensionFromFrame(render_frame_host, true);
+  return extension ? extension->id() : std::string();
 }
 
 const Extension* ExtensionWebContentsObserver::GetExtensionFromFrame(
-    content::RenderFrameHost* render_frame_host) const {
-  return ExtensionRegistry::Get(
-             render_frame_host->GetProcess()->GetBrowserContext())
-      ->enabled_extensions()
-      .GetByID(GetExtensionIdFromFrame(render_frame_host));
+    content::RenderFrameHost* render_frame_host,
+    bool verify_url) const {
+  const GURL site_url(render_frame_host->GetSiteInstance()->GetSiteURL());
+  if (!site_url.SchemeIs(kExtensionScheme))
+    return nullptr;
+
+  const std::string& extension_id = site_url.host();
+  const Extension* extension =
+      ExtensionRegistry::Get(
+          render_frame_host->GetProcess()->GetBrowserContext())
+          ->enabled_extensions()
+          .GetByID(extension_id);
+  if (!extension)
+    return nullptr;
+
+  if (verify_url) {
+    // Without site isolation, this check is needed to eliminate non-extension
+    // schemes. With site isolation, this is still needed to exclude sandboxed
+    // extension frames with a unique origin.
+    if (!render_frame_host->GetLastCommittedOrigin().IsSameOriginWith(
+            url::Origin(site_url)) &&

[ncarter (slow), 2015/12/01 17:51:42]

I think this works only because extension urls don't have ports and subdomains.

If these could be http URLs, then this origin comparison would fail --
GetLastCommittedOrigin() could be http://mail.google.com:1234, but if you
canonicalize that to a site URL, it's http://google.com, stripping the port and
subdomain. (This canonicalization happens is because mail.google.com could
legally modify its origin by assigning document.domain to a superdomain
('google.com'), so the canonical site URL has to match for all origins that
could become same-origin through this kind of modification.

I think the slightly safer thing to do is to convert the origin back to a site
URL, and do the equality comparison in SiteURL-space:

if (origin.unique() ||
  site_url !=
  content::SiteInstance::GetSiteForURL(GURL(origin.Serialize()))) {

}

This also will map an app.com origin back to its effective chrome-extension URL.
I am thinking we should probably expose
content::SiteInstance::GetSiteForOrigin() to make this pattern more obvious -- I
will take care of that.

[robwu, 2015/12/01 23:19:58]

Done.

+        (!extension->is_hosted_app() ||
+         !extension->web_extent().MatchesURL(
+             render_frame_host->GetLastCommittedURL())))

[ncarter (slow), 2015/12/01 17:51:42]

Does this MatchesURL logic work properly for the about:blank case?

[robwu, 2015/12/01 23:19:58]

No.

+      return nullptr;
+  }
+
+  return extension;
 }
 
 const Extension* ExtensionWebContentsObserver::GetExtension(