Track all extension frames in ProcessManager, inspect extensionoptions

ProcessManager::GetRenderFrameHostsForExtension did not return all RFHs
for a given extension, because of two errors:

1. An extension can have multiple SiteInstances, so SiteInstances should
   not be compared by pointer.
2. ExtensionWebContentsObserver failed to register RFHs after a process
   swap.

I discovered this when I noticed that tests started to fail unexpectedly
after applying https://codereview.chromium.org/1413543005. That patch
changes the extension messaging API, to route messages to RFHs instead
of processes. This requires extension frames to be tracked correctly...

Extension tabs, frames, etc. are now visible at "Inspect views" in the
extensions view (when developer mode is enabled), thanks to the fact
that all extension frames are now being tracked (excluding extension
frames that are hosted in a view without classification, e.g. developer
tools and most of the GuestViews).

BUG=432875,550022
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
TEST=ExtensionApiTest.MessagingNoBackground does not fail any more with
https://codereview.chromium.org/1413543005 after applying this patch.
DeveloperPrivateApiTest.InspectEmbeddedOptionsPage passes.
ProcessManagerBrowserTest.NoBackgroundPage passes.
ProcessManagerBrowserTest.FrameClassification passes with and without
OOPIF.

Review URL: https://codereview.chromium.org/1413853005

Cr-Commit-Position: refs/heads/master@{#363368}

1 files changed, 29 insertions, 1 deletions

diff --git a/extensions/browser/extension_web_contents_observer.h b/extensions/browser/extension_web_contents_observer.h
index c92ad9c..5b86dc0 100644
--- a/extensions/browser/extension_web_contents_observer.h
+++ b/extensions/browser/extension_web_contents_observer.h
@@ -29,6 +29,23 @@ class Extension;
 // WebContents. It must be a subclass so that creating an instance via
 // content::WebContentsUserData::CreateForWebContents() provides an object of
 // the correct type. For an example, see ChromeExtensionWebContentsObserver.
+//
+// This class is responsible for maintaining the registrations of extension
+// frames with the ProcessManager. Only frames in an extension process are
+// registered. If out-of-process frames are enabled, every frame hosts a
+// chrome-extension: page. Otherwise non-extension frames may erroneously be
+// registered, but only briefly until they are correctly classified. This is
+// achieved using the following notifications:
+// 1. RenderFrameCreated - registers all new frames in extension processes.
+// 2. DidCommitProvisionalLoadForFrame - unregisters non-extension frames.
+// 3. DidNavigateAnyFrame - registers extension frames if they had been
+//    unregistered.
+//
+// Without OOPIF, non-extension frames created by the Chrome extension are also
+// registered at RenderFrameCreated. When the non-extension page is committed,
+// we detect that the unexpected URL and unregister the frame.
+// With OOPIF only the first notification is sufficient in most cases, except
+// for sandboxed frames with a unique origin.
 class ExtensionWebContentsObserver
     : public content::WebContentsObserver,
       public ExtensionFunctionDispatcher::Delegate {
@@ -60,6 +77,13 @@ class ExtensionWebContentsObserver
 
   void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
+  void DidCommitProvisionalLoadForFrame(
+      content::RenderFrameHost* render_frame_host,
+      const GURL& url,
+      ui::PageTransition transition_type) override;
+  void DidNavigateAnyFrame(content::RenderFrameHost* render_frame_host,
+                           const content::LoadCommittedDetails& details,
+                           const content::FrameNavigateParams& params) override;
 
   // Subclasses should call this first before doing their own message handling.
   bool OnMessageReceived(const IPC::Message& message,
@@ -77,8 +101,12 @@ class ExtensionWebContentsObserver
 
   // Returns the extension associated with the given |render_frame_host|, or
   // null if there is none.
+  // If |verify_url| is false, only the SiteInstance is taken into account.
+  // If |verify_url| is true, the frame's last committed URL is also used to
+  // improve the classification of the frame.
   const Extension* GetExtensionFromFrame(
-      content::RenderFrameHost* render_frame_host) const;
+      content::RenderFrameHost* render_frame_host,
+      bool verify_url) const;
 
   // TODO(devlin): Remove these once callers are updated to use the FromFrame
   // equivalents.