Remove the RenderProcessHost observer and attach the WebContentsObserver earlier to the GuestView.

extensions/browser/guest_view/guest_view_base.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/guest_view/guest_view_base.h"
 
 #include "base/lazy_instance.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ skipping 39 matching lines @@
 
 GuestViewBase::Event::~Event() {
 }
 
 scoped_ptr<base::DictionaryValue> GuestViewBase::Event::GetArguments() {
   return args_.Pass();
 }
 
 // This observer ensures that the GuestViewBase destroys itself when its
 // embedder goes away.
-class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
+class GuestViewBase::EmbedderLifetimeObserver : public WebContentsObserver {
  public:
-  explicit EmbedderWebContentsObserver(GuestViewBase* guest)
-      : WebContentsObserver(guest->embedder_web_contents()),
+  EmbedderLifetimeObserver(GuestViewBase* guest,
+                           content::WebContents* embedder_web_contents)
+      : WebContentsObserver(embedder_web_contents),
         destroyed_(false),
-        guest_(guest) {
-  }
+        guest_(guest) {}
 
-  virtual ~EmbedderWebContentsObserver() {
-  }
+  virtual ~EmbedderLifetimeObserver() {}
 
   // WebContentsObserver implementation.
   virtual void WebContentsDestroyed() override {
     // If the embedder is destroyed then destroy the guest.
     Destroy();
   }
 
   virtual void AboutToNavigateRenderView(
       content::RenderViewHost* render_view_host) override {
     // If the embedder navigates then destroy the guest.
@@ skipping 11 matching lines @@
 
   void Destroy() {
     if (destroyed_)
       return;
     destroyed_ = true;
     guest_->embedder_web_contents_ = NULL;
     guest_->EmbedderDestroyed();
     guest_->Destroy();
   }
 
-  DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
+  DISALLOW_COPY_AND_ASSIGN(EmbedderLifetimeObserver);
 };
 
 GuestViewBase::GuestViewBase(content::BrowserContext* browser_context,
                              int guest_instance_id)
     : embedder_web_contents_(NULL),
       embedder_render_process_id_(0),
       browser_context_(browser_context),
       guest_instance_id_(guest_instance_id),
       view_instance_id_(guestview::kInstanceIDNone),
       element_instance_id_(guestview::kInstanceIDNone),
@@ skipping 40 matching lines @@
     return;
   }
 
   CreateWebContents(embedder_extension_id,
                     embedder_process_id,
                     embedder_site_url,
                     create_params,
                     base::Bind(&GuestViewBase::CompleteInit,
                                weak_ptr_factory_.GetWeakPtr(),
                                embedder_extension_id,
-                               embedder_process_id,
+                               embedder_web_contents,
                                callback));
 }
 
 void GuestViewBase::InitWithWebContents(
     const std::string& embedder_extension_id,
-    int embedder_render_process_id,
+    content::WebContents* embedder_web_contents,
     content::WebContents* guest_web_contents) {
   DCHECK(guest_web_contents);
+  DCHECK(embedder_web_contents);
+  int embedder_render_process_id =
+      embedder_web_contents->GetRenderProcessHost()->GetID();
   content::RenderProcessHost* embedder_render_process_host =
       content::RenderProcessHost::FromID(embedder_render_process_id);
 
   embedder_extension_id_ = embedder_extension_id;
   embedder_render_process_id_ = embedder_render_process_host->GetID();
-  embedder_render_process_host->AddObserver(this);
+
+  // At this point, we have just created the guest WebContents, we need to add
+  // an observer to the embedder WebContents. This observer will be responsible
+  // for destroying the guest WebContents if the embedder goes away.
+  embedder_web_contents_observer_.reset(
+      new EmbedderLifetimeObserver(this, embedder_web_contents));
 
   WebContentsObserver::Observe(guest_web_contents);
   guest_web_contents->SetDelegate(this);
   webcontents_guestview_map.Get().insert(
       std::make_pair(guest_web_contents, this));
   GuestViewManager::FromBrowserContext(browser_context_)->
       AddGuest(guest_instance_id_, guest_web_contents);
 
   // Give the derived class an opportunity to perform additional initialization.
   DidInitialize();
@@ skipping 84 matching lines @@
 }
 
 bool GuestViewBase::IsAutoSizeSupported() const {
   return false;
 }
 
 bool GuestViewBase::IsDragAndDropEnabled() const {
   return false;
 }
 
