Getting rid of more webview memory leaks.

components/guest_view/browser/guest_view_manager.h

Index: components/guest_view/browser/guest_view_manager.h
diff --git a/components/guest_view/browser/guest_view_manager.h b/components/guest_view/browser/guest_view_manager.h
index 034f7565310a0e025dc52d1927e36c908e820cc4..e85e02bafd9bbfdd67ceba6e4bf4beef0117dade 100644
--- a/components/guest_view/browser/guest_view_manager.h
+++ b/components/guest_view/browser/guest_view_manager.h
@@ -6,6 +6,7 @@
 #define COMPONENTS_GUEST_VIEW_BROWSER_GUEST_VIEW_MANAGER_H_
 
 #include <map>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/gtest_prod_util.h"
@@ -89,9 +90,17 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
     // registered here.
     if (guest_view_registry_.count(T::Type))
       return;
-    guest_view_registry_[T::Type] = base::Bind(&T::Create);
+    auto new_entry = std::make_pair(T::Type, GuestViewData(

[lazyboy, 2015/06/05 23:09:55]

nit: Can we break lines a bit differently?

std::make_pair(
    T::Type,
    GuestViewData(base::Bind(&T::Create),
                  base::Bind(&T::CleanUp)));

It's concise if each param has their own indentation for this kind of
situation, much easier to read.

nit: s/new_entry/registry_entry

[paulmeyer, 2015/06/08 17:53:58]

Done.

+        base::Bind(&T::Create), base::Bind(&T::CleanUp)));
+    guest_view_registry_.insert(new_entry);
   }
 
+  // Registers a callback to be called when the view identified by
+  // |embedder_process_id| and |view_instance_id| is destroyed.
+  void RegisterViewCallback(int embedder_process_id,

[lazyboy, 2015/06/05 23:09:55]

RegisterViewDestructionCallback

[paulmeyer, 2015/06/08 17:53:58]

Done.

+                            int view_instance_id,
+                            const base::Closure& callback);
+
   using WebContentsCreatedCallback =
       base::Callback<void(content::WebContents*)>;
   void CreateGuest(const std::string& view_type,
@@ -121,12 +130,20 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
   friend class GuestViewEvent;
   friend class GuestViewMessageFilter;
 
-  // Can be overriden in tests.
+  // The virtual methods are virtual so that they can be overriden in tests.
+
   virtual void AddGuest(int guest_instance_id,
                         content::WebContents* guest_web_contents);
   virtual void RemoveGuest(int guest_instance_id);
+
+  // Called when a GuestView has been created in JavaScript.
+  virtual void ViewCreated(int embedder_process_id,
+                           int view_instance_id,
+                           const std::string& view_type);
+
+  // Called when a GuestView has been garbage collected in JavaScript.
   virtual void ViewGarbageCollected(int embedder_process_id,
-                                    int view_instance_id) {}
+                                    int view_instance_id);
 
   // Creates a guest of the provided |view_type|.
   GuestViewBase* CreateGuestInternal(content::WebContents* owner_web_contents,
@@ -146,6 +163,10 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
                      GuestViewBase* guest,
                      int instance_id);
 
+  // This method is called when the embedder with ID |embedder_process_id| is
+  // about to be destroyed.
+  void EmbedderWillBeDestroyed(int embedder_process_id);
+
   content::WebContents* GetGuestByInstanceID(int guest_instance_id);
 
   bool CanEmbedderAccessInstanceIDMaybeKill(
@@ -190,11 +211,18 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
   using GuestInstanceIDReverseMap = std::map<int, ElementInstanceKey>;
   GuestInstanceIDReverseMap reverse_instance_id_map_;
 
-  using GuestCreationCallback =
+  using GuestViewCreateFunction =
       base::Callback<GuestViewBase*(content::WebContents*)>;
-  using GuestViewCreationMap =
-      std::map<std::string, GuestViewManager::GuestCreationCallback>;
-  GuestViewCreationMap guest_view_registry_;
+  using GuestViewCleanUpFunction = base::Callback<void(int, int)>;
+  struct GuestViewData {
+    GuestViewData(const GuestViewCreateFunction& create,
+                  const GuestViewCleanUpFunction& clean_up);
+    ~GuestViewData();
+    const GuestViewCreateFunction create;

[lazyboy, 2015/06/05 23:09:55]

s/create/create_function
s/clean_up/cleanup_function

[paulmeyer, 2015/06/08 17:53:59]

Done.

+    const GuestViewCleanUpFunction clean_up;
+  };
+  using GuestViewMethodMap = std::map<std::string, GuestViewData>;
+  GuestViewMethodMap guest_view_registry_;
 
   int current_instance_id_;
 
@@ -211,6 +239,10 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
 
   scoped_ptr<GuestViewManagerDelegate> delegate_;
 
+  // |view_callback_map_| maps from embedder process ID to view ID to a
+  // vector of callback functions to be called when that view is destroyed.
+  std::map<int, std::map<int, std::vector<base::Closure>>> view_callback_map_;

[lazyboy, 2015/06/05 23:09:55]

1)
It's not obvious from the usage of this map why the callback
functions need to be vector and not just one base::Closure.
In which case does that happen? i.e. we get more than one
destruction callback registered for one view?

You should add comment if that's the case.

2)
view_callback_map_ -> view_destruction_callback_map_

3)
Did you check /w Fady if there's a better way that this...
A way to grab all the view instance ids of an embedder,
given a render process id.

Maybe with
GuestManager::ForEachGuest can help? Altho that doesn't give
you <webview>s that doesn't have GuestView created for them.

[paulmeyer, 2015/06/08 17:53:58]

I addressed the first two. For the third, as per our offline conversation, I'll
make some typedefs to make this more readable.

+
   DISALLOW_COPY_AND_ASSIGN(GuestViewManager);
 };