From 508d27e151318b0a2ffdc6a4089ea3613462c26d Mon Sep 17 00:00:00 2001 From: "thakis@chromium.org" Date: Thu, 1 Jul 2010 16:17:37 +0000 Subject: Introduce RemoveRowTableModel interface, let GeolocationExceptionsTableModel derive from it. No functionality change. This will be used to share the content settings exceptions dialog code between geolocation and notifications. BUG=45547 Review URL: http://codereview.chromium.org/2838037 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51370 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/nibs/GeolocationExceptionsWindow.xib | 47 +++++++++++++--------- .../geolocation_exceptions_window_controller.h | 4 +- .../geolocation_exceptions_window_controller.mm | 10 ++--- ...cation_exceptions_window_controller_unittest.mm | 2 +- .../geolocation_exceptions_table_model.cc | 14 +++---- .../geolocation_exceptions_table_model.h | 14 +++---- .../geolocation_exceptions_table_model_unittest.cc | 10 ++--- .../geolocation_content_exceptions_window.cc | 4 +- chrome/browser/remove_rows_table_model.h | 30 ++++++++++++++ .../views/options/geolocation_exceptions_view.cc | 4 +- chrome/chrome_browser.gypi | 1 + 11 files changed, 89 insertions(+), 51 deletions(-) create mode 100644 chrome/browser/remove_rows_table_model.h diff --git a/chrome/app/nibs/GeolocationExceptionsWindow.xib b/chrome/app/nibs/GeolocationExceptionsWindow.xib index f3c57aa..c6de992 100644 --- a/chrome/app/nibs/GeolocationExceptionsWindow.xib +++ b/chrome/app/nibs/GeolocationExceptionsWindow.xib @@ -8,7 +8,7 @@ 353.00 YES - + YES @@ -398,22 +398,6 @@ 27 - - removeException: - - - - 34 - - - - removeAllExceptions: - - - - 35 - - removeButton_ @@ -461,6 +445,22 @@ 47 + + + removeRow: + + + + 48 + + + + removeAll: + + + + 49 + @@ -736,7 +736,7 @@ - 47 + 49 @@ -823,8 +823,8 @@ YES closeSheet: - removeAllExceptions: - removeException: + removeAll: + removeRow: YES @@ -866,6 +866,13 @@ NSObject IBProjectSource + browser/cocoa/objc_zombie.h + + + + NSObject + + IBProjectSource browser/cocoa/status_bubble_mac.h diff --git a/chrome/browser/cocoa/geolocation_exceptions_window_controller.h b/chrome/browser/cocoa/geolocation_exceptions_window_controller.h index f2debe0..76b29d83 100644 --- a/chrome/browser/cocoa/geolocation_exceptions_window_controller.h +++ b/chrome/browser/cocoa/geolocation_exceptions_window_controller.h @@ -37,7 +37,7 @@ class GeolocationObserverBridge; - (void)attachSheetTo:(NSWindow*)window; - (IBAction)closeSheet:(id)sender; -- (IBAction)removeException:(id)sender; -- (IBAction)removeAllExceptions:(id)sender; +- (IBAction)removeRow:(id)sender; +- (IBAction)removeAll:(id)sender; @end diff --git a/chrome/browser/cocoa/geolocation_exceptions_window_controller.mm b/chrome/browser/cocoa/geolocation_exceptions_window_controller.mm index fbb69d3..654d8e8 100644 --- a/chrome/browser/cocoa/geolocation_exceptions_window_controller.mm +++ b/chrome/browser/cocoa/geolocation_exceptions_window_controller.mm @@ -127,7 +127,7 @@ GeolocationExceptionsWindowController* g_exceptionWindow = nil; case NSDeleteFunctionKey: // Delete deletes. if ([[tableView_ selectedRowIndexes] count] > 0) - [self removeException:self]; + [self removeRow:self]; return; } } @@ -153,13 +153,13 @@ GeolocationExceptionsWindowController* g_exceptionWindow = nil; [NSApp endSheet:[self window]]; } -- (IBAction)removeException:(id)sender { +- (IBAction)removeRow:(id)sender { GeolocationExceptionsTableModel::Rows rows; [self selectedRows:&rows]; - model_->RemoveExceptions(rows); + model_->RemoveRows(rows); } -- (IBAction)removeAllExceptions:(id)sender { +- (IBAction)removeAll:(id)sender { model_->RemoveAll(); } @@ -208,7 +208,7 @@ GeolocationExceptionsWindowController* g_exceptionWindow = nil; - (void)adjustEditingButtons { GeolocationExceptionsTableModel::Rows rows; [self selectedRows:&rows]; - [removeButton_ setEnabled:model_->CanRemoveExceptions(rows)]; + [removeButton_ setEnabled:model_->CanRemoveRows(rows)]; [removeAllButton_ setEnabled:([tableView_ numberOfRows] > 0)]; } diff --git a/chrome/browser/cocoa/geolocation_exceptions_window_controller_unittest.mm b/chrome/browser/cocoa/geolocation_exceptions_window_controller_unittest.mm index 54df8f4..ae2036d 100644 --- a/chrome/browser/cocoa/geolocation_exceptions_window_controller_unittest.mm +++ b/chrome/browser/cocoa/geolocation_exceptions_window_controller_unittest.mm @@ -31,7 +31,7 @@ class GeolocationExceptionsWindowControllerTest : public CocoaTest { } void ClickRemoveAll(GeolocationExceptionsWindowController* controller) { - [controller removeAllExceptions:nil]; + [controller removeAll:nil]; } protected: diff --git a/chrome/browser/geolocation/geolocation_exceptions_table_model.cc b/chrome/browser/geolocation/geolocation_exceptions_table_model.cc index 95d2e79..c8a6202 100644 --- a/chrome/browser/geolocation/geolocation_exceptions_table_model.cc +++ b/chrome/browser/geolocation/geolocation_exceptions_table_model.cc @@ -57,7 +57,7 @@ GeolocationExceptionsTableModel::GeolocationExceptionsTableModel( AddEntriesForOrigin(i->first, i->second); } -bool GeolocationExceptionsTableModel::CanRemoveExceptions( +bool GeolocationExceptionsTableModel::CanRemoveRows( const Rows& rows) const { for (Rows::const_iterator i(rows.begin()); i != rows.end(); ++i) { const Entry& entry = entries_[*i]; @@ -73,7 +73,7 @@ bool GeolocationExceptionsTableModel::CanRemoveExceptions( return !rows.empty(); } -void GeolocationExceptionsTableModel::RemoveExceptions(const Rows& rows) { +void GeolocationExceptionsTableModel::RemoveRows(const Rows& rows) { for (Rows::const_reverse_iterator i(rows.rbegin()); i != rows.rend(); ++i) { size_t row = *i; Entry* entry = &entries_[row]; @@ -95,11 +95,11 @@ void GeolocationExceptionsTableModel::RemoveExceptions(const Rows& rows) { entries_.erase(entries_.begin() + row); // Note: |entry| is now garbage. if (observer_) observer_->OnItemsRemoved(row, 1); - // If we remove the last non-default child of a default parent, we - // should remove the parent too. We do these removals one-at-a-time - // because the table view will end up being called back as each row is - // removed, in turn calling back to CanRemoveExceptions(), and if we've - // already removed more entries than the view has, we'll have problems. + // If we remove the last non-default child of a default parent, we should + // remove the parent too. We do these removals one-at-a-time because the + // table view will end up being called back as each row is removed, in + // turn calling back to CanRemoveRows(), and if we've already removed + // more entries than the view has, we'll have problems. if ((row == 0) || rows.count(row - 1)) break; entry = &entries_[--row]; diff --git a/chrome/browser/geolocation/geolocation_exceptions_table_model.h b/chrome/browser/geolocation/geolocation_exceptions_table_model.h index 6328166..c458c0e 100644 --- a/chrome/browser/geolocation/geolocation_exceptions_table_model.h +++ b/chrome/browser/geolocation/geolocation_exceptions_table_model.h @@ -8,32 +8,32 @@ #include #include -#include "app/table_model.h" #include "chrome/browser/geolocation/geolocation_content_settings_map.h" +#include "chrome/browser/remove_rows_table_model.h" #include "chrome/common/content_settings.h" #include "chrome/common/content_settings_types.h" -class GeolocationExceptionsTableModel : public TableModel { +class GeolocationExceptionsTableModel : public RemoveRowsTableModel { public: - typedef std::set Rows; - explicit GeolocationExceptionsTableModel( GeolocationContentSettingsMap* map); + // RemoveRowsTableModel overrides: + // Return whether the given set of rows can be removed. A parent with setting // of CONTENT_SETTING_DEFAULT can't be removed unless all its children are // also being removed. - bool CanRemoveExceptions(const Rows& rows) const; + virtual bool CanRemoveRows(const Rows& rows) const; // Removes the exceptions at the specified indexes. If an exception is a // parent, and it has children, the row in model will be updated to have // CONTENT_SETTING_DEFAULT. If it is the only child of a // CONTENT_SETTING_DEFAULT parent, the parent will be removed from the model // too. - void RemoveExceptions(const Rows& rows); + virtual void RemoveRows(const Rows& rows); // Removes all the exceptions from both the map and model. - void RemoveAll(); + virtual void RemoveAll(); // TableModel overrides: virtual int RowCount(); diff --git a/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc b/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc index 62b65c0..d6d1f5e 100644 --- a/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc +++ b/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc @@ -71,7 +71,7 @@ TEST_F(GeolocationExceptionsTableModelTest, CanRemoveException) { EXPECT_EQ(1, model_->RowCount()); GeolocationExceptionsTableModel::Rows rows; rows.insert(0U); - EXPECT_TRUE(model_->CanRemoveExceptions(rows)); + EXPECT_TRUE(model_->CanRemoveRows(rows)); // Ensure an entry with children can't be removed. @@ -80,13 +80,13 @@ TEST_F(GeolocationExceptionsTableModelTest, CanRemoveException) { map->SetContentSetting(kUrl0, kUrl2, CONTENT_SETTING_BLOCK); ResetModel(); EXPECT_EQ(3, model_->RowCount()); - EXPECT_FALSE(model_->CanRemoveExceptions(rows)); + EXPECT_FALSE(model_->CanRemoveRows(rows)); // Ensure it can be removed if removing all children. rows.clear(); rows.insert(1U); rows.insert(2U); - EXPECT_TRUE(model_->CanRemoveExceptions(rows)); + EXPECT_TRUE(model_->CanRemoveRows(rows)); } TEST_F(GeolocationExceptionsTableModelTest, RemoveExceptions) { @@ -97,7 +97,7 @@ TEST_F(GeolocationExceptionsTableModelTest, RemoveExceptions) { // Test removing parent exception. GeolocationExceptionsTableModel::Rows rows; rows.insert(0U); - model_->RemoveExceptions(rows); + model_->RemoveRows(rows); EXPECT_EQ(CONTENT_SETTING_ASK, map->GetContentSetting(kUrl0, kUrl0)); EXPECT_EQ(CONTENT_SETTING_ALLOW, map->GetContentSetting(kUrl0, kUrl1)); EXPECT_EQ(CONTENT_SETTING_ALLOW, map->GetContentSetting(kUrl0, kUrl2)); @@ -109,7 +109,7 @@ TEST_F(GeolocationExceptionsTableModelTest, RemoveExceptions) { rows.clear(); rows.insert(1U); rows.insert(2U); - model_->RemoveExceptions(rows); + model_->RemoveRows(rows); EXPECT_EQ(0, model_->RowCount()); EXPECT_EQ(CONTENT_SETTING_ASK, map->GetContentSetting(kUrl0, kUrl0)); EXPECT_EQ(CONTENT_SETTING_ASK, map->GetContentSetting(kUrl0, kUrl1)); diff --git a/chrome/browser/gtk/options/geolocation_content_exceptions_window.cc b/chrome/browser/gtk/options/geolocation_content_exceptions_window.cc index 79cfe20..e1e28e4 100644 --- a/chrome/browser/gtk/options/geolocation_content_exceptions_window.cc +++ b/chrome/browser/gtk/options/geolocation_content_exceptions_window.cc @@ -141,7 +141,7 @@ void GeolocationContentExceptionsWindow::UpdateButtonState() { GeolocationExceptionsTableModel::Rows rows; GetSelectedRows(&rows); - gtk_widget_set_sensitive(remove_button_, model_->CanRemoveExceptions(rows)); + gtk_widget_set_sensitive(remove_button_, model_->CanRemoveRows(rows)); gtk_widget_set_sensitive(remove_all_button_, row_count > 0); } @@ -156,7 +156,7 @@ void GeolocationContentExceptionsWindow::GetSelectedRows( void GeolocationContentExceptionsWindow::Remove(GtkWidget* widget) { GeolocationExceptionsTableModel::Rows rows; GetSelectedRows(&rows); - model_->RemoveExceptions(rows); + model_->RemoveRows(rows); UpdateButtonState(); } diff --git a/chrome/browser/remove_rows_table_model.h b/chrome/browser/remove_rows_table_model.h new file mode 100644 index 0000000..564d91d --- /dev/null +++ b/chrome/browser/remove_rows_table_model.h @@ -0,0 +1,30 @@ +// 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. + +#ifndef CHROME_BROWSER_REMOVE_ROWS_TABLE_MODEL_H_ +#define CHROME_BROWSER_REMOVE_ROWS_TABLE_MODEL_H_ + +#include +#include + +#include "app/table_model.h" + +// A table model that also supports removing rows. Used for example for the +// content settings exception dialog for geolocation. +class RemoveRowsTableModel : public TableModel { + public: + typedef std::set Rows; + + // Returns whether or not the rows can be removed. + virtual bool CanRemoveRows(const Rows& rows) const = 0; + + // Remove the rows from the table. + virtual void RemoveRows(const Rows& rows) = 0; + + // Removes all the rows. + virtual void RemoveAll() = 0; +}; + +#endif // CHROME_BROWSER_REMOVE_ROWS_TABLE_MODEL_H_ + diff --git a/chrome/browser/views/options/geolocation_exceptions_view.cc b/chrome/browser/views/options/geolocation_exceptions_view.cc index 869fcbb..4adff3b 100644 --- a/chrome/browser/views/options/geolocation_exceptions_view.cc +++ b/chrome/browser/views/options/geolocation_exceptions_view.cc @@ -165,12 +165,12 @@ GeolocationExceptionsTableModel::Rows } void GeolocationExceptionsView::UpdateButtonState() { - remove_button_->SetEnabled(model_.CanRemoveExceptions(GetSelectedRows())); + remove_button_->SetEnabled(model_.CanRemoveRows(GetSelectedRows())); remove_all_button_->SetEnabled(model_.RowCount() > 0); } void GeolocationExceptionsView::Remove() { - model_.RemoveExceptions(GetSelectedRows()); + model_.RemoveRows(GetSelectedRows()); UpdateButtonState(); } diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index f2369ae..ffbb693 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1956,6 +1956,7 @@ 'browser/profile_import_process_host.h', 'browser/profile_manager.cc', 'browser/profile_manager.h', + 'browser/remove_rows_table_model.h', 'browser/renderer_host/async_resource_handler.cc', 'browser/renderer_host/async_resource_handler.h', 'browser/renderer_host/audio_renderer_host.cc', -- cgit v1.1