Allow a main interstitial frame to act as an ExtensionMessagePort opener

chrome/browser/extensions/api/messaging/extension_message_port.cc

Index: chrome/browser/extensions/api/messaging/extension_message_port.cc
diff --git a/chrome/browser/extensions/api/messaging/extension_message_port.cc b/chrome/browser/extensions/api/messaging/extension_message_port.cc
index aba30480fc5a704aa6b5e8512e73c5d42844f1c4..ccfddb73aeecf8d195a95af71bd8e80d1f6193db 100644
--- a/chrome/browser/extensions/api/messaging/extension_message_port.cc
+++ b/chrome/browser/extensions/api/messaging/extension_message_port.cc
@@ -6,6 +6,7 @@
 
 #include "base/scoped_observer.h"
 #include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -27,7 +28,7 @@ class ExtensionMessagePort::FrameTracker : public content::WebContentsObserver,
                                            public ProcessManagerObserver {
  public:
   explicit FrameTracker(ExtensionMessagePort* port)
-      : pm_observer_(this), port_(port) {}
+      : pm_observer_(this), port_(port), interstitial_frame_(nullptr) {}
   ~FrameTracker() override {}
 
   void TrackExtensionProcessFrames() {
@@ -38,6 +39,17 @@ class ExtensionMessagePort::FrameTracker : public content::WebContentsObserver,
     Observe(tab);
   }
 
+  void TrackInterstitialFrame(content::WebContents* tab,
+                              content::RenderFrameHost* interstitial_frame) {
+    // |tab| should never be nullptr, because an interstitial's lifetime is
+    // tied to a tab. This is a CHECK, not a DCHECK because we really need an
+    // observer subject to detect frame removal (via DidDetachInterstitialPage).
+    CHECK(tab);
+    DCHECK(interstitial_frame);
+    interstitial_frame_ = interstitial_frame;
+    Observe(tab);
+  }
+
  private:
   // content::WebContentsObserver overrides:
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host)
@@ -45,18 +57,17 @@ class ExtensionMessagePort::FrameTracker : public content::WebContentsObserver,
     port_->UnregisterFrame(render_frame_host);
   }
 
-  // TODO(robwu): This should be superfluous with RenderFrameDeleted above, but
-  // we are not entirely sure.
-  void FrameDeleted(content::RenderFrameHost* render_frame_host) override {
-    port_->UnregisterFrame(render_frame_host);
-  }
-
   void DidNavigateAnyFrame(content::RenderFrameHost* render_frame_host,
                            const content::LoadCommittedDetails& details,
                            const content::FrameNavigateParams&) override {
     if (!details.is_in_page)
       port_->UnregisterFrame(render_frame_host);
   }
+  
+  void DidDetachInterstitialPage() override {
+    if (interstitial_frame_)
+      port_->UnregisterFrame(interstitial_frame_);
+  }
 
   // extensions::ProcessManagerObserver overrides:
   void OnExtensionFrameUnregistered(
@@ -69,6 +80,11 @@ class ExtensionMessagePort::FrameTracker : public content::WebContentsObserver,
   ScopedObserver<ProcessManager, ProcessManagerObserver> pm_observer_;
   ExtensionMessagePort* port_;  // Owns this FrameTracker.
 
+  // Set to the main frame of an interstitial if we are tracking an interstitial
+  // page, because RenderFrameDeleted is never triggered for frames in an
+  // interstitial (and we only support tracking the interstitial's main frame).
+  content::RenderFrameHost* interstitial_frame_;
+
   DISALLOW_COPY_AND_ASSIGN(FrameTracker);
 };
 
@@ -108,7 +124,28 @@ ExtensionMessagePort::ExtensionMessagePort(
       background_host_ptr_(nullptr),
       frame_tracker_(new FrameTracker(this)) {
   content::WebContents* tab = content::WebContents::FromRenderFrameHost(rfh);
-  CHECK(tab);
+  content::InterstitialPage* interstitial = nullptr;

[Charlie Reis, 2016/01/26 23:58:27]

nit: Move declaration inside the if block, since it's not used outside.

[robwu, 2016/01/27 23:30:06]

Done.

+  if (!tab) {
+    interstitial = content::InterstitialPage::FromRenderFrameHost(rfh);
+    // A RenderFrameHost must be hosted in a WebContents or InterstitialPage.
+    CHECK(interstitial);
+
+    // Only the main frame of an interstitial is supported, because frames in
+    // the interstitial do not trigger RenderFrameCreated / RenderFrameDeleted
+    // on WebContentObservers. Consequently, (1) we cannot detect removal of
+    // RenderFrameHosts, and (2) even if the RenderFrameDeleted is propagated,
+    // then WebContentsObserverSanityChecker triggers a CHECK when ut detects

[Charlie Reis, 2016/01/26 23:58:27]

nit: s/ut/it/

[robwu, 2016/01/27 23:30:06]

Done.

+    // frame notifications without a corresponding RenderFrameCreated.
+    if (!rfh->GetParent()) {
+      // It is safe to pass the interstitial's WebContents here because we use

[Charlie Reis, 2016/01/26 23:58:27]

nit: because we only use it to

[robwu, 2016/01/27 23:30:06]

Done.

+      // it to observe DidDetachInterstitialPage.
+      frame_tracker_->TrackInterstitialFrame(interstitial->GetWebContents(),
+                                             rfh);
+      RegisterFrame(rfh);
+    }
+    return;
+  }
+
   frame_tracker_->TrackTabFrames(tab);
   if (include_child_frames) {
     tab->ForEachFrame(base::Bind(&ExtensionMessagePort::RegisterFrame,