From d1f6a9c4f1690ac9322e96b85a7ccb5a471f8b5a Mon Sep 17 00:00:00 2001 From: "sky@chromium.org" Date: Wed, 19 Jan 2011 16:22:46 +0000 Subject: 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 --- chrome/browser/tabs/tab_strip_model.cc | 20 ++++++++++++++++++-- chrome/browser/tabs/tab_strip_model.h | 3 +++ 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_; -- cgit v1.1