Sorry, this is making the Heapcheck bot unhappy. http://build.chromium.org/buildbot/memory/builders/Linux%20Heapcheck/builds/4501/steps/heapcheck%20test:%20unit/logs/stdio

i.e.
The 197 largest leaks:
Leak of 128 bytes in 1 objects allocated from:
  @ ed9000 __gnu_cxx::new_allocator::allocate
  @ ed9027 std::_Vector_base::_M_allocate
  @ ed932c std::vector::_M_insert_aux
  @ ed94e4 std::vector::push_back
  @ ed59b2 HostContentSettingsMap::GetSettingsForOneType
  @ 11f2edb ContentExceptionsTableModel
  @ e491e1 ContentExceptionsWindowGtk
  @ 5fdbb7 ContentExceptionsWindowGtkUnittest::BuildWindow
  @ 5facf2 ContentExceptionsWindowGtkUnittest_TestComplexRemoval_Test::TestBody
  @ 15b7dfd testing::Test::Run
  @ 15bb5f2 testing::internal::TestInfoImpl::Run
  @ 15bb728 testing::TestCase::Run
  @ 15bc18a testing::internal::UnitTestImpl::RunAllTests
  @ 15bc301 testing::UnitTest::Run
  @ 14b7b33 TestSuite::Run
  @ 14b627d main
  @ 2b410ff7e1c4 __libc_start_main

Revert 51450 - Reland r51414: "GTK: Fix sorting in content exception window."

Don't use scoped_ptr in unit test. Manually delete the dailog (which deletes
ContentExceptionsWindowGtk.)

First Review URL: http://codereview.chromium.org/2876032
BUG=47841
TEST=ContentExceptionsWindowGtkUnittest.* + see bug. Ran valgrind on several gtk tests including new ones and fixed the memory errors.

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

TBR=erg@google.com


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

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

3 files changed, 16 insertions, 259 deletions

