New Blue style for for autofill Info-bar, to be eventually used for all info ...

New Blue style for for autofill Info-bar, to be eventually used for all info bars (for that more changes needed), it also Windows only.
BUG=47233
TEST=Mocks @ http://folder/roma/design/chrome/autofill/ page 23

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

[James Hawkins, 2010-07-16 21:51:12]

http://codereview.chromium.org/3027004/diff/15001/1009
File chrome/browser/autofill/autofill_cc_infobar_win.cc (right):

http://codereview.chromium.org/3027004/diff/15001/1009#newcode35
chrome/browser/autofill/autofill_cc_infobar_win.cc:35: views::View* parent,
nit: These two params should line up with 'bool is_add,'

http://codereview.chromium.org/3027004/diff/15001/1009#newcode50
chrome/browser/autofill/autofill_cc_infobar_win.cc:50: views::TextButton*
save_button_;
scoped_ptr.

http://codereview.chromium.org/3027004/diff/15001/1009#newcode126
chrome/browser/autofill/autofill_cc_infobar_win.cc:126: RemoveInfoBar();
The InfoBar should not go away if the user clicks the Learn More link.

http://codereview.chromium.org/3027004/diff/15001/1009#newcode153
chrome/browser/autofill/autofill_cc_infobar_win.cc:153: return std::max(0,
link_->x() - (save_button_->GetPreferredSize().width() +
This would be much more readable if you broke up the calculation into steps.

[GeorgeY, 2010-07-16 21:59:40]

http://codereview.chromium.org/3027004/diff/15001/1009
File chrome/browser/autofill/autofill_cc_infobar_win.cc (right):

http://codereview.chromium.org/3027004/diff/15001/1009#newcode35
chrome/browser/autofill/autofill_cc_infobar_win.cc:35: views::View* parent,
On 2010/07/16 21:51:12, James Hawkins wrote:
> nit: These two params should line up with 'bool is_add,'

Done.

http://codereview.chromium.org/3027004/diff/15001/1009#newcode50
chrome/browser/autofill/autofill_cc_infobar_win.cc:50: views::TextButton*
save_button_;
On 2010/07/16 21:51:12, James Hawkins wrote:
> scoped_ptr.

No, its ownership is transferred to the view as soon as AddChildView is called
and we still need a pointer to it, so we cannot do detach().

http://codereview.chromium.org/3027004/diff/15001/1009#newcode126
chrome/browser/autofill/autofill_cc_infobar_win.cc:126: RemoveInfoBar();
On 2010/07/16 21:51:12, James Hawkins wrote:
> The InfoBar should not go away if the user clicks the Learn More link.

Done.

http://codereview.chromium.org/3027004/diff/15001/1009#newcode153
chrome/browser/autofill/autofill_cc_infobar_win.cc:153: return std::max(0,
link_->x() - (save_button_->GetPreferredSize().width() +
On 2010/07/16 21:51:12, James Hawkins wrote:
> This would be much more readable if you broke up the calculation into steps.

Done.

[James Hawkins, 2010-07-16 22:04:32]

http://codereview.chromium.org/3027004/diff/15001/1009
File chrome/browser/autofill/autofill_cc_infobar_win.cc (right):

http://codereview.chromium.org/3027004/diff/15001/1009#newcode50
chrome/browser/autofill/autofill_cc_infobar_win.cc:50: views::TextButton*
save_button_;
On 2010/07/16 21:59:40, GeorgeY wrote:
> On 2010/07/16 21:51:12, James Hawkins wrote:
> > scoped_ptr.
> 
> No, its ownership is transferred to the view as soon as AddChildView is called
> and we still need a pointer to it, so we cannot do detach().

Ah that's unfortunate. Can you add a comment stating that the view (or whatever)
ends up owning these?

[GeorgeY, 2010-07-16 22:16:48]

On 2010/07/16 22:04:32, James Hawkins wrote:
> http://codereview.chromium.org/3027004/diff/15001/1009
> File chrome/browser/autofill/autofill_cc_infobar_win.cc (right):
> 
> http://codereview.chromium.org/3027004/diff/15001/1009#newcode50
> chrome/browser/autofill/autofill_cc_infobar_win.cc:50: views::TextButton*
> save_button_;
> On 2010/07/16 21:59:40, GeorgeY wrote:
> > On 2010/07/16 21:51:12, James Hawkins wrote:
> > > scoped_ptr.
> > 
> > No, its ownership is transferred to the view as soon as AddChildView is
called
> > and we still need a pointer to it, so we cannot do detach().
> 
> Ah that's unfortunate. Can you add a comment stating that the view (or
whatever)
> ends up owning these?

done.

[James Hawkins, 2010-07-16 22:18:30]

LGTM with comment fix.

http://codereview.chromium.org/3027004/diff/24001/5007
File chrome/browser/autofill/autofill_cc_infobar_win.cc (right):

http://codereview.chromium.org/3027004/diff/24001/5007#newcode50
chrome/browser/autofill/autofill_cc_infobar_win.cc:50: // The buttons are owned
by InfoBar view from the moment they added to its
they are added*

[GeorgeY, 2010-07-16 22:37:05]

http://codereview.chromium.org/3027004/diff/24001/5007
File chrome/browser/autofill/autofill_cc_infobar_win.cc (right):

http://codereview.chromium.org/3027004/diff/24001/5007#newcode50
chrome/browser/autofill/autofill_cc_infobar_win.cc:50: // The buttons are owned
by InfoBar view from the moment they added to its
On 2010/07/16 22:18:30, James Hawkins wrote:
> they are added*

Done.