Attempt at fixing crash in tab code. It appears that we end up in a

situation where the opener ends up the same as the tab being
closed. The only way I could see this happening is a new tab getting
the same address as a tab that was deleted. This seems unlikely, but
I've added the code to make sure we clean up properly when a tab is
deleted. I'm also adding a couple more checks.

BUG=34135
TEST=none

Review URL: http://codereview.chromium.org/6346008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71800 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 21 insertions, 2 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 5e877c1..d6e5bc1 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -141,6 +141,8 @@ void TabStripModel::InsertTabContentsAt(int index,
     }
     // Anything opened by a link we deem to have an opener.
     data->SetGroup(&selected_contents->controller());
+    // TODO(sky): nuke when we figure out what is causing 34135.
+    CHECK(data->opener != &(contents->controller()));
   } else if ((add_types & ADD_INHERIT_OPENER) && selected_contents) {
     if (foreground) {
       // Forget any existing relationships, we don't want to make things too
@@ -148,6 +150,8 @@ void TabStripModel::InsertTabContentsAt(int index,
       ForgetAllOpeners();
     }
     data->opener = &selected_contents->controller();
+    // TODO(sky): nuke when we figure out what is causing 34135.
+    CHECK(data->opener != &(contents->controller()));
   }
 
   contents_data_.insert(contents_data_.begin() + index, data);
@@ -168,11 +172,11 @@ void TabStripModel::InsertTabContentsAt(int index,
 TabContentsWrapper* TabStripModel::ReplaceTabContentsAt(
     int index,
     TabContentsWrapper* new_contents) {
-  // TODO: this should reset group/opener of any tabs that point at
-  // old_contents.
   DCHECK(ContainsIndex(index));
   TabContentsWrapper* old_contents = GetContentsAt(index);
 
+  ForgetOpenersAndGroupsReferencing(&(old_contents->controller()));
+
   contents_data_[index]->contents = new_contents;
 
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
@@ -217,6 +221,7 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
   volatile TabContentsData old_data = *contents_data_.at(index);
   delete contents_data_.at(index);
   contents_data_.erase(contents_data_.begin() + index);
+  ForgetOpenersAndGroupsReferencing(&(removed_contents->controller()));
   if (empty())
     closing_all_ = true;
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
@@ -1016,3 +1021,14 @@ bool TabStripModel::OpenerMatches(const TabContentsData* data,
                                   bool use_group) {
   return data->opener == opener || (use_group && data->group == opener);
 }
+
+void TabStripModel::ForgetOpenersAndGroupsReferencing(
+    const NavigationController* tab) {
+  for (TabContentsDataVector::const_iterator i = contents_data_.begin();
+       i != contents_data_.end(); ++i) {
+    if ((*i)->group == tab)
+      (*i)->group = NULL;
+    if ((*i)->opener == tab)
+      (*i)->opener = NULL;
+  }
+}
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 7a3c8d9..7ed42f1 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -480,6 +480,9 @@ class TabStripModel : public NotificationObserver {
                             const NavigationController* opener,
                             bool use_group);
 
+  // Sets the group/opener of any tabs that reference |tab| to NULL.
+  void ForgetOpenersAndGroupsReferencing(const NavigationController* tab);
+
   // Our delegate.
   TabStripModelDelegate* delegate_;