Fixed error message bubbles according to the latest mocks

Fixed error message bubbles according to the latest mocks

BUG=chromium-os:5762, chromium-os:5761
TEST=Enter wrong password on Chrome OS login screen.

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

[sky, 2010-08-20 15:51:12]

http://codereview.chromium.org/3201003/diff/1/3
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3201003/diff/1/3#newcode198
chrome/browser/chromeos/login/existing_user_controller.cc:198: if
(selected_view_index_ == static_cast<size_t>(i - controllers_.begin())) {
I don't think you need the static_cast here.

http://codereview.chromium.org/3201003/diff/1/3#newcode374
chrome/browser/chromeos/login/existing_user_controller.cc:374: if
(num_login_attempts_ > static_cast<size_t>(1))
Do you need the static_cast here?

http://codereview.chromium.org/3201003/diff/1/4
File chrome/browser/chromeos/login/existing_user_controller.h (right):

http://codereview.chromium.org/3201003/diff/1/4#newcode145
chrome/browser/chromeos/login/existing_user_controller.h:145: // Login attempts,
needs to show help link on n>1 unsuccessful logins.
nit: Number of login attempts. Used to show help link when > 1 un...

http://codereview.chromium.org/3201003/diff/1/6
File chrome/browser/chromeos/login/helper.h (right):

http://codereview.chromium.org/3201003/diff/1/6#newcode11
chrome/browser/chromeos/login/helper.h:11: #include "gfx/rect.h"
forward declare rect and gurl.

http://codereview.chromium.org/3201003/diff/1/9
File chrome/browser/chromeos/login/message_bubble.cc (right):

http://codereview.chromium.org/3201003/diff/1/9#newcode26
chrome/browser/chromeos/login/message_bubble.cc:26: parent_(parent),
Initialize help_link_ to NULL.

http://codereview.chromium.org/3201003/diff/1/10
File chrome/browser/chromeos/login/message_bubble.h (right):

http://codereview.chromium.org/3201003/diff/1/10#newcode80
chrome/browser/chromeos/login/message_bubble.h:80: const std::wstring& text,
const std::wstring& help,
When you wrap you should have one param per line (see thread named 'Function
argument indentation and wrapping').

http://codereview.chromium.org/3201003/diff/1/11
File chrome/browser/chromeos/login/new_user_view.cc (right):

http://codereview.chromium.org/3201003/diff/1/11#newcode386
chrome/browser/chromeos/login/new_user_view.cc:386: return screen_bounds;
Shorten this to:

gfx::Point origin;
views::View::ConvertPointToScreen(username_field_, &origin);
return gfx::Rect(origin, username_field_->size());

and extract a function that both GetPasswordBounds and GetUsernameBounds use.

http://codereview.chromium.org/3201003/diff/1/15
File chrome/browser/chromeos/login/user_controller.cc (right):

http://codereview.chromium.org/3201003/diff/1/15#newcode393
chrome/browser/chromeos/login/user_controller.cc:393: gfx::Rect
screen_bounds(existing_user_view_->password_field()->bounds());
Yow, what a lot of code duplication. Extract the method I mentioned in
NewUserView and put it on View so we don't duplicate this every where.

[Dmitry Polukhin, 2010-08-23 10:30:03]

http://codereview.chromium.org/3201003/diff/1/3
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3201003/diff/1/3#newcode198
chrome/browser/chromeos/login/existing_user_controller.cc:198: if
(selected_view_index_ == static_cast<size_t>(i - controllers_.begin())) {
On 2010/08/20 15:51:12, sky wrote:
> I don't think you need the static_cast here.

I agree but GCC complains:
chrome/browser/chromeos/login/existing_user_controller.cc:198: error: comparison
between signed and unsigned integer expressions

It is absolutely safe in but I have to add static_cast or some other trick to
inhibit the warning.

http://codereview.chromium.org/3201003/diff/1/3#newcode374
chrome/browser/chromeos/login/existing_user_controller.cc:374: if
(num_login_attempts_ > static_cast<size_t>(1))
On 2010/08/20 15:51:12, sky wrote:
> Do you need the static_cast here?

Same here.

http://codereview.chromium.org/3201003/diff/1/4
File chrome/browser/chromeos/login/existing_user_controller.h (right):

http://codereview.chromium.org/3201003/diff/1/4#newcode145
chrome/browser/chromeos/login/existing_user_controller.h:145: // Login attempts,
needs to show help link on n>1 unsuccessful logins.
On 2010/08/20 15:51:12, sky wrote:
> nit: Number of login attempts. Used to show help link when > 1 un...

Done.

http://codereview.chromium.org/3201003/diff/1/6
File chrome/browser/chromeos/login/helper.h (right):

http://codereview.chromium.org/3201003/diff/1/6#newcode11
chrome/browser/chromeos/login/helper.h:11: #include "gfx/rect.h"
On 2010/08/20 15:51:12, sky wrote:
> forward declare rect and gurl.

Done.

http://codereview.chromium.org/3201003/diff/1/9
File chrome/browser/chromeos/login/message_bubble.cc (right):

http://codereview.chromium.org/3201003/diff/1/9#newcode26
chrome/browser/chromeos/login/message_bubble.cc:26: parent_(parent),
On 2010/08/20 15:51:12, sky wrote:
> Initialize help_link_ to NULL.

Done.

http://codereview.chromium.org/3201003/diff/1/10
File chrome/browser/chromeos/login/message_bubble.h (right):

http://codereview.chromium.org/3201003/diff/1/10#newcode80
chrome/browser/chromeos/login/message_bubble.h:80: const std::wstring& text,
const std::wstring& help,
On 2010/08/20 15:51:12, sky wrote:
> When you wrap you should have one param per line (see thread named 'Function
> argument indentation and wrapping').

Done.

http://codereview.chromium.org/3201003/diff/1/11
File chrome/browser/chromeos/login/new_user_view.cc (right):

http://codereview.chromium.org/3201003/diff/1/11#newcode386
chrome/browser/chromeos/login/new_user_view.cc:386: return screen_bounds;
On 2010/08/20 15:51:12, sky wrote:
> Shorten this to:
> 
> gfx::Point origin;
> views::View::ConvertPointToScreen(username_field_, &origin);
> return gfx::Rect(origin, username_field_->size());
> 
> and extract a function that both GetPasswordBounds and GetUsernameBounds use.

Done.

http://codereview.chromium.org/3201003/diff/1/15
File chrome/browser/chromeos/login/user_controller.cc (right):

http://codereview.chromium.org/3201003/diff/1/15#newcode393
chrome/browser/chromeos/login/user_controller.cc:393: gfx::Rect
screen_bounds(existing_user_view_->password_field()->bounds());
On 2010/08/20 15:51:12, sky wrote:
> Yow, what a lot of code duplication. Extract the method I mentioned in
> NewUserView and put it on View so we don't duplicate this every where.

Done.

[sky, 2010-08-23 15:36:43]

LGTM