Mac: Select right tab in content settings dialog.

Mac: Select right tab in content settings dialog.

Also fix an unrelated TODO in this area.

xib change: Make ContentSettingsWindowController the NSTabView's delegate and hook the controller's IBOutlet up to the NSTabView.

BUG=36949
TEST=Open content settings via a blocked popups bubble. Popup tab should be selected. Select javascript tab, close window. Reopen window, javascript tab should still be selected.

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

[Nico, 2010-03-02 04:50:31]

The xib file is amazingly clean.

[Nico, 2010-03-02 04:53:30]

On 2010/03/02 04:50:31, Nico wrote:
> The xib file is amazingly clean.

(the diff, that is)

[viettrungluu, 2010-03-02 05:02:24]

http://codereview.chromium.org/661354/diff/19/1004
File chrome/browser/cocoa/content_settings_dialog_controller.h (right):

http://codereview.chromium.org/661354/diff/19/1004#newcode34
chrome/browser/cocoa/content_settings_dialog_controller.h:34: -
(void)selectTab:(ContentSettingsType)settingsType;
Why should this be public?

http://codereview.chromium.org/661354/diff/19/1005
File chrome/browser/cocoa/content_settings_dialog_controller.mm (right):

http://codereview.chromium.org/661354/diff/19/1005#newcode137
chrome/browser/cocoa/content_settings_dialog_controller.mm:137: [self window]; 
// Make sure the nib file is loaded.
Is this really the most appropriate place to make sure the nib is loaded?

http://codereview.chromium.org/661354/diff/19/1005#newcode147
chrome/browser/cocoa/content_settings_dialog_controller.mm:147: DCHECK(index >
CONTENT_SETTINGS_TYPE_DEFAULT &&
Probably a pair of DCHECK_GT()'s (or whatever) would be better.

http://codereview.chromium.org/661354/diff/19/1005#newcode151
chrome/browser/cocoa/content_settings_dialog_controller.mm:151:
lastSelectedTab_.SetValue(index);
Or, if you're lazy, you can do |else NOTREACHED()|.

[Nico, 2010-03-02 05:06:35]

http://codereview.chromium.org/661354/diff/19/1004
File chrome/browser/cocoa/content_settings_dialog_controller.h (right):

http://codereview.chromium.org/661354/diff/19/1004#newcode34
chrome/browser/cocoa/content_settings_dialog_controller.h:34: -
(void)selectTab:(ContentSettingsType)settingsType;
On 2010/03/02 05:02:24, viettrungluu wrote:
> Why should this be public?

Done.

http://codereview.chromium.org/661354/diff/19/1005
File chrome/browser/cocoa/content_settings_dialog_controller.mm (right):

http://codereview.chromium.org/661354/diff/19/1005#newcode137
chrome/browser/cocoa/content_settings_dialog_controller.mm:137: [self window]; 
// Make sure the nib file is loaded.
On 2010/03/02 05:02:24, viettrungluu wrote:
> Is this really the most appropriate place to make sure the nib is loaded?

This function requires that it's loaded (so that tabView_ is set), so yes.

http://codereview.chromium.org/661354/diff/19/1005#newcode147
chrome/browser/cocoa/content_settings_dialog_controller.mm:147: DCHECK(index >
CONTENT_SETTINGS_TYPE_DEFAULT &&
On 2010/03/02 05:02:24, viettrungluu wrote:
> Probably a pair of DCHECK_GT()'s (or whatever) would be better.

Done.

http://codereview.chromium.org/661354/diff/19/1005#newcode151
chrome/browser/cocoa/content_settings_dialog_controller.mm:151:
lastSelectedTab_.SetValue(index);
On 2010/03/02 05:02:24, viettrungluu wrote:
> Or, if you're lazy, you can do |else NOTREACHED()|.

Done.

[viettrungluu, 2010-03-02 05:12:38]

LGTM.

http://codereview.chromium.org/661354/diff/19/1005
File chrome/browser/cocoa/content_settings_dialog_controller.mm (right):

http://codereview.chromium.org/661354/diff/19/1005#newcode151
chrome/browser/cocoa/content_settings_dialog_controller.mm:151:
lastSelectedTab_.SetValue(index);
On 2010/03/02 05:06:37, Nico wrote:
> On 2010/03/02 05:02:24, viettrungluu wrote:
> > Or, if you're lazy, you can do |else NOTREACHED()|.
> 
> Done.

Done.