diff --git a/chrome/browser/gtk/options/content_exceptions_window_gtk.cc b/chrome/browser/gtk/options/content_exceptions_window_gtk.cc
index c7d6052..0aa85e3 100644
--- a/chrome/browser/gtk/options/content_exceptions_window_gtk.cc
+++ b/chrome/browser/gtk/options/content_exceptions_window_gtk.cc
@@ -47,14 +47,11 @@ ContentExceptionsWindowGtk::ContentExceptionsWindowGtk(
     GtkWindow* parent,
     HostContentSettingsMap* map,
     ContentSettingsType type) {
-  // Build the list backing that GTK uses, along with an adapter which will
-  // sort stuff without changing the underlying backing store.
+  // Build the model adapters that translate views and TableModels into
+  // something GTK can use.
   list_store_ = gtk_list_store_new(COL_COUNT, G_TYPE_STRING, G_TYPE_STRING);
-  sort_list_store_ = gtk_tree_model_sort_new_with_model(
-      GTK_TREE_MODEL(list_store_));
-  treeview_ = gtk_tree_view_new_with_model(GTK_TREE_MODEL(sort_list_store_));
+  treeview_ = gtk_tree_view_new_with_model(GTK_TREE_MODEL(list_store_));
   g_object_unref(list_store_);
-  g_object_unref(sort_list_store_);
 
   // Set up the properties of the treeview
   gtk_tree_view_set_headers_visible(GTK_TREE_VIEW(treeview_), TRUE);
@@ -188,7 +185,7 @@ void ContentExceptionsWindowGtk::UpdateButtonState() {
   int num_selected = gtk_tree_selection_count_selected_rows(
       treeview_selection_);
   int row_count = gtk_tree_model_iter_n_children(
-      GTK_TREE_MODEL(sort_list_store_), NULL);
+      GTK_TREE_MODEL(list_store_), NULL);
 
   // TODO(erg): http://crbug.com/34177 , support editing of more than one entry
   // at a time.
@@ -205,10 +202,10 @@ void ContentExceptionsWindowGtk::Add(GtkWidget* widget) {
 }
 
 void ContentExceptionsWindowGtk::Edit(GtkWidget* widget) {
-  std::set<std::pair<int, int> > indices;
-  GetSelectedModelIndices(&indices);
+  std::set<int> indices;
+  gtk_tree::GetSelectedIndices(treeview_selection_, &indices);
   DCHECK_GT(indices.size(), 0u);
-  int index = indices.begin()->first;
+  int index = *indices.begin();
   const HostContentSettingsMap::PatternSettingPair& entry =
       model_->entry_at(index);
   new ContentExceptionEditor(GTK_WINDOW(dialog_), this, model_.get(), index,
@@ -216,16 +213,14 @@ void ContentExceptionsWindowGtk::Edit(GtkWidget* widget) {
 }
 
 void ContentExceptionsWindowGtk::Remove(GtkWidget* widget) {
-  std::set<std::pair<int, int> > model_selected_indices;
-  GetSelectedModelIndices(&model_selected_indices);
-
-  int selected_row = gtk_tree_model_iter_n_children(
-      GTK_TREE_MODEL(sort_list_store_), NULL);
-  for (std::set<std::pair<int, int> >::reverse_iterator i =
-           model_selected_indices.rbegin();
-       i != model_selected_indices.rend(); ++i) {
-    model_->RemoveException(i->first);
-    selected_row = std::min(selected_row, i->second);
+  std::set<int> selected_indices;
+  gtk_tree::GetSelectedIndices(treeview_selection_, &selected_indices);
+
+  int selected_row = 0;
+  for (std::set<int>::reverse_iterator i = selected_indices.rbegin();
+       i != selected_indices.rend(); ++i) {
+    model_->RemoveException(*i);
+    selected_row = *i;
   }
 
   int row_count = model_->RowCount();
@@ -262,25 +257,6 @@ std::string ContentExceptionsWindowGtk::GetWindowTitle() const {
   return std::string();
 }
 
-void ContentExceptionsWindowGtk::GetSelectedModelIndices(
-    std::set<std::pair<int, int> >* indices) {
-  GtkTreeModel* model;
-  GList* paths = gtk_tree_selection_get_selected_rows(treeview_selection_,
-                                                      &model);
-  for (GList* item = paths; item; item = item->next) {
-    GtkTreePath* sorted_path = reinterpret_cast<GtkTreePath*>(item->data);
-    int sorted_row = gtk_tree::GetRowNumForPath(sorted_path);
-    GtkTreePath* path = gtk_tree_model_sort_convert_path_to_child_path(
-        GTK_TREE_MODEL_SORT(sort_list_store_), sorted_path);
-    int row = gtk_tree::GetRowNumForPath(path);
-    gtk_tree_path_free(path);
-    indices->insert(std::make_pair(row, sorted_row));
-  }
-
-  g_list_foreach(paths, (GFunc)gtk_tree_path_free, NULL);
-  g_list_free(paths);
-}
-
 void ContentExceptionsWindowGtk::OnTreeViewRowActivate(
     GtkWidget* sender,
     GtkTreePath* path,
diff --git a/chrome/browser/gtk/options/content_exceptions_window_gtk.h b/chrome/browser/gtk/options/content_exceptions_window_gtk.h
index 473c6be..c415419 100644
--- a/chrome/browser/gtk/options/content_exceptions_window_gtk.h
+++ b/chrome/browser/gtk/options/content_exceptions_window_gtk.h
@@ -65,10 +65,6 @@ class ContentExceptionsWindowGtk : public gtk_tree::TableAdapter::Delegate,
   // was set to in the constructor).
   std::string GetWindowTitle() const;
 
-  // Gets the selected indicies in the two list stores. Indicies are returned
-  // in <list_store_, sort_list_store_> order.
-  void GetSelectedModelIndices(std::set<std::pair<int, int> >* indices);
-
   // GTK Callbacks
   CHROMEGTK_CALLBACK_2(ContentExceptionsWindowGtk, void,
                        OnTreeViewRowActivate, GtkTreePath*, GtkTreeViewColumn*);
@@ -76,12 +72,7 @@ class ContentExceptionsWindowGtk : public gtk_tree::TableAdapter::Delegate,
   CHROMEGTK_CALLBACK_0(ContentExceptionsWindowGtk, void,
                        OnTreeSelectionChanged);
 
-  // The list presented in |treeview_|. Separate from |list_store_|, the list
-  // that backs |sort_model_|. This separation comes so the user can sort the
-  // data on screen without changing the underlying |list_store_|.
-  GtkTreeModel* sort_list_store_;
-
-  // The backing to |sort_list_store_|. Doesn't change when sorted.
+  // The list presented in |treeview_|; a gobject instead of a C++ object.
   GtkListStore* list_store_;
 
   // The C++, views-ish, cross-platform model class that actually contains the
@@ -105,8 +96,6 @@ class ContentExceptionsWindowGtk : public gtk_tree::TableAdapter::Delegate,
   GtkWidget* edit_button_;
   GtkWidget* remove_button_;
   GtkWidget* remove_all_button_;
-
-  friend class ContentExceptionsWindowGtkUnittest;
 };
 
 #endif  // CHROME_BROWSER_GTK_OPTIONS_CONTENT_EXCEPTIONS_WINDOW_GTK_H_
diff --git a/chrome/browser/gtk/options/content_exceptions_window_gtk_unittest.cc b/chrome/browser/gtk/options/content_exceptions_window_gtk_unittest.cc
deleted file mode 100644
index 7da9ce5..0000000
--- a/chrome/browser/gtk/options/content_exceptions_window_gtk_unittest.cc
+++ /dev/null
@@ -1,208 +0,0 @@
-// Copyright (c) 2010 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/gtk/options/content_exceptions_window_gtk.h"
-
-#include "chrome/test/testing_profile.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using ::testing::ElementsAre;
-
-class ContentExceptionsWindowGtkUnittest : public testing::Test {
- public:
-  ContentExceptionsWindowGtkUnittest()
-      : ui_thread_(ChromeThread::UI, &message_loop_),
-        host_content_settings_map_(profile_.GetHostContentSettingsMap()),
-        window_(NULL) {
-  }
-
-  ~ContentExceptionsWindowGtkUnittest() {
-    // This will delete window_ too.
-    gtk_widget_destroy(window_->dialog_);
-  }
-
-  void AddException(const std::string& pattern, ContentSetting value) {
-    host_content_settings_map_->SetContentSetting(
-        HostContentSettingsMap::Pattern(pattern),
-        CONTENT_SETTINGS_TYPE_JAVASCRIPT,
-        value);
-  }
-
-  void BuildWindow() {
-    window_ = new ContentExceptionsWindowGtk(
-        NULL,
-        host_content_settings_map_,
-        CONTENT_SETTINGS_TYPE_JAVASCRIPT);
-  }
-
-  // Actions:
-  void SelectRows(const std::vector<int>& rows) {
-    gtk_tree_selection_unselect_all(window_->treeview_selection_);
-
-    for (std::vector<int>::const_iterator it = rows.begin(); it != rows.end();
-         ++it) {
-      GtkTreeIter iter;
-      if (!gtk_tree_model_iter_nth_child(window_->sort_list_store_,
-                                         &iter, NULL, *it)) {
-        NOTREACHED();
-        return;
-      }
-
-      gtk_tree_selection_select_iter(window_->treeview_selection_, &iter);
-    }
-  }
-
-  void Remove() {
-    window_->Remove(window_->remove_button_);
-  }
-
-  // Getters:
-
-  void GetSelectedRows(std::set<std::pair<int, int> >* selected) {
-    window_->GetSelectedModelIndices(selected);
-  }
-
-  int StoreListStoreCount() {
-    return gtk_tree_model_iter_n_children(
-        GTK_TREE_MODEL(window_->sort_list_store_), NULL);
-  }
-
-  int ListStoreCount() {
-    return gtk_tree_model_iter_n_children(
-        GTK_TREE_MODEL(window_->list_store_), NULL);
-  }
-
-  std::set<std::string> Paths() {
-    std::set<std::string> paths;
-    GtkTreeIter iter;
-    bool valid = gtk_tree_model_get_iter_first(
-        GTK_TREE_MODEL(window_->sort_list_store_), &iter);
-
-    while (valid) {
-      gchar* str = NULL;
-      gtk_tree_model_get(GTK_TREE_MODEL(window_->sort_list_store_), &iter,
-                         0, &str, -1);
-      paths.insert(str);
-      g_free(str);
-
-      valid = gtk_tree_model_iter_next(
-          GTK_TREE_MODEL(window_->sort_list_store_), &iter);
-    }
-
-    return paths;
-  }
-
-  // Whether certain widgets are enabled:
-  bool EditButtonSensitive() {
-    return GTK_WIDGET_SENSITIVE(window_->edit_button_);
-  }
-
-  bool RemoveButtonSensitive() {
-    return GTK_WIDGET_SENSITIVE(window_->remove_button_);
-  }
-
-  bool RemoveAllSensitive() {
-    return GTK_WIDGET_SENSITIVE(window_->remove_all_button_);
-  }
-
- private:
-  MessageLoop message_loop_;
-  ChromeThread ui_thread_;
-
-  TestingProfile profile_;
-  HostContentSettingsMap* host_content_settings_map_;
-
-  ContentExceptionsWindowGtk* window_;
-};
-
-TEST_F(ContentExceptionsWindowGtkUnittest, TestEmptyDisplay) {
-  BuildWindow();
-
-  EXPECT_EQ(0, StoreListStoreCount());
-  EXPECT_EQ(0, ListStoreCount());
-
-  EXPECT_FALSE(RemoveAllSensitive());
-}
-
-TEST_F(ContentExceptionsWindowGtkUnittest, TestDisplay) {
-  AddException("a", CONTENT_SETTING_BLOCK);
-  AddException("b", CONTENT_SETTING_ALLOW);
-
-  BuildWindow();
-
-  EXPECT_EQ(2, StoreListStoreCount());
-  EXPECT_EQ(2, ListStoreCount());
-
-  EXPECT_TRUE(RemoveAllSensitive());
-}
-
-TEST_F(ContentExceptionsWindowGtkUnittest, TestSelection) {
-  AddException("a", CONTENT_SETTING_BLOCK);
-
-  BuildWindow();
-
-  EXPECT_FALSE(EditButtonSensitive());
-  EXPECT_FALSE(RemoveButtonSensitive());
-
-  std::vector<int> rows;
-  rows.push_back(0);
-  SelectRows(rows);
-
-  EXPECT_TRUE(EditButtonSensitive());
-  EXPECT_TRUE(RemoveButtonSensitive());
-}
-
-TEST_F(ContentExceptionsWindowGtkUnittest, TestSimpleRemoval) {
-  AddException("a", CONTENT_SETTING_BLOCK);
-  AddException("b", CONTENT_SETTING_ALLOW);
-  AddException("c", CONTENT_SETTING_BLOCK);
-  AddException("d", CONTENT_SETTING_ALLOW);
-
-  BuildWindow();
-
-  std::vector<int> rows;
-  rows.push_back(1);
-  SelectRows(rows);
-
-  Remove();
-
-  EXPECT_EQ(3, StoreListStoreCount());
-  EXPECT_EQ(3, ListStoreCount());
-  EXPECT_TRUE(EditButtonSensitive());
-  EXPECT_TRUE(RemoveButtonSensitive());
-
-  std::set<std::pair<int, int> > selected;
-  GetSelectedRows(&selected);
-
-  ASSERT_EQ(1u, selected.size());
-  EXPECT_EQ(1, selected.begin()->first);
-  EXPECT_EQ(1, selected.begin()->second);
-  EXPECT_THAT(Paths(), ElementsAre("a", "c", "d"));
-}
-
-TEST_F(ContentExceptionsWindowGtkUnittest, TestComplexRemoval) {
-  AddException("a", CONTENT_SETTING_BLOCK);
-  AddException("b", CONTENT_SETTING_ALLOW);
-  AddException("c", CONTENT_SETTING_BLOCK);
-  AddException("d", CONTENT_SETTING_ALLOW);
-  AddException("e", CONTENT_SETTING_BLOCK);
-
-  BuildWindow();
-
-  std::vector<int> rows;
-  rows.push_back(1);
-  rows.push_back(3);
-  SelectRows(rows);
-
-  Remove();
-
-  std::set<std::pair<int, int> > selected;
-  GetSelectedRows(&selected);
-
-  ASSERT_EQ(1u, selected.size());
-  EXPECT_EQ(1, selected.begin()->first);
-  EXPECT_EQ(1, selected.begin()->second);
-  EXPECT_THAT(Paths(), ElementsAre("a", "c", "e"));
-}