[Windows] Bookmark-menu button should be themed like any other menu button....

[Windows] Bookmark-menu button should be themed like any other menu button.
Command:
$ chrome --bookmark-menu

BUG=18954
TEST=open chrome/chromium with --bookmark-menu switch in the command line, apply a theme, see if the bookmark-menu
is themed? Reset to default theme, see if the bookmark-menu come backs to default state.

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

[tfarina (gmail-do not use), 2009-09-11 21:31:23]

Hi sky, could you review?

[sky, 2009-09-11 21:51:09]

http://codereview.chromium.org/203029/diff/1/5
File chrome/browser/views/bookmark_menu_button.cc (right):

http://codereview.chromium.org/203029/diff/1/5#newcode174
Line 174: void BookmarkMenuButton::ApplyTheme() {
Definition needs to match position of declaration.

http://codereview.chromium.org/203029/diff/1/5#newcode175
Line 175: if (browser_ == NULL)
Why the null check? The constructor doesn't do that.

http://codereview.chromium.org/203029/diff/1/3
File chrome/browser/views/toolbar_view.cc (right):

http://codereview.chromium.org/203029/diff/1/3#newcode987
Line 987: if (bookmark_menu_ != NULL) {
Why following a different pattern with BookmarkMenu then the rest? That is, you
should set the icon using SetIcon and not add ApplyTheme.

http://codereview.chromium.org/203029/diff/1/2
File chrome/browser/views/toolbar_view.h (right):

http://codereview.chromium.org/203029/diff/1/2#newcode226
Line 226: BookmarkMenuButton* bookmark_menu_;
Keep this as MenuButton.

[tfarina (gmail-do not use), 2009-09-12 18:44:25]

Sky thanks for the comments. Please have another look.

[sky, 2009-09-14 17:53:56]

http://codereview.chromium.org/203029/diff/3002/3004
File chrome/browser/views/bookmark_menu_button.cc (right):

http://codereview.chromium.org/203029/diff/3002/3004#newcode32
Line 32: ThemeProvider* tp = browser_->profile()->GetThemeProvider();
include theme_provider.h

http://codereview.chromium.org/203029/diff/3002/3003
File chrome/browser/views/toolbar_view.cc (right):

http://codereview.chromium.org/203029/diff/3002/3003#newcode988
Line 988: if (bookmark_menu_ != NULL) {
nit: no {}

[tfarina (gmail-do not use), 2009-09-14 18:17:42]

http://codereview.chromium.org/203029/diff/3002/3004
File chrome/browser/views/bookmark_menu_button.cc (right):

http://codereview.chromium.org/203029/diff/3002/3004#newcode32
Line 32: ThemeProvider* tp = browser_->profile()->GetThemeProvider();
On 2009/09/14 17:53:56, sky wrote:
> include theme_provider.h
Included

http://codereview.chromium.org/203029/diff/3002/3003
File chrome/browser/views/toolbar_view.cc (right):

http://codereview.chromium.org/203029/diff/3002/3003#newcode988
Line 988: if (bookmark_menu_ != NULL) {
On 2009/09/14 17:53:56, sky wrote:
> nit: no {}

Done.

[sky, 2009-09-14 18:40:16]

LGTM

[tfarina (gmail-do not use), 2009-09-17 15:46:34]

On 2009/09/14 18:40:16, sky wrote:
> LGTM
Hi sky, can you land this to me?

[sky, 2009-09-17 23:35:18]

It's been a bad day for the tree. I'll land it for you tomorrow.

  -Scott

On Thu, Sep 17, 2009 at 8:46 AM,  <thiago.farina@gmail.com> wrote:
> On 2009/09/14 18:40:16, sky wrote:
>>
>> LGTM
>
> Hi sky, can you land this to me?
>
>
> http://codereview.chromium.org/203029
>

[sky, 2009-09-18 13:54:31]

Landed in r26563.

  -Scott

On Thu, Sep 17, 2009 at 4:34 PM, Scott Violet <sky@chromium.org> wrote:
> It's been a bad day for the tree. I'll land it for you tomorrow.
>
> =A0-Scott
>
> On Thu, Sep 17, 2009 at 8:46 AM, =A0<thiago.farina@gmail.com> wrote:
>> On 2009/09/14 18:40:16, sky wrote:
>>>
>>> LGTM
>>
>> Hi sky, can you land this to me?
>>
>>
>> http://codereview.chromium.org/203029
>>
>