Remove EPM:all_hosts_ and use all_extension_views_ instead.

chrome/browser/renderer_host/chrome_render_view_host_observer.cc

 // Copyright (c) 2012 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 "chrome/browser/renderer_host/chrome_render_view_host_observer.h"
 
 #include "base/command_line.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_view_host_delegate.h"
 #include "content/public/browser/site_instance.h"
+#include "content/public/browser/web_contents.h"
+#include "content/browser/web_contents/web_contents_impl.h"

[benwells, 2012/04/19 11:34:02]

This causes a build error due to an illegal dependency.

 
 using content::ChildProcessSecurityPolicy;
 using content::RenderViewHost;
 using content::SiteInstance;
 
 ChromeRenderViewHostObserver::ChromeRenderViewHostObserver(
     RenderViewHost* render_view_host, chrome_browser_net::Predictor* predictor)
     : content::RenderViewHostObserver(render_view_host),
       predictor_(predictor) {
   SiteInstance* site_instance = render_view_host->GetSiteInstance();
@@ skipping 46 matching lines @@
 void ChromeRenderViewHostObserver::InitRenderViewHostForExtensions() {
   const Extension* extension = GetExtension();
   if (!extension)
     return;
 
   ExtensionProcessManager* process_manager =
       profile_->GetExtensionProcessManager();
   CHECK(process_manager);
 
   // TODO(creis): Use this to replace SetInstalledAppForRenderer.
-  process_manager->RegisterRenderViewHost(render_view_host(), extension);
+  content::WebContents* contents = GetWebContents(render_view_host());
+  if (contents)
+    process_manager->RegisterWebContents(contents, extension);
 
   if (extension->is_app()) {
     // Record which, if any, installed app is associated with this process.
     // TODO(aa): Totally lame to store this state in a global map in extension
     // service. Can we get it from EPM instead?
     profile_->GetExtensionService()->SetInstalledAppForRenderer(
         render_view_host()->GetProcess()->GetID(), extension);
   }
 }
 
@@ skipping 63 matching lines @@
   if (service->GetTerminatedExtension(site.host()))
     service->ReloadExtension(site.host());
 
   // May be null if the extension doesn't exist, for example if somebody typos
   // a chrome-extension:// URL.
   return service->extensions()->GetByID(site.host());
 }
 
 void ChromeRenderViewHostObserver::RemoveRenderViewHostForExtensions(
     RenderViewHost* rvh) {
+  content::WebContents* contents = GetWebContents(rvh);
+  if (!contents)
+    return;
+
   ExtensionProcessManager* process_manager =
       profile_->GetExtensionProcessManager();
   if (process_manager)
-    process_manager->UnregisterRenderViewHost(rvh);
+    process_manager->UnregisterWebContents(contents);

[Charlie Reis, 2012/04/19 21:42:10]

I don't think this will get called if you navigate away from an extension while
there are other things keeping the extension process alive.  The RenderViewHost
will just be "swapped out" and the EPM will think this WebContents is showing an
extension when it is not.

+}
+
+content::WebContents* ChromeRenderViewHostObserver::GetWebContents(
+    RenderViewHost* rvh) {
+  // TODO(benwells): Remove this hack. Instead registration should
+  // be made by the view owning the WebContents (e.g. ShellWindow),
+  // or we go back to registering RenderViewHosts and expose their
+  // is_loading field.
+  content::ViewType view_type = rvh->GetDelegate()->GetRenderViewType();
+  if (view_type == content::VIEW_TYPE_WEB_CONTENTS)
+    return static_cast<WebContentsImpl*>(rvh->GetDelegate());

[benwells, 2012/04/19 11:34:02]

How should I do this correctly?

If I use reinterpret_cast  to cast to WebContents it doesn't work, which I
*think* is because the multiple inheritance in the WebContentsImpl making the
WebContents and WebContentsImpl pointers not equal.

As it is it doesn't work either, as evidenced by the badness in the try runs.

[Matt Perry, 2012/04/19 20:53:19]

You're having trouble here because you're trying to get around layering
constraints. Since you're changing EPM from registering RenderViewHosts to
registering WebContents, you should move the registration to when the
WebContents is created. There's already a NOTIFICATION_WEB_CONTENTS_DESTROYED,
but it looks like you have to add one for _CREATED. Then have EPM listen to
those, and do registration there.

+  return NULL;
 }
 
 void ChromeRenderViewHostObserver::OnFocusedEditableNodeTouched() {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_FOCUSED_EDITABLE_NODE_TOUCHED,
       content::Source<RenderViewHost>(render_view_host()),
       content::NotificationService::NoDetails());
 }