-void GuestViewBase::RenderProcessExited(content::RenderProcessHost* host,
-                                        base::ProcessHandle handle,
-                                        base::TerminationStatus status,
-                                        int exit_code) {
-  // GuestViewBase tracks the lifetime of its embedder render process until it
-  // is attached to a particular embedder WebContents. At that point, its
-  // lifetime is restricted in scope to the lifetime of its embedder
-  // WebContents.
-  CHECK(!attached());
-  CHECK_EQ(host->GetID(), embedder_render_process_id());
-
-  // This code path may be reached if the embedder WebContents is killed for
-  // whatever reason immediately after a called to GuestViewInternal.createGuest
-  // and before attaching the new guest to a frame.
-  Destroy();
-}
-
 void GuestViewBase::DidAttach(int guest_proxy_routing_id) {
   // Give the derived class an opportunity to perform some actions.
   DidAttachToEmbedder();
 
   // Inform the associated GuestViewContainer that the contentWindow is ready.
   embedder_web_contents()->Send(new ExtensionMsg_GuestAttached(
       embedder_web_contents()->GetMainFrame()->GetRoutingID(),
       element_instance_id_,
       guest_proxy_routing_id));
 
   SendQueuedEvents();
 }
 
 void GuestViewBase::ElementSizeChanged(const gfx::Size& old_size,
                                        const gfx::Size& new_size) {
   element_size_ = new_size;
 }
 
 void GuestViewBase::GuestSizeChanged(const gfx::Size& old_size,
                                      const gfx::Size& new_size) {
   if (!auto_size_enabled_)
     return;
   guest_size_ = new_size;
   GuestSizeChangedDueToAutoSize(old_size, new_size);
 }
 
 void GuestViewBase::Destroy() {
   DCHECK(web_contents());
 
-  content::RenderProcessHost* host =
-      content::RenderProcessHost::FromID(embedder_render_process_id());
-  if (host)
-    host->RemoveObserver(this);
-
   // Give the derived class an opportunity to perform some cleanup.
   WillDestroy();
 
   // Invalidate weak pointers now so that bound callbacks cannot be called late
   // into destruction. We must call this after WillDestroy because derived types
   // may wish to access their openers.
   weak_ptr_factory_.InvalidateWeakPtrs();
 
   // Give the content module an opportunity to perform some cleanup.
   if (!destruction_callback_.is_null())
@@ skipping 21 matching lines @@
   opener_ = base::WeakPtr<GuestViewBase>();
 }
 
 void GuestViewBase::RegisterDestructionCallback(
     const DestructionCallback& callback) {
   destruction_callback_ = callback;
 }
 
 void GuestViewBase::WillAttach(content::WebContents* embedder_web_contents,
                                int element_instance_id) {
-  // After attachment, this GuestViewBase's lifetime is restricted to the
-  // lifetime of its embedder WebContents. Observing the RenderProcessHost
-  // of the embedder is no longer necessary.
-  embedder_web_contents->GetRenderProcessHost()->RemoveObserver(this);
   embedder_web_contents_ = embedder_web_contents;
-  embedder_web_contents_observer_.reset(
-      new EmbedderWebContentsObserver(this));
+
+  // If we are attaching to a different WebContents than the one that created
+  // the guest, we need to create a new LifetimeObserver.
+  if (embedder_web_contents != embedder_web_contents_observer_->web_contents())
+    embedder_web_contents_observer_.reset(

[Fady Samuel, 2014/10/09 15:58:49]

nit: since this statement occupies multiple lines, please place this in a block
to avoid future confusion.

[lfg, 2014/10/09 17:56:02]

Done.

+        new EmbedderLifetimeObserver(this, embedder_web_contents));
+
   element_instance_id_ = element_instance_id;
 
   WillAttachToEmbedder();
 }
 
 void GuestViewBase::DidStopLoading(content::RenderViewHost* render_view_host) {
   if (!IsDragAndDropEnabled()) {
     const char script[] = "window.addEventListener('dragstart', function() { "
                           "  window.event.preventDefault(); "
                           "});";
@@ skipping 91 matching lines @@
   if (!attached())
     return;
   while (!pending_events_.empty()) {
     linked_ptr<Event> event_ptr = pending_events_.front();
     pending_events_.pop_front();
     DispatchEventToEmbedder(event_ptr.release());
   }
 }
 
 void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
-                                 int embedder_render_process_id,
+                                 content::WebContents* embedder_web_contents,
                                  const WebContentsCreatedCallback& callback,
                                  content::WebContents* guest_web_contents) {
   if (!guest_web_contents) {
     // The derived class did not create a WebContents so this class serves no
     // purpose. Let's self-destruct.
     delete this;
     callback.Run(NULL);
     return;
   }
-  InitWithWebContents(embedder_extension_id,
-                      embedder_render_process_id,
-                      guest_web_contents);
+  InitWithWebContents(
+      embedder_extension_id, embedder_web_contents, guest_web_contents);
   callback.Run(guest_web_contents);
 }
 
 // static
 void GuestViewBase::RegisterGuestViewTypes() {
   AppViewGuest::Register();
   ExtensionOptionsGuest::Register();
   MimeHandlerViewGuest::Register();
   WebViewGuest::Register();
 }
 
 }  // namespace extensions