This patch improves the way that GuestViewManager tracks the destruction of GuestView embedders.

components/guest_view/browser/guest_view_manager.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 "components/guest_view/browser/guest_view_manager.h"
 
 #include "base/macros.h"
 #include "base/strings/stringprintf.h"
 #include "components/guest_view/browser/guest_view_base.h"
 #include "components/guest_view/browser/guest_view_manager_delegate.h"
 #include "components/guest_view/browser/guest_view_manager_factory.h"
 #include "components/guest_view/common/guest_view_constants.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "url/gurl.h"
 
 using content::BrowserContext;
+using content::RenderProcessHost;
 using content::SiteInstance;
 using content::WebContents;
 
 namespace guest_view {
 
+// This observer observes the RenderProcessHosts of GuestView embedders, and
+// notifies the GuestViewManager when they are destroyed.
+class GuestViewManager::EmbedderRenderProcessHostObserver
+    : public content::RenderProcessHostObserver {
+ public:
+  EmbedderRenderProcessHostObserver(
+      base::WeakPtr<GuestViewManager> guest_view_manager,
+      int embedder_process_id)
+      : guest_view_manager_(guest_view_manager), id_(embedder_process_id) {
+    RenderProcessHost* rph = RenderProcessHost::FromID(id_);
+    rph->AddObserver(this);
+  }
+
+  ~EmbedderRenderProcessHostObserver() override {
+    RenderProcessHost* rph = RenderProcessHost::FromID(id_);
+    if (rph)
+      rph->RemoveObserver(this);
+  }
+
+  void RenderProcessHostDestroyed(RenderProcessHost* host) override {
+    if (guest_view_manager_.get())
+      guest_view_manager_->EmbedderProcessDestroyed(id_);
+    delete this;
+  }
+
+ private:
+  base::WeakPtr<GuestViewManager> guest_view_manager_;
+  int id_;
+};
+
 // static
 GuestViewManagerFactory* GuestViewManager::factory_ = nullptr;
 
 GuestViewManager::GuestViewManager(
     content::BrowserContext* context,
     scoped_ptr<GuestViewManagerDelegate> delegate)
     : current_instance_id_(0),
       last_instance_id_removed_(0),
       context_(context),
-      delegate_(delegate.Pass()) {
+      delegate_(delegate.Pass()),
+      weak_ptr_factory_(this) {
 }
 
 GuestViewManager::~GuestViewManager() {}
 
 // static
 GuestViewManager* GuestViewManager::CreateWithDelegate(
     BrowserContext* context,
     scoped_ptr<GuestViewManagerDelegate> delegate) {
   GuestViewManager* guest_manager = FromBrowserContext(context);
   if (!guest_manager) {
@@ skipping 188 matching lines @@
       if (instance_id != last_instance_id_removed_ + 1)
         break;
       ++last_instance_id_removed_;
       removed_instance_ids_.erase(iter++);
     }
   } else {
     removed_instance_ids_.insert(guest_instance_id);
   }
 }
 
-void GuestViewManager::EmbedderWillBeDestroyed(int embedder_process_id) {
-  // Find and call any callbacks associated with the embedder that is being
-  // destroyed.
-  auto embedder_it = view_destruction_callback_map_.find(embedder_process_id);
-  if (embedder_it == view_destruction_callback_map_.end())
-    return;
-  CallbacksForEachViewID& callbacks_for_embedder = embedder_it->second;
-  for (auto& view_pair : callbacks_for_embedder) {
-    Callbacks& callbacks_for_view = view_pair.second;
-    for (auto& callback : callbacks_for_view) {
-      callback.Run();
-    }
-  }
-  view_destruction_callback_map_.erase(embedder_it);
+void GuestViewManager::EmbedderProcessDestroyed(int embedder_process_id) {
+  embedders_observed_.erase(embedder_process_id);
+  CallViewDestructionCallbacks(embedder_process_id);
 }
 
 void GuestViewManager::ViewCreated(int embedder_process_id,
                                    int view_instance_id,
                                    const std::string& view_type) {
   if (guest_view_registry_.empty())
     RegisterGuestViewTypes();
   auto view_it = guest_view_registry_.find(view_type);
   CHECK(view_it != guest_view_registry_.end())
       << "Invalid GuestView created of type \"" << view_type << "\"";
 
   // Register the cleanup callback for when this view is destroyed.
   RegisterViewDestructionCallback(embedder_process_id,
                                   view_instance_id,
                                   base::Bind(view_it->second.cleanup_function,
+                                             context_,

[Fady Samuel, 2015/07/14 17:56:59]

Why does the cleanup function need the browser context?

[paulmeyer, 2015/07/14 18:04:07]

It always needed it to clean up some of the browser-side state. It used to rely
on getting it via the RenderProcessHost, but I don't think that is actually
always safe since cleanup can be triggered by the destruction of the
RenderProcessHost. So, I'm now just passing it in instead.

                                              embedder_process_id,
                                              view_instance_id));
 }
 
 void GuestViewManager::ViewGarbageCollected(int embedder_process_id,
                                             int view_instance_id) {
-  // Find and call any callbacks associated with the view that has been garbage
-  // collected.
+  CallViewDestructionCallbacks(embedder_process_id, view_instance_id);
+}
+
+void GuestViewManager::CallViewDestructionCallbacks(int embedder_process_id,
+                                                    int view_instance_id) {
+  // Find the callbacks for the embedder with ID |embedder_process_id|.
   auto embedder_it = view_destruction_callback_map_.find(embedder_process_id);
   if (embedder_it == view_destruction_callback_map_.end())
     return;
   CallbacksForEachViewID& callbacks_for_embedder = embedder_it->second;
+
+  // If |view_instance_id| is guest_view::kInstanceIDNone, then all callbacks
+  // for this embedder should be called.
+  if (view_instance_id == guest_view::kInstanceIDNone) {
+    // Call all callbacks for the embedder with ID |embedder_process_id|.
+    for (auto& view_pair : callbacks_for_embedder) {
+      Callbacks& callbacks_for_view = view_pair.second;
+      for (auto& callback : callbacks_for_view)
+        callback.Run();
+    }
+    view_destruction_callback_map_.erase(embedder_it);
+    return;
+  }
+
+  // Otherwise, call the callbacks only for the specific view with ID
+  // |view_instance_id|.
   auto view_it = callbacks_for_embedder.find(view_instance_id);
   if (view_it == callbacks_for_embedder.end())
     return;
   Callbacks& callbacks_for_view = view_it->second;
   for (auto& callback : callbacks_for_view)
     callback.Run();
   callbacks_for_embedder.erase(view_it);
 }
 
+void GuestViewManager::CallViewDestructionCallbacks(int embedder_process_id) {
+  CallViewDestructionCallbacks(embedder_process_id,
+                               guest_view::kInstanceIDNone);
+}
+
 GuestViewBase* GuestViewManager::CreateGuestInternal(
     content::WebContents* owner_web_contents,
     const std::string& view_type) {
   if (guest_view_registry_.empty())
     RegisterGuestViewTypes();
 
   auto it = guest_view_registry_.find(view_type);
   if (it == guest_view_registry_.end()) {
     NOTREACHED();
     return nullptr;
   }
 
   return it->second.create_function.Run(owner_web_contents);
 }
 
 void GuestViewManager::RegisterGuestViewTypes() {
   delegate_->RegisterAdditionalGuestViewTypes();
 }
 
 void GuestViewManager::RegisterViewDestructionCallback(
     int embedder_process_id,
     int view_instance_id,
     const base::Closure& callback) {
+  // When an embedder is registered for the first time, create an observer to
+  // watch for its destruction.
+  if (!embedders_observed_.count(embedder_process_id)) {
+    embedders_observed_.insert(embedder_process_id);
+    new EmbedderRenderProcessHostObserver(weak_ptr_factory_.GetWeakPtr(),
+                                          embedder_process_id);
+  }
+
   view_destruction_callback_map_[embedder_process_id][view_instance_id]
       .push_back(callback);
 }
 
 bool GuestViewManager::IsGuestAvailableToContext(GuestViewBase* guest) {
   return delegate_->IsGuestAvailableToContext(guest);
 }
 
 void GuestViewManager::DispatchEvent(const std::string& event_name,
                                      scoped_ptr<base::DictionaryValue> args,
@@ skipping 101 matching lines @@
 }
 
 GuestViewManager::GuestViewData::GuestViewData(
     const GuestViewCreateFunction& create_function,
     const GuestViewCleanUpFunction& cleanup_function)
     : create_function(create_function), cleanup_function(cleanup_function) {}
 
 GuestViewManager::GuestViewData::~GuestViewData() {}
 
 }  // namespace guest_view