Use tab to group "Show saved passwords" and "Exceptions"...

Use tab to group "Show saved passwords" and "Exceptions"
BUG=9026
"Show saved passwords" and "Exceptions" were splited into
2 dialogs, group together in one single dialog with
different pages by tabbing.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=13786

[tim (not reviewing), 2009-04-13 23:42:36]

sorry for the delay! looks good, just a bunch of nits, most aren't your fault
but the copy/paste exposed them :) I'll make a final pass too.

http://codereview.chromium.org/67055/diff/1022/1027
File chrome/browser/views/options/exceptions_page_view.cc (right):

http://codereview.chromium.org/67055/diff/1022/1027#newcode11
Line 11: #include "chrome/common/pref_names.h"
nit - alphabetize includes

http://codereview.chromium.org/67055/diff/1022/1027#newcode84
Line 84: table_model_(profile) {
table_view_(NULL) would be nice.

http://codereview.chromium.org/67055/diff/1022/1027#newcode160
Line 160: // Tell the table model we are concern about how many rows it has.
nit- concerned

http://codereview.chromium.org/67055/diff/1022/1026
File chrome/browser/views/options/passwords_exceptions_window_view.cc (right):

http://codereview.chromium.org/67055/diff/1022/1026#newcode25
Line 25: profile_(profile) {
tabs_(NULL)

http://codereview.chromium.org/67055/diff/1022/1026#newcode29
Line 29: }
this can go in .h

http://codereview.chromium.org/67055/diff/1022/1026#newcode79
Line 79: // to NULL.
nit - instance

http://codereview.chromium.org/67055/diff/1022/1028
File chrome/browser/views/options/passwords_exceptions_window_view.h (right):

http://codereview.chromium.org/67055/diff/1022/1028#newcode5
Line 5: #ifndef CHROME_BROWSER_VIEWS_OPTIONS_PASSWORDS_EXCEPTIONS_WINDOW_H_
ifdef should match class name

http://codereview.chromium.org/67055/diff/1022/1028#newcode18
Line 18: // PasswordsExceptionsWindowView
i would have just used one of PasswordsExceptions{View, Window}, but no big
deal.

http://codereview.chromium.org/67055/diff/1022/1028#newcode61
Line 61: DISALLOW_EVIL_CONSTRUCTORS(PasswordsExceptionsWindowView);
DISALLOW_COPY_AND_ASSIGN

http://codereview.chromium.org/67055/diff/1022/1030
File chrome/browser/views/options/passwords_page_view.cc (right):

http://codereview.chromium.org/67055/diff/1022/1030#newcode11
Line 11: #include "chrome/common/pref_names.h"
alphabetize includes

http://codereview.chromium.org/67055/diff/1022/1030#newcode68
Line 68: int col_id) {
indent mangled a bit

http://codereview.chromium.org/67055/diff/1022/1030#newcode92
Line 92: int column_id) {
fix indent

http://codereview.chromium.org/67055/diff/1022/1030#newcode194
Line 194: table_model_(profile) {
table_view_(NULL)

http://codereview.chromium.org/67055/diff/1022/1031
File chrome/browser/views/options/passwords_page_view.h (right):

http://codereview.chromium.org/67055/diff/1022/1031#newcode13
Line 13: #include "chrome/common/stl_util-inl.h"
nit- alphabetize includes

http://codereview.chromium.org/67055/diff/1022/1031#newcode56
Line 56: DISALLOW_EVIL_CONSTRUCTORS(MultiLabelButtons);
nit - disallow_copy_and_assign

http://codereview.chromium.org/67055/diff/1022/1031#newcode136
Line 136: DISALLOW_EVIL_CONSTRUCTORS(PasswordsTableModel);
nit - disallow_copy

[Alpha Left Google, 2009-04-14 01:17:50]

http://codereview.chromium.org/67055/diff/1022/1027
File chrome/browser/views/options/exceptions_page_view.cc (right):

http://codereview.chromium.org/67055/diff/1022/1027#newcode11
Line 11: #include "chrome/common/pref_names.h"
On 2009/04/13 23:42:36, timsteele wrote:
> nit - alphabetize includes

done.

http://codereview.chromium.org/67055/diff/1022/1027#newcode84
Line 84: table_model_(profile) {
On 2009/04/13 23:42:36, timsteele wrote:
> table_view_(NULL) would be nice.

done.

http://codereview.chromium.org/67055/diff/1022/1027#newcode160
Line 160: // Tell the table model we are concern about how many rows it has.
On 2009/04/13 23:42:36, timsteele wrote:
> nit- concerned

done.

http://codereview.chromium.org/67055/diff/1022/1026
File chrome/browser/views/options/passwords_exceptions_window_view.cc (right):

http://codereview.chromium.org/67055/diff/1022/1026#newcode25
Line 25: profile_(profile) {
On 2009/04/13 23:42:36, timsteele wrote:
> tabs_(NULL)

Done.

http://codereview.chromium.org/67055/diff/1022/1026#newcode29
Line 29: }
On 2009/04/13 23:42:36, timsteele wrote:
> this can go in .h

Done.

http://codereview.chromium.org/67055/diff/1022/1026#newcode29
Line 29: }
On 2009/04/13 23:42:36, timsteele wrote:
> this can go in .h

Done.

http://codereview.chromium.org/67055/diff/1022/1026#newcode79
Line 79: // to NULL.
On 2009/04/13 23:42:36, timsteele wrote:
> nit - instance

Done.

http://codereview.chromium.org/67055/diff/1022/1028
File chrome/browser/views/options/passwords_exceptions_window_view.h (right):

http://codereview.chromium.org/67055/diff/1022/1028#newcode5
Line 5: #ifndef CHROME_BROWSER_VIEWS_OPTIONS_PASSWORDS_EXCEPTIONS_WINDOW_H_
On 2009/04/13 23:42:36, timsteele wrote:
> ifdef should match class name

Done.

http://codereview.chromium.org/67055/diff/1022/1028#newcode18
Line 18: // PasswordsExceptionsWindowView
On 2009/04/13 23:42:36, timsteele wrote:
> i would have just used one of PasswordsExceptions{View, Window}, but no big
> deal.

I'm trying to stay the same with FontsLanguagesWindowView.

http://codereview.chromium.org/67055/diff/1022/1028#newcode61
Line 61: DISALLOW_EVIL_CONSTRUCTORS(PasswordsExceptionsWindowView);
On 2009/04/13 23:42:36, timsteele wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

http://codereview.chromium.org/67055/diff/1022/1030
File chrome/browser/views/options/passwords_page_view.cc (right):

http://codereview.chromium.org/67055/diff/1022/1030#newcode11
Line 11: #include "chrome/common/pref_names.h"
On 2009/04/13 23:42:36, timsteele wrote:
> alphabetize includes

Done.

http://codereview.chromium.org/67055/diff/1022/1030#newcode68
Line 68: int col_id) {
On 2009/04/13 23:42:36, timsteele wrote:
> indent mangled a bit

Done.

http://codereview.chromium.org/67055/diff/1022/1030#newcode92
Line 92: int column_id) {
On 2009/04/13 23:42:36, timsteele wrote:
> fix indent

Done.

http://codereview.chromium.org/67055/diff/1022/1031
File chrome/browser/views/options/passwords_page_view.h (right):

http://codereview.chromium.org/67055/diff/1022/1031#newcode13
Line 13: #include "chrome/common/stl_util-inl.h"
On 2009/04/13 23:42:36, timsteele wrote:
> nit- alphabetize includes

Done.

http://codereview.chromium.org/67055/diff/1022/1031#newcode56
Line 56: DISALLOW_EVIL_CONSTRUCTORS(MultiLabelButtons);
On 2009/04/13 23:42:36, timsteele wrote:
> nit - disallow_copy_and_assign

Done.

http://codereview.chromium.org/67055/diff/1022/1031#newcode136
Line 136: DISALLOW_EVIL_CONSTRUCTORS(PasswordsTableModel);
On 2009/04/13 23:42:36, timsteele wrote:
> nit - disallow_copy

Done.

[tim (not reviewing), 2009-04-14 05:19:19]

It LGTM