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

chrome/browser/guest_view/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 "chrome/browser/guest_view/guest_view_manager.h"
 
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/guest_view/guest_view_base.h"
 #include "chrome/browser/guest_view/guest_view_constants.h"
@@ skipping 42 matching lines @@
 
   virtual void WebContentsDestroyed() OVERRIDE {
     delete this;
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(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() {}
 
 // static.
 GuestViewManager* GuestViewManager::FromBrowserContext(
     BrowserContext* context) {
   GuestViewManager* guest_manager =
       static_cast<GuestViewManager*>(context->GetUserData(
           guestview::kGuestViewManagerKeyName));
   if (!guest_manager) {
@@ skipping 108 matching lines @@
       continue;
 
     if (callback.Run(guest))
       return true;
   }
   return false;
 }
 
 void 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(!ContainsKey(guest_web_contents_by_instance_id_, guest_instance_id));
+  CHECK(CanUseGuestInstanceID(guest_instance_id));
   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);
 }
 
 void GuestViewManager::RemoveGuest(int guest_instance_id) {
   GuestInstanceMap::iterator it =
       guest_web_contents_by_instance_id_.find(guest_instance_id);
   DCHECK(it != guest_web_contents_by_instance_id_.end());
   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.
+  // The following code compacts the set by incrementing
+  // |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)

[Fady Samuel, 2014/05/22 19:11:00]

Add a DCHECK and comment how instance_id > last_instance_id removed.

[lazyboy, 2014/05/22 21:14:25]

Done.

+        break;
+      ++last_instance_id_removed_;
+      removed_instance_ids_.erase(iter++);
+    }
+  } else {
+    removed_instance_ids_.insert(guest_instance_id);
+  }
 }
 
 void GuestViewManager::AddRenderProcessHostID(int render_process_host_id) {
   render_process_host_id_multiset_.insert(render_process_host_id);
 }
 
 content::WebContents* GuestViewManager::GetGuestByInstanceID(
     int guest_instance_id,
     int embedder_render_process_id) {
   GuestInstanceMap::const_iterator it =
@@ skipping 13 matching lines @@
         base::UserMetricsAction("BadMessageTerminate_BPGM"));
     base::KillProcess(
         content::RenderProcessHost::FromID(embedder_render_process_id)->
             GetHandle(),
         content::RESULT_CODE_KILLED_BAD_MESSAGE, false);
     return false;
   }
   return true;
 }
 
+bool GuestViewManager::CanUseGuestInstanceID(int guest_instance_id) {
+  if (guest_instance_id <= last_instance_id_removed_)
+    return false;
+  return !ContainsKey(removed_instance_ids_, guest_instance_id);
+}
+
 bool GuestViewManager::CanEmbedderAccessInstanceID(
     int embedder_render_process_id,
     int guest_instance_id) {
   // The embedder is trying to access a guest with a negative or zero
   // instance ID.
   if (guest_instance_id <= guestview::kInstanceIDNone)
     return false;
 
   // The embedder is trying to access an instance ID that has not yet been
   // allocated by GuestViewManager. This could cause instance ID
@@ skipping 23 matching lines @@
       return false;
 
     return embedder_render_process_id ==
         guest->GetOpener()->embedder_web_contents()->GetRenderProcessHost()->
             GetID();
   }
 
   return embedder_render_process_id ==
       guest->embedder_web_contents()->GetRenderProcessHost()->GetID();
 }