Linux: fix crash in PopulateCookieDetails called by OnSelectionChanged callback

which can be called while the view and model are in inconsistent states.

BUG=25535
TEST=open cookies manager, select all, remove all

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

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

5 files changed, 50 insertions, 0 deletions

diff --git a/chrome/browser/gtk/options/cookies_view.cc b/chrome/browser/gtk/options/cookies_view.cc
index b2b16e0..6e47043 100644
--- a/chrome/browser/gtk/options/cookies_view.cc
+++ b/chrome/browser/gtk/options/cookies_view.cc
@@ -366,7 +366,16 @@ void CookiesView::RemoveSelectedCookies() {
   }
 }
 
+void CookiesView::OnAnyModelUpdateStart() {
+  g_signal_handlers_block_by_func(
+      G_OBJECT(selection_), reinterpret_cast<gpointer>(OnSelectionChanged),
+      this);
+}
+
 void CookiesView::OnAnyModelUpdate() {
+  g_signal_handlers_unblock_by_func(
+      G_OBJECT(selection_), reinterpret_cast<gpointer>(OnSelectionChanged),
+      this);
   EnableControls();
 }
 
diff --git a/chrome/browser/gtk/options/cookies_view.h b/chrome/browser/gtk/options/cookies_view.h
index f47a95b..be9dbb8 100644
--- a/chrome/browser/gtk/options/cookies_view.h
+++ b/chrome/browser/gtk/options/cookies_view.h
@@ -33,6 +33,7 @@ class CookiesView : public gtk_tree::ModelAdapter::Delegate {
   static void Show(Profile* profile);
 
   // gtk_tree::ModelAdapter::Delegate implementation.
+  virtual void OnAnyModelUpdateStart();
   virtual void OnAnyModelUpdate();
   virtual void SetColumnValues(int row, GtkTreeIter* iter);
 
@@ -140,6 +141,7 @@ class CookiesView : public gtk_tree::ModelAdapter::Delegate {
   friend class CookiesViewTest;
   FRIEND_TEST(CookiesViewTest, Empty);
   FRIEND_TEST(CookiesViewTest, RemoveAll);
+  FRIEND_TEST(CookiesViewTest, RemoveAllWithAllSelected);
   FRIEND_TEST(CookiesViewTest, Remove);
   FRIEND_TEST(CookiesViewTest, RemoveMultiple);
   FRIEND_TEST(CookiesViewTest, RemoveDefaultSelection);
diff --git a/chrome/browser/gtk/options/cookies_view_unittest.cc b/chrome/browser/gtk/options/cookies_view_unittest.cc
index 29337ff..ce0db2d 100644
--- a/chrome/browser/gtk/options/cookies_view_unittest.cc
+++ b/chrome/browser/gtk/options/cookies_view_unittest.cc
@@ -142,6 +142,38 @@ TEST_F(CookiesViewTest, RemoveAll) {
   }
 }
 
+// When removing all items, if multiple items were selected the
+// OnSelectionChanged callback could get called while the gtk list view and the
+// CookiesTableModel were inconsistent.  Test that it doesn't crash.
+TEST_F(CookiesViewTest, RemoveAllWithAllSelected) {
+  net::CookieMonster* monster =
+      profile_->GetRequestContext()->cookie_store()->GetCookieMonster();
+  monster->SetCookie(GURL("http://foo"), "A=1");
+  monster->SetCookie(GURL("http://foo2"), "B=1");
+  CookiesView cookies_view(profile_.get());
+
+  gtk_tree_selection_select_all(cookies_view.selection_);
+  {
+    SCOPED_TRACE("Before removing");
+    EXPECT_EQ(TRUE, GTK_WIDGET_SENSITIVE(cookies_view.remove_all_button_));
+    EXPECT_EQ(TRUE, GTK_WIDGET_SENSITIVE(cookies_view.remove_button_));
+    CheckDetailsSensitivity(FALSE, cookies_view);
+    EXPECT_EQ(2, gtk_tree_model_iter_n_children(
+        GTK_TREE_MODEL(cookies_view.list_store_), NULL));
+  }
+
+  gtk_button_clicked(GTK_BUTTON(cookies_view.remove_all_button_));
+  {
+    SCOPED_TRACE("After removing");
+    EXPECT_EQ(0u, monster->GetAllCookies().size());
+    EXPECT_EQ(FALSE, GTK_WIDGET_SENSITIVE(cookies_view.remove_all_button_));
+    EXPECT_EQ(FALSE, GTK_WIDGET_SENSITIVE(cookies_view.remove_button_));
+    CheckDetailsSensitivity(FALSE, cookies_view);
+    EXPECT_EQ(0, gtk_tree_model_iter_n_children(
+        GTK_TREE_MODEL(cookies_view.list_store_), NULL));
+  }
+}
+
 TEST_F(CookiesViewTest, Remove) {
   net::CookieMonster* monster =
       profile_->GetRequestContext()->cookie_store()->GetCookieMonster();
diff --git a/chrome/common/gtk_tree.cc b/chrome/common/gtk_tree.cc
index 6bca98b..de9df51 100644
--- a/chrome/common/gtk_tree.cc
+++ b/chrome/common/gtk_tree.cc
@@ -80,6 +80,7 @@ void ModelAdapter::AddNodeToList(int row) {
 }
 
 void ModelAdapter::OnModelChanged() {
+  delegate_->OnAnyModelUpdateStart();
   gtk_list_store_clear(list_store_);
   delegate_->OnModelChanged();
   for (int i = 0; i < table_model_->RowCount(); ++i)
@@ -88,6 +89,7 @@ void ModelAdapter::OnModelChanged() {
 }
 
 void ModelAdapter::OnItemsChanged(int start, int length) {
+  delegate_->OnAnyModelUpdateStart();
   GtkTreeIter iter;
   bool rv = gtk_tree_model_iter_nth_child(GTK_TREE_MODEL(list_store_), &iter,
                                           NULL, start);
@@ -103,6 +105,7 @@ void ModelAdapter::OnItemsChanged(int start, int length) {
 }
 
 void ModelAdapter::OnItemsAdded(int start, int length) {
+  delegate_->OnAnyModelUpdateStart();
   for (int i = 0; i < length; ++i) {
     AddNodeToList(start + i);
   }
@@ -110,6 +113,7 @@ void ModelAdapter::OnItemsAdded(int start, int length) {
 }
 
 void ModelAdapter::OnItemsRemoved(int start, int length) {
+  delegate_->OnAnyModelUpdateStart();
   GtkTreeIter iter;
   bool rv = gtk_tree_model_iter_nth_child(GTK_TREE_MODEL(list_store_), &iter,
                                           NULL, start);
diff --git a/chrome/common/gtk_tree.h b/chrome/common/gtk_tree.h
index 8caa64c..e0f7ba8 100644
--- a/chrome/common/gtk_tree.h
+++ b/chrome/common/gtk_tree.h
@@ -36,6 +36,9 @@ class ModelAdapter : public TableModelObserver {
     // Should fill in the column and row.
     virtual void SetColumnValues(int row, GtkTreeIter* iter) = 0;
 
+    // Called before any change to the TableModel.  Overriding optional.
+    virtual void OnAnyModelUpdateStart() {}
+
     // Called after any change to the TableModel.  Overriding optional.
     virtual void OnAnyModelUpdate() {}