Change password form UI for no accounts stored case

Change password form UI for no accounts stored case

 This is UI for saving password on change password form when there is no username on a change password form and no accounts in the PasswordManager. It looks like in mocks
https://folio.googleplex.com/chrome-ux/mocks/321-password-manager/password%20update#%2F00%20password%20update%20bubbles%20-%20UI%20review.png%3Fc=show  (case 1)

BUG=359315
Committed: https://crrev.com/cc79a33d41524a7609d78bd5ba77837992cbe6c5
Cr-Commit-Position: refs/heads/master@{#342338}

[dvadym, 2015-08-05 12:40:13]

dvadym@chromium.org changed reviewers:
+ vasilii@chromium.org

[dvadym, 2015-08-05 12:40:13]

Hi Vasilii,

 Could you please review this CL?

Best regards,
Vadym

[vasilii, 2015-08-05 13:47:15]

https://codereview.chromium.org/1271283002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/1271283002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:916:
pending_credentials_.new_password_element.clear();
I think you can just remove these 2 lines.

[dvadym, 2015-08-05 16:12:59]

Thanks!
 I've changed UI according to discussion with UX to do not to show
username/password line at all.

https://codereview.chromium.org/1271283002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/1271283002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:916:
pending_credentials_.new_password_element.clear();
On 2015/08/05 13:47:14, vasilii wrote:
> I think you can just remove these 2 lines.

Done.

[vasilii, 2015-08-05 16:28:26]

https://codereview.chromium.org/1271283002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc (right):

https://codereview.chromium.org/1271283002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:425: if
(!parent->model()
Please no new line here.

https://codereview.chromium.org/1271283002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:431: new
ManagePasswordItemsView(parent_->model(), credentials);
Can you split this and initialize |item| above?

[dvadym, 2015-08-05 16:51:45]

Thanks

https://codereview.chromium.org/1271283002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc (right):

https://codereview.chromium.org/1271283002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:425: if
(!parent->model()
On 2015/08/05 16:28:26, vasilii wrote:
> Please no new line here.

Done.

https://codereview.chromium.org/1271283002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:431: new
ManagePasswordItemsView(parent_->model(), credentials);
On 2015/08/05 16:28:26, vasilii wrote:
> Can you split this and initialize |item| above?

Done.

[vasilii, 2015-08-05 17:22:18]

https://codereview.chromium.org/1271283002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc (right):

https://codereview.chromium.org/1271283002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:399:
.IsPossibleChangePasswordFormWithoutUsername()) {
4 space indent should be right

https://codereview.chromium.org/1271283002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:1034: item =
new ManagePasswordItemsView(parent_->model(), forms);
This block and below don't belong to this CL.

[dvadym, 2015-08-06 08:15:59]

Thanks, PTAL

https://codereview.chromium.org/1271283002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc (right):

https://codereview.chromium.org/1271283002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:399:
.IsPossibleChangePasswordFormWithoutUsername()) {
On 2015/08/05 17:22:18, vasilii wrote:
> 4 space indent should be right

Done.

https://codereview.chromium.org/1271283002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:1034: item =
new ManagePasswordItemsView(parent_->model(), forms);
On 2015/08/05 17:22:17, vasilii wrote:
> This block and below don't belong to this CL.

Merged

[vasilii, 2015-08-06 08:45:02]

lgtm

[dvadym, 2015-08-06 15:01:32]

The CQ bit was checked by dvadym@chromium.org

[commit-bot: I haz the power, 2015-08-06 15:01:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1271283002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1271283002/100001

[commit-bot: I haz the power, 2015-08-06 15:15:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-06 15:15:51]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[dvadym, 2015-08-07 09:40:12]

Hi Vasilii,

 I've fixed unittests, it turned out that they were dependent on
new_password_element. PTAL.

[vasilii, 2015-08-07 09:51:44]

lgtm

[dvadym, 2015-08-07 10:29:27]

The CQ bit was checked by dvadym@chromium.org

[commit-bot: I haz the power, 2015-08-07 10:29:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1271283002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1271283002/120001

[commit-bot: I haz the power, 2015-08-07 11:02:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-07 11:02:13]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dvadym, 2015-08-07 11:04:55]

The CQ bit was checked by dvadym@chromium.org

[commit-bot: I haz the power, 2015-08-07 11:05:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1271283002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1271283002/120001

[commit-bot: I haz the power, 2015-08-07 12:17:54]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-08-07 12:18:28]

Patchset 7 (id:??) landed as
https://crrev.com/cc79a33d41524a7609d78bd5ba77837992cbe6c5
Cr-Commit-Position: refs/heads/master@{#342338}