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

components/guest_view/browser/guest_view_manager.cc

Index: components/guest_view/browser/guest_view_manager.cc
diff --git a/components/guest_view/browser/guest_view_manager.cc b/components/guest_view/browser/guest_view_manager.cc
index 07da1adf11f022be2079599f915fbe9e41e7c06c..51555be9c39e0cd1512574f84d908863647dad6e 100644
--- a/components/guest_view/browser/guest_view_manager.cc
+++ b/components/guest_view/browser/guest_view_manager.cc
@@ -1,4 +1,4 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
+i// 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.
 
@@ -22,11 +22,39 @@
 #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(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 {
+    guest_view_manager_->EmbedderProcessDestroyed(id_);
+  }
+
+ private:
+  GuestViewManager* guest_view_manager_;
+  int id_;
+};
+
 // static
 GuestViewManagerFactory* GuestViewManager::factory_ = nullptr;
 
@@ -245,20 +273,8 @@ void GuestViewManager::RemoveGuest(int 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) {
+  CallViewDestructionCallbacks(embedder_process_id);
 }
 
 void GuestViewManager::ViewCreated(int embedder_process_id,
@@ -274,18 +290,39 @@ void GuestViewManager::ViewCreated(int embedder_process_id,
   RegisterViewDestructionCallback(embedder_process_id,
                                   view_instance_id,
                                   base::Bind(view_it->second.cleanup_function,
+                                             context_,
                                              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;
@@ -295,6 +332,11 @@ void GuestViewManager::ViewGarbageCollected(int embedder_process_id,
   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) {
@@ -318,6 +360,11 @@ void GuestViewManager::RegisterViewDestructionCallback(
     int embedder_process_id,
     int view_instance_id,
     const base::Closure& callback) {
+  if (!view_destruction_callback_map_.count(embedder_process_id)) {
+    embedder_render_process_host_observers_.push_back(

[Fady Samuel, 2015/07/10 15:24:01]

When do you remove an observer? I would just have an observer manage its own
lifetime. If you never need to iterate over observers then you don't need a
vector.

[paulmeyer, 2015/07/10 15:34:47]

Done.

+        new EmbedderRenderProcessHostObserver(this, embedder_process_id));
+  }
+
   view_destruction_callback_map_[embedder_process_id][view_instance_id]
       .push_back(callback);
 }