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..6d0366deb28e4496b85998fbb5e19f4bf3ba1daa 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,13 @@ class ExtensionMessagePort::FrameTracker : public content::WebContentsObserver,
     Observe(tab);
   }
 
+  void TrackInterstitialFrame(content::WebContents* tab,
+                              content::RenderFrameHost* interstitial_frame) {
+    DCHECK(interstitial_frame);
+    interstitial_frame_ = interstitial_frame;
+    Observe(tab);
+  }
+
  private:
   // content::WebContentsObserver overrides:
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host)
@@ -45,18 +53,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 +76,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 +120,33 @@ ExtensionMessagePort::ExtensionMessagePort(
       background_host_ptr_(nullptr),
       frame_tracker_(new FrameTracker(this)) {
   content::WebContents* tab = content::WebContents::FromRenderFrameHost(rfh);
-  CHECK(tab);
+  content::InterstitialPage* interstitial = nullptr;
+  if (!tab) {
+    interstitial = content::InterstitialPage::FromRenderFrameHost(rfh);
+    // A RenderFrameHost must be hosted in a WebContents or InterstitialPage.
+    CHECK(interstitial);
+
+    tab = interstitial->GetUnderlyingWebContents();
+
+    // |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);
+
+    if (!rfh->GetParent()) {
+      // Only the main frame of an interstitial is supported, because frames in

[Charlie Reis, 2016/01/26 01:14:30]

nit: Please move this comment above line 136, since it's about the conditional
and not the TrackInterstitialFrame call.

[robwu, 2016/01/26 11:29:32]

Done.

+      // the interstitial do not trigger RenderFrameCreated / RenderFrameDeleted
+      // on WebContentObservers. Consequently, (1) we cannot detect removal of
+      // RenderFrameHosts, and (2) even if we propagate the RenderFrameDeleted
+      // notification, then WebContentsObserverSanityChecker triggers a CHECK
+      // when it detects frame notifications without a corresponding
+      // RenderFrameCreated.
+      frame_tracker_->TrackInterstitialFrame(tab, rfh);

[Charlie Reis, 2016/01/26 01:14:30]

Please add a comment above this line:
It is safe to pass the interstitial's WebContents here because we only use it to
observe DidDetachInterstitialPage.

[robwu, 2016/01/26 11:29:32]

Done.

+      RegisterFrame(rfh);
+    }
+    return;
+  }
+
   frame_tracker_->TrackTabFrames(tab);
   if (include_child_frames) {
     tab->ForEachFrame(base::Bind(&ExtensionMessagePort::RegisterFrame,