New issue
Advanced search Search tips

Issue 845458 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocking:
issue 835234



Sign in to add a comment

Refactor the password generation UI flows

Project Member Reported by ioanap@chromium.org, May 22 2018

Issue description

This includes:
  - a clear delimitation between the automatic and manual flows
  - moving UI display responsibility from renderer to browser

The refactoring aims to make it possible for the new Android password generation UI (automatic and manual) to use the same flows as the desktop UI.
 

Comment 1 by ioanap@chromium.org, May 22 2018

Description: Show this description

Comment 2 by ioanap@chromium.org, May 22 2018

Blocking: 835234
Components: -UI>Browser>Passwords UI>Browser>Passwords>Generation
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7999140d9eda18e1536b5e875b58c57497d1639a

commit 7999140d9eda18e1536b5e875b58c57497d1639a
Author: Ioana Pandele <ioanap@chromium.org>
Date: Tue Jun 05 14:50:57 2018

Refactor automatic generation UI flow

This aims to make the flow easier to use by the new password generation
Android UI by decoupling the renderer logic in PasswordGenerationAgent
from the UI display logic.

A follow-up CL will refactor the manual generation flow as well.

Bug:845458

Change-Id: I441546003cf3afcaa2de6ff84ccbcee6f131d150
Reviewed-on: https://chromium-review.googlesource.com/1044372
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564494}
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/chrome/renderer/autofill/fake_password_manager_client.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/chrome/renderer/autofill/fake_password_manager_client.h
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/BUILD.gn
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/autofill_types_struct_traits_unittest.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/common/test_autofill_types.mojom
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/content/renderer/password_generation_agent.h
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/core/common/BUILD.gn
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/core/common/password_generation_util.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/core/common/password_generation_util.h
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/core/common/save_password_progress_logger.cc
[modify] https://crrev.com/7999140d9eda18e1536b5e875b58c57497d1639a/components/autofill/core/common/save_password_progress_logger.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7

commit 09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7
Author: Ioana Pandele <ioanap@chromium.org>
Date: Tue Jun 05 15:02:51 2018

Rename HidePopup to GenerationRejectedByTyping in PasswordGenerationAgent

The renderer should only notify the browser about the event, rather than
directly control what happens with the UI. The browser decides which UI
to hide if any, as this might be different on different platforms.

Bug: 845458
Change-Id: I1feed186a5726a6c490b6fd995b084858246ed3c
Reviewed-on: https://chromium-review.googlesource.com/1075529
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564497}
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/chrome/renderer/autofill/fake_password_manager_client.cc
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/chrome/renderer/autofill/fake_password_manager_client.h
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/09ec7a22025ad5c005b0fad9ec5b7541e22a1ae7/components/autofill/content/renderer/password_generation_agent.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6

commit d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Tue Jun 05 16:01:53 2018

[Android] Implement bridge for password accessory

This CL adds the JNI classes to wire the communication between
native code and UI code of the password-specific accessory.

Testing is not really possible:
1. On the native side, verifying interacting with the UI isn't
really doable (esp. because this CL makes no UI changes yet).
2. On the UI-side, we don't currently have a way to set up a
synced profile to save and provide passwords.

I am open to any good idea which extends the integration
tests in the follow-up CL.

Bug: 811747, 845458
Change-Id: I10775bb92d576bcaab0f9c998d64f3377a713707
Reviewed-on: https://chromium-review.googlesource.com/1068671
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564520}
[add] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/android/java_sources.gni
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/BUILD.gn
[add] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/android/password_manager/OWNERS
[add] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/android/password_manager/password_accessory_view_android.cc
[add] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/d97145ecaeb8d4a36c46f226ddec04ec9ede6dd6/chrome/browser/password_manager/password_accessory_view_interface.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f85f6777d010bd4bbe7ff059888afad87b3971e

commit 3f85f6777d010bd4bbe7ff059888afad87b3971e
Author: Ioana Pandele <ioanap@chromium.org>
Date: Wed Jun 13 16:44:55 2018

Don't re-trigger generation popup for manual generation

Previously, the generation popup used to be triggered again for a
field where manual generation was requested when either the password
was completely deleted or the focus was changed from the field back
again.

This CL removes the re-trigger for the popup for fields where manual
generation was requested.

Bug: 845458
Change-Id: I955802666faab62f4623ab56fc3a956b97b2cfd9
Reviewed-on: https://chromium-review.googlesource.com/1088619
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566866}
[modify] https://crrev.com/3f85f6777d010bd4bbe7ff059888afad87b3971e/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/3f85f6777d010bd4bbe7ff059888afad87b3971e/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/3f85f6777d010bd4bbe7ff059888afad87b3971e/components/autofill/content/renderer/password_generation_agent.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227

commit 2d6c15d8e43fd1a2a5f24cec2c160f79cb044227
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Wed Nov 07 13:38:13 2018

Delete "local-heuristics-only-for-password-generation" flag.

The flag was used only in the tests and had a dedicated production code.

Bug: 845458
Change-Id: I27265ad5a69eecb31663b5fb1526c696f1b35d3d
Reviewed-on: https://chromium-review.googlesource.com/c/1322831
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606027}
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/chrome/browser/password_manager/password_generation_interactive_uitest.cc
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/chrome/browser/password_manager/password_manager_test_base.cc
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/chrome/test/data/password/framed_signup_form.html
[add] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/chrome/test/data/password/signup_form_new_password.html
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/components/autofill/core/common/autofill_switches.cc
[modify] https://crrev.com/2d6c15d8e43fd1a2a5f24cec2c160f79cb044227/components/autofill/core/common/autofill_switches.h

Sign in to add a comment