Do not allow GuestViewManager to (re)use an instance ID that was already removed.

chrome/browser/guest_view/guest_view_manager.cc

Index: chrome/browser/guest_view/guest_view_manager.cc
diff --git a/chrome/browser/guest_view/guest_view_manager.cc b/chrome/browser/guest_view/guest_view_manager.cc
index 833123f4be4954d9a877688aa32d7e7293d5daf3..4a0cd04cdfa447782aa42f728fb09c4790ce222f 100644
--- a/chrome/browser/guest_view/guest_view_manager.cc
+++ b/chrome/browser/guest_view/guest_view_manager.cc
@@ -60,8 +60,8 @@ class GuestWebContentsObserver
 };
 
 GuestViewManager::GuestViewManager(content::BrowserContext* context)
-    : current_instance_id_(0),
-      context_(context) {}
+    : current_instance_id_(0), last_instance_id_removed_(0), context_(context) {
+}
 
 GuestViewManager::~GuestViewManager() {}
 
@@ -188,13 +188,14 @@ bool GuestViewManager::ForEachGuest(WebContents* embedder_web_contents,
   return false;
 }
 
-void GuestViewManager::AddGuest(int guest_instance_id,
+bool GuestViewManager::AddGuest(int guest_instance_id,
                                 WebContents* guest_web_contents) {
-  DCHECK(guest_web_contents_by_instance_id_.find(guest_instance_id) ==
-         guest_web_contents_by_instance_id_.end());
+  CHECK(guest_web_contents_by_instance_id_.find(guest_instance_id) ==
+        guest_web_contents_by_instance_id_.end());
   guest_web_contents_by_instance_id_[guest_instance_id] = guest_web_contents;
   // This will add the RenderProcessHost ID when we get one.
   new GuestWebContentsObserver(guest_web_contents);
+  return true;
 }
 
 void GuestViewManager::RemoveGuest(int guest_instance_id) {
@@ -204,6 +205,26 @@ void GuestViewManager::RemoveGuest(int guest_instance_id) {
   render_process_host_id_multiset_.erase(
       it->second->GetRenderProcessHost()->GetID());
   guest_web_contents_by_instance_id_.erase(it);
+
+  // All the instance IDs that lie within [0, last_instance_id_removed_]
+  // are invalid.
+  // The remaining sparse invalid ids are kept in |removed_instance_ids_| set.
+  // Following code compacts the set by incrementing

[Fady Samuel, 2014/05/22 17:41:53]

The following code...

[lazyboy, 2014/05/22 18:34:45]

Done.

+  // |last_instance_id_removed_|.
+  if (guest_instance_id == last_instance_id_removed_ + 1) {
+    ++last_instance_id_removed_;
+    // Compact.
+    std::set<int>::iterator iter = removed_instance_ids_.begin();
+    while (iter != removed_instance_ids_.end()) {
+      int instance_id = *iter;
+      if (instance_id != last_instance_id_removed_ + 1)
+        break;
+      ++last_instance_id_removed_;

[Fady Samuel, 2014/05/22 17:41:53]

This line really confuses me. Let's chat about this algorithm.

[lazyboy, 2014/05/22 18:34:45]

Hmmm, seems I uploaded the half of it, the check in AddGuest wasn't uploaded.

Sure let's chat.

+      removed_instance_ids_.erase(iter++);
+    }
+  } else {
+    removed_instance_ids_.insert(guest_instance_id);
+  }
 }
 
 void GuestViewManager::AddRenderProcessHostID(int render_process_host_id) {