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

extensions/browser/guest_view/guest_view_base.cc

Index: extensions/browser/guest_view/guest_view_base.cc
diff --git a/extensions/browser/guest_view/guest_view_base.cc b/extensions/browser/guest_view/guest_view_base.cc
index 151cefbbfd75d5de2ad5cb4040d4acba5004cfe2..9ac6225344ca91cdd5e60c668def7d485804e36d 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -57,16 +57,15 @@ scoped_ptr<base::DictionaryValue> GuestViewBase::Event::GetArguments() {
 
 // 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 {
@@ -98,7 +97,7 @@ class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
     guest_->Destroy();
   }
 
-  DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
+  DISALLOW_COPY_AND_ASSIGN(EmbedderLifetimeObserver);
 };
 
 GuestViewBase::GuestViewBase(content::BrowserContext* browser_context,
@@ -159,21 +158,29 @@ void GuestViewBase::Init(const std::string& embedder_extension_id,
                     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);
@@ -278,23 +285,6 @@ 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();
@@ -324,11 +314,6 @@ void GuestViewBase::GuestSizeChanged(const gfx::Size& old_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();
 
@@ -370,13 +355,14 @@ void GuestViewBase::RegisterDestructionCallback(
 
 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();
@@ -488,7 +474,7 @@ void GuestViewBase::SendQueuedEvents() {
 }
 
 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) {
@@ -498,9 +484,8 @@ void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
     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);
 }