Track all extension frames in ProcessManager, inspect extensionoptions

extensions/browser/extension_web_contents_observer.cc

 // Copyright 2014 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 "extensions/browser/extension_web_contents_observer.h"
 
 #include "content/public/browser/child_process_security_policy.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"
@@ skipping 92 matching lines @@
     content::RenderFrameHost* render_frame_host) {
   InitializeRenderFrame(render_frame_host);
 }
 
 void ExtensionWebContentsObserver::RenderFrameDeleted(
     content::RenderFrameHost* render_frame_host) {
   ProcessManager::Get(browser_context_)
       ->UnregisterRenderFrameHost(render_frame_host);
 }
 
+void ExtensionWebContentsObserver::RenderFrameHostChanged(

[Devlin, 2015/10/30 01:21:44]

We used to have this, and it was removed for some reason.  Unfortunately, I
think that reason is documented in since-removed chat logs.  So + a few content
gurus to double check this.

Basically, what we want is an up-to-date set of render frames so that we can
initialize them with extension-y data.

+    content::RenderFrameHost* old_host,
+    content::RenderFrameHost* new_host) {
+  if (new_host->IsRenderFrameLive())
+    InitializeRenderFrame(new_host);

[ncarter (slow), 2015/10/30 06:22:20]

Does your test really fail if you don't add this override? RenderFrameCreated
and RenderFrameDeleted, together with ForEachFrame() when you install the
observer**, ought to cover cover your use case here.

If the RenderFrameHostChanged override is really necessary here, then that's a
content bug, a pretty bad one, and I am very very eager to learn more.

As written, I would expect this code to end up sending
ExtensionMsg_NotifyRenderViewType and calling RegisterMojoServices()
redundantly. As a rule of thumb, if you don't do anything with |old_host|, then
you probably shouldn't override RenderFrameHostChanged.

** There is one theoretical bug I know of, which is that ForEachFrame doesn't
tell you about existing live pending RenderFrameHosts. I've not seen any case
yet where this matters, and I've been on the lookout for one. It's not been a
problem in practice, apparently, because most WebContentsObservers in practice
install themselves right at the time of WebContents creation, before there's
been a chance to have a pending navigation.

*** I know these observer methods are really confusing. I'm going to try to do
something about it this quarter.

[robwu, 2015/10/30 10:15:49]

This override is needed, because the RFH's GetLastCommittedURL() is used by
GetExtensionFromFrame to determine whether the frame should be registered with
the process manager. When RenderFrameCreated is called, the committed URL is
still the one of the previous page. When I add some more logging
(http://pastebin.com/bUeJwYtM), I get the a log (http://pastebin.com/AFrkPGgi)
that shows the issue. If I remove the RenderFrameHostChanged override, then the
frame will never be registered with the ProcessManager.

If your concern is the duplicate IPC, I can add a conditional around 
ExtensionMsg_NotifyRenderViewType, so that it is only sent once (in the
constructor and in RenderFrameCreated).

[ncarter (slow), 2015/10/30 15:53:44]

I see. I'll take another look, then.

+}
+
 bool ExtensionWebContentsObserver::OnMessageReceived(
     const IPC::Message& message,
     content::RenderFrameHost* render_frame_host) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(
       ExtensionWebContentsObserver, message, render_frame_host)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_Request, OnRequest)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ skipping 80 matching lines @@
   // Since this is called for all existing RenderFrameHosts during the
   // ExtensionWebContentsObserver's creation, it's possible that not all hosts
   // are ready.
   // We only initialize the frame if the renderer counterpart is live; otherwise
   // we wait for the RenderFrameCreated notification.
   if (render_frame_host->IsRenderFrameLive())
     InitializeRenderFrame(render_frame_host);
 }
 
 }  // namespace extensions