GuestView: Move lifetime management out of content

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 9f5a244ffa0f36a2185a864d22e6b7f86746e78c..b29ebbd6655aebb4400ab85482d62be8c70361ed 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -79,6 +79,11 @@ class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
     Destroy();
   }
 
+  virtual void AboutToNavigateRenderView(
+      content::RenderViewHost* render_view_host) OVERRIDE {
+    Destroy();

[lfg, 2014/09/30 18:59:27]

Why do we need this?

Is it safe? This call will destroy the WebContents, which is the object that
called this function in the first place.

[Fady Samuel, 2014/09/30 22:50:03]

Add comments throughout to explain lifetime management.

[lfg, 2014/09/30 23:29:42]

I see. Makes sense now :)

+  }
+
   virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE {
     Destroy();
   }
@@ -297,24 +302,6 @@ void GuestViewBase::RenderProcessExited(content::RenderProcessHost* host,
   Destroy();
 }
 
-void GuestViewBase::Destroy() {
-  DCHECK(web_contents());
-  content::RenderProcessHost* host =
-      content::RenderProcessHost::FromID(embedder_render_process_id());
-  if (host)
-    host->RemoveObserver(this);
-  WillDestroy();
-  if (!destruction_callback_.is_null())
-    destruction_callback_.Run();
-
-  webcontents_guestview_map.Get().erase(web_contents());
-  GuestViewManager::FromBrowserContext(browser_context_)->
-      RemoveGuest(guest_instance_id_);
-  pending_events_.clear();
-
-  delete web_contents();
-}
-
 void GuestViewBase::DidAttach(int guest_proxy_routing_id) {
   // Give the derived class an opportunity to perform some actions.
   DidAttachToEmbedder();
@@ -341,6 +328,24 @@ void GuestViewBase::GuestSizeChanged(const gfx::Size& old_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);
+  WillDestroy();
+  if (!destruction_callback_.is_null())
+    destruction_callback_.Run();
+
+  webcontents_guestview_map.Get().erase(web_contents());
+  GuestViewManager::FromBrowserContext(browser_context_)->
+      RemoveGuest(guest_instance_id_);
+  pending_events_.clear();
+
+  delete web_contents();
+}
+
 void GuestViewBase::SetAttachParams(const base::DictionaryValue& params) {
   attach_params_.reset(params.DeepCopy());
   attach_params_->GetInteger(guestview::kParameterInstanceId,