Introduce RemoveRowTableModel interface, let GeolocationExceptionsTableModel derive from it.

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
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51370

[bulach, 2010-07-01 15:48:06]

LGTM

thanks, thakis! a few suggestions and meta-comments below:

http://codereview.chromium.org/2838037/diff/1/2
File chrome/browser/geolocation/geolocation_exceptions_table_model.h (left):

http://codereview.chromium.org/2838037/diff/1/2#oldcode32
chrome/browser/geolocation/geolocation_exceptions_table_model.h:32: // too.
see below, I think these comments are fairly specific for this case, and perhaps
they should be kept here..

http://codereview.chromium.org/2838037/diff/1/3
File chrome/browser/remove_rows_table_model.h (right):

http://codereview.chromium.org/2838037/diff/1/3#newcode21
chrome/browser/remove_rows_table_model.h:21: // also being removed.
I wouldn't mention anything about CONTENT_SETTING and Exception at this level..
these specific comments should be on the geolocation implementation.

perhaps something like:

// Returns whether or not the rows can be removed.
virtual bool CanRemoveRows(const Rows& rows) const = 0;

// Effectively remove the rows from the table.
virtual void RemoveRows(const Rows& rows) = 0;

// Removes all rows.
virtual void RemoveAll() = 0;

[Nico, 2010-07-01 16:00:04]

Thanks!

http://codereview.chromium.org/2838037/diff/1/2
File chrome/browser/geolocation/geolocation_exceptions_table_model.h (left):

http://codereview.chromium.org/2838037/diff/1/2#oldcode32
chrome/browser/geolocation/geolocation_exceptions_table_model.h:32: // too.
On 2010/07/01 15:48:06, bulach wrote:
> see below, I think these comments are fairly specific for this case, and
perhaps
> they should be kept here..

Good point, done

http://codereview.chromium.org/2838037/diff/1/3
File chrome/browser/remove_rows_table_model.h (right):

http://codereview.chromium.org/2838037/diff/1/3#newcode21
chrome/browser/remove_rows_table_model.h:21: // also being removed.
On 2010/07/01 15:48:06, bulach wrote:
> I wouldn't mention anything about CONTENT_SETTING and Exception at this
level..
> these specific comments should be on the geolocation implementation.
> 
> perhaps something like:
> 
> // Returns whether or not the rows can be removed.
> virtual bool CanRemoveRows(const Rows& rows) const = 0;
> 
> // Effectively remove the rows from the table.
> virtual void RemoveRows(const Rows& rows) = 0;
> 
> // Removes all rows.
> virtual void RemoveAll() = 0;

Done.

[bulach, 2010-07-01 16:04:06]

LGTM

thanks!