GuestView: Move lifetime management out of content

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 61 matching lines @@
   virtual void WebContentsDestroyed() OVERRIDE {
     Destroy();
   }
 
   virtual void RenderViewHostChanged(
       content::RenderViewHost* old_host,
       content::RenderViewHost* new_host) OVERRIDE {
     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();
   }
 
  private:
   bool destroyed_;
   GuestViewBase* guest_;
 
   void Destroy() {
     if (destroyed_)
@@ skipping 198 matching lines @@
   // 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::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();
 
   // 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);
+  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,
                              &view_instance_id_);
 }
 
 void GuestViewBase::SetOpener(GuestViewBase* guest) {
   if (guest && guest->IsViewType(GetViewType())) {
     opener_ = guest->AsWeakPtr();
     return;
@@ skipping 137 matching lines @@
 
 // static
 void GuestViewBase::RegisterGuestViewTypes() {
   AppViewGuest::Register();
   ExtensionOptionsGuest::Register();
   MimeHandlerViewGuest::Register();
   WebViewGuest::Register();
 }
 
 }  // namespace extensions