Get rid of non-functional elements from the themes prefs; add switcher....

Get rid of non-functional elements from the themes prefs; add switcher.

BUG=http://crbug.com/18094
TEST=try the theme picker and enjoy!


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

[Avi (use Gerrit), 2009-07-31 21:03:17]

Alas, relies on http://codereview.chromium.org/159705

[stuartmorgan, 2009-07-31 21:28:58]

http://codereview.chromium.org/159727/diff/1/5
File chrome/browser/cocoa/autoseparating_menu.h (right):

http://codereview.chromium.org/159727/diff/1/5#newcode7
Line 7: @interface AutoseparatingMenu : NSMenu {
Needs a class-level comment.

http://codereview.chromium.org/159727/diff/1/4
File chrome/browser/cocoa/preferences_window_controller.mm (right):

http://codereview.chromium.org/159727/diff/1/4#newcode658
Line 658: NSMutableArray* themeIds = [[NSMutableArray array] retain];
I had to read down a bunch to see why this wasn't wrong; it would be clearer as
themeIds_.reset([themeIds retain]).

http://codereview.chromium.org/159727/diff/1/4#newcode682
Line 682: return [themeIds_.get() indexOfObject:themeId] + 2;  // 2 = Default +
-
Having this "2" spread out across three methods makes me nervous. Seems like
this might be less ugly using the old fashioned approach, where you could just
ask the sender for the ID, but I guess bindings are the new hotness.

How about at least having an indexOffset_ member that is set when you add the
extra bits to the menu and used for the calculations?

[Avi (use Gerrit), 2009-08-03 15:29:16]

http://codereview.chromium.org/159727/diff/1/5
File chrome/browser/cocoa/autoseparating_menu.h (right):

http://codereview.chromium.org/159727/diff/1/5#newcode7
Line 7: @interface AutoseparatingMenu : NSMenu {
On 2009/07/31 21:28:59, stuartmorgan wrote:
> Needs a class-level comment.

Done.

http://codereview.chromium.org/159727/diff/1/4
File chrome/browser/cocoa/preferences_window_controller.mm (right):

http://codereview.chromium.org/159727/diff/1/4#newcode658
Line 658: NSMutableArray* themeIds = [[NSMutableArray array] retain];
On 2009/07/31 21:28:59, stuartmorgan wrote:
> I had to read down a bunch to see why this wasn't wrong; it would be clearer
as
> themeIds_.reset([themeIds retain]).

Done.

http://codereview.chromium.org/159727/diff/1/4#newcode682
Line 682: return [themeIds_.get() indexOfObject:themeId] + 2;  // 2 = Default +
-
On 2009/07/31 21:28:59, stuartmorgan wrote:
> Having this "2" spread out across three methods makes me nervous. Seems like
> this might be less ugly using the old fashioned approach, where you could just
> ask the sender for the ID, but I guess bindings are the new hotness.
> 
> How about at least having an indexOffset_ member that is set when you add the
> extra bits to the menu and used for the calculations?

Done.

[stuartmorgan, 2009-08-03 17:01:36]

LGTM