Makes TabStripModel deal handle the case of the TabStripModel

chrome/browser/ui/tabs/tab_strip_model.cc

Index: chrome/browser/ui/tabs/tab_strip_model.cc
diff --git a/chrome/browser/ui/tabs/tab_strip_model.cc b/chrome/browser/ui/tabs/tab_strip_model.cc
index da4302a9707f818b363d7e0fc77c6ad60449d138..360b5d2f4d321927a4180b785a217dc5b1219896 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model.cc
@@ -45,6 +45,68 @@ bool ShouldForgetOpenersForTransition(content::PageTransition transition) {
       transition == content::PAGE_TRANSITION_AUTO_TOPLEVEL;
 }
 
+// CloseTracker is used when closing a set of WebContents. It listens for
+// deletions of the WebContents and removes from the internal set any time one
+// is deleted.
+class CloseTracker : public content::NotificationObserver {
+ public:
+  typedef std::vector<content::WebContents*> Contents;

[Avi (use Gerrit), 2013/06/03 18:41:35]

Note that we have "using content::WebContents" above. You don't need to qualify
WebContents anywhere in this file.

+
+  explicit CloseTracker(const Contents& contents);
+  virtual ~CloseTracker();
+
+  // Returns true if there is another WebContents in the Tracker.
+  bool HasNext() const;
+
+  // Returns the next WebContents, or NULL if there are no more.
+  content::WebContents* Next();
+
+ private:
+  // NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  Contents contents_;
+
+  content::NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(CloseTracker);
+};
+
+CloseTracker::CloseTracker(const Contents& contents)
+    : contents_(contents) {
+  registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+}
+
+CloseTracker::~CloseTracker() {
+}
+
+bool CloseTracker::HasNext() const {
+  return !contents_.empty();
+}
+
+content::WebContents* CloseTracker::Next() {
+  if (contents_.empty())
+    return NULL;
+
+  content::WebContents* web_contents = contents_[0];
+  contents_.erase(contents_.begin());
+  return web_contents;
+}
+
+void CloseTracker::Observe(int type,
+                           const content::NotificationSource& source,
+                           const content::NotificationDetails& details) {
+  content::WebContents* web_contents =
+      content::Source<WebContents>(source).ptr();
+  Contents::iterator i =
+      std::find(contents_.begin(), contents_.end(), web_contents);
+  if (i != contents_.end())
+    contents_.erase(i);
+}
+
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -960,6 +1022,14 @@ bool TabStripModel::ContextMenuCommandToBrowserCommand(int cmd_id,
 ///////////////////////////////////////////////////////////////////////////////
 // TabStripModel, private:
 
+std::vector<content::WebContents*> TabStripModel::GetWebContentsFromIndicies(
+    const std::vector<int>& indices) const {
+  std::vector<WebContents*> contents;
+  for (size_t i = 0; i < indices.size(); ++i)
+    contents.push_back(GetWebContentsAtImpl(indices[i]));
+  return contents;
+}
+
 void TabStripModel::GetIndicesWithSameDomain(int index,
                                              std::vector<int>* indices) {
   std::string domain = GetWebContentsAt(index)->GetURL().host();
@@ -1014,12 +1084,7 @@ bool TabStripModel::InternalCloseTabs(const std::vector<int>& indices,
   if (indices.empty())
     return true;
 
-  // Map the indices to WebContentses, that way if deleting a tab deletes
-  // other tabs we're ok. Crashes seem to indicate during tab deletion other
-  // tabs are getting removed.
-  std::vector<WebContents*> closing_contentses;
-  for (size_t i = 0; i < indices.size(); ++i)
-    closing_contentses.push_back(GetWebContentsAtImpl(indices[i]));
+  CloseTracker close_tracker(GetWebContentsFromIndicies(indices));
 
   // We only try the fast shutdown path if the whole browser process is *not*
   // shutting down. Fast shutdown during browser termination is handled in
@@ -1044,8 +1109,8 @@ bool TabStripModel::InternalCloseTabs(const std::vector<int>& indices,
 
   // We now return to our regularly scheduled shutdown procedure.
   bool retval = true;
-  for (size_t i = 0; i < closing_contentses.size(); ++i) {
-    WebContents* closing_contents = closing_contentses[i];
+  while (close_tracker.HasNext()) {
+    WebContents* closing_contents = close_tracker.Next();
     int index = GetIndexOfWebContents(closing_contents);
     // Make sure we still contain the tab.
     if (index == kNoTab)