Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(11)

Issue 2896533002: cros: Simple password view for lock. Adds test support.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week ago by jdufault
Modified:
1 day, 11 hours ago
CC:
chromium-reviews, kalyank, sadrul
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

cros: Simple password view for lock. Adds test support. This CL primarily adds test support code; the password view is still WIP. BUG=719015

Patch Set 1 : Initial upload #

Total comments: 11
Unified diffs Side-by-side diffs Delta from patch set Stats (+450 lines, -0 lines) Patch
M ash/BUILD.gn View 3 chunks +6 lines, -0 lines 0 comments Download
A ash/login/ui/login_password_view.h View 1 chunk +65 lines, -0 lines 0 comments Download
A ash/login/ui/login_password_view.cc View 1 chunk +137 lines, -0 lines 4 comments Download
A ash/login/ui/login_password_view_test.cc View 1 chunk +113 lines, -0 lines 5 comments Download
A ash/login/ui/login_test_base.h View 1 chunk +54 lines, -0 lines 0 comments Download
A ash/login/ui/login_test_base.cc View 1 chunk +69 lines, -0 lines 2 comments Download
M ash/system/tray/size_range_layout.h View 1 chunk +3 lines, -0 lines 0 comments Download
M ash/system/tray/size_range_layout.cc View 1 chunk +3 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 23 (21 generated)
jdufault
jamescook@, xiyuan@, PTAL. The whole point of this CL is to establish a test fixture ...
1 day, 12 hours ago (2017-05-24 19:05:11 UTC) #22
xiyuan
1 day, 11 hours ago (2017-05-24 20:11:03 UTC) #23
https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
File ash/login/ui/login_password_view.cc (right):

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view.cc:24: const int kSubmitButtonWidthDp = 20;
nit: const -> constexpr ?

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view.cc:76: textfield_->SetLayoutManager(new
views::FillLayout());
Should this be set to |textfield_sizer | instead?

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view.cc:127: SubmitPassword();
nit: DCHCKE_EQ(sender, submit_button_)

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view.cc:134: textfield_->SchedulePaint();
SchedulePaint() is probably not needed since SetText should trigger a paint.

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
File ash/login/ui/login_password_view_test.cc (right):

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view_test.cc:11: #include "ash/test/ash_test_base.h"
nit: unused?

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view_test.cc:13: #include "ui/aura/window.h"
nit: unused?

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view_test.cc:15: #include "ui/views/view.h"
nit: unused?

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view_test.cc:16: #include "ui/views/widget/widget.h"
nit: unused?

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_pass...
ash/login/ui/login_password_view_test.cc:41: LoginPasswordView* view = nullptr;
Put member vars in protected. And view -> view_, password -> password_.

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_test...
File ash/login/ui/login_test_base.cc (right):

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_test...
ash/login/ui/login_test_base.cc:30: views::View* initially_focused_;
nit: IMHO, it is easier to read if this is called |content_|.

https://codereview.chromium.org/2896533002/diff/60001/ash/login/ui/login_test...
ash/login/ui/login_test_base.cc:57: fake_user_manager_ =
base::MakeUnique<user_manager::FakeUserManager>();
I don't think we will have a UserManager running inside ash. Everything ash
knows about user/session should go through SessionController.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06