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 52 matching lines @@
       : WebContentsObserver(guest->embedder_web_contents()),
         destroyed_(false),
         guest_(guest) {
   }
 
   virtual ~EmbedderWebContentsObserver() {
   }
 
   // WebContentsObserver implementation.
   virtual void WebContentsDestroyed() OVERRIDE {
+    // If the embedder is destroyed then destroy the guest.
     Destroy();
   }
 
-  virtual void RenderViewHostChanged(
-      content::RenderViewHost* old_host,
-      content::RenderViewHost* new_host) OVERRIDE {
+  virtual void AboutToNavigateRenderView(
+      content::RenderViewHost* render_view_host) OVERRIDE {
+    // If the embedder navigates then destroy the guest.
     Destroy();
   }
 
   virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE {
+    // If the embedder crashes, then destroy the guest.
     Destroy();
   }
 
  private:
   bool destroyed_;
   GuestViewBase* guest_;
 
   void Destroy() {
     if (destroyed_)
       return;
@@ skipping 197 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);
+
+  // Give the derived class an opportunity to perform some cleanup.
+  WillDestroy();
+
+  // Give the content module an opportunity to perform some cleanup.
+  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 35 matching lines @@
   GuestReady();
   content::RenderViewHost* rvh = web_contents()->GetRenderViewHost();
   if (auto_size_enabled_) {
     rvh->EnableAutoResize(min_auto_size_, max_auto_size_);
   } else {
     rvh->DisableAutoResize(element_size_);
   }
 }
 
 void GuestViewBase::WebContentsDestroyed() {
+  // Let the derived class know that its WebContents is in the process of
+  // being destroyed. web_contents() is still valid at this point.
+  // TODO(fsamuel): This allows for reentrant code into WebContents during
+  // destruction. This could potentially lead to bugs. Perhaps we should get rid
+  // of this?
   GuestDestroyed();
+
+  // Self-destruct.
   delete this;
 }
 
 void GuestViewBase::ActivateContents(WebContents* web_contents) {
   if (!attached() || !embedder_web_contents()->GetDelegate())
     return;
 
   embedder_web_contents()->GetDelegate()->ActivateContents(
       embedder_web_contents());
 }
@@ skipping 81 matching lines @@
 
 // static
 void GuestViewBase::RegisterGuestViewTypes() {
   AppViewGuest::Register();
   ExtensionOptionsGuest::Register();
   MimeHandlerViewGuest::Register();
   WebViewGuest::Register();
 }
 
 }  // namespace extensions