New issue
Advanced search Search tips

Issue 902425 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocking:
issue 896690



Sign in to add a comment

Generalize password keyboard accessory code to also handle Autofill data

Project Member Reported by ftirelo@chromium.org, Nov 6

Issue description

Feature description: the goal is to generalize the keyboard accessory code to handle addresses and credit cards; today it only handles passwords.

The main change is in the data structure "TabData" to represent the data to be rendered on the bottom sheet (see attached image), that will replace password_manager::AccessoryItem (C++)/KeyboardAccessoryData.Item (Java) used today.

This will be done in the following steps (roughly one step = one CL):

1. Introduce the new type in C++ and Java and use it in the backend code. As a simplification, we will send a boolean "is_password" instead of the field type. The bridge will translated from the new data structure to AccessoryItem, so that this CL will be more contained.

2. Replace all occurrences of KeyboardAccessoryItem.Item with TabData in the Java code and delete KeyboardAccessoryItem.Item.

3. Replace is_password with the field type in UserInfo.Field.

4. Generalize favicon provider to also handle credit card static assets.

5. Define an enum for the footer command type.

6. Add the UserInfo.Row class, to allow multiple fields to be grouped into a single row.

Eng owner: ftirelo
Product owner: durgapandey

Design doc: go/autofill-manual-fallback-doc

Are you planning on experimenting before launch? No
Any new strings? No
Any implications for Google webservices (i.e. sync, translate)? No
Binary size? No
Do the existing perf tests exercise all aspects of your new feature(s)?  No

 
data-diagram.png
101 KB View Download
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 9

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

commit bdc08330fa2d9707793b4266b094baba8c0ea106
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Fri Nov 09 22:13:36 2018

[Mfill Android] Generalize data passed to password keyboard accessory

This CL is the first step in the refactoring and:
 - Adds a new class (TabData) to represent the data in both C++ and
   Java, which replaces the AccessoryItem/KeyboardAccessoryData.Item
   classes.
 - Replaces AccessoryItem with TabData in the back end and sends it
   to the Java frontend.
 - Converts from TabData to KeyboardAccessoryData.Item in the Java
   frontend, so we don't need to propagate these changes everywhere.

A follow-up CL will completely replace KeyboardAccessoryData.Item
with TabData and remove the conversion listed in the last step above.

Please check the linked bug for the desired final state as well as
the steps of this refactoring to be sent in follow-up CLs.

Bug: 902425
Change-Id: I6c28a31c682176c2409d3e3df2f6604768aed16d
Reviewed-on: https://chromium-review.googlesource.com/c/1320732
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Friedrich Horschig [CET] <fhorschig@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606992}
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/chrome/browser/password_manager/password_accessory_view_interface.h
[modify] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/components/autofill/core/browser/BUILD.gn
[add] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/components/autofill/core/browser/accessory_sheet_data.cc
[add] https://crrev.com/bdc08330fa2d9707793b4266b094baba8c0ea106/components/autofill/core/browser/accessory_sheet_data.h

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 13

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

commit 7c346a904ab017c2f7b21f541ed338bd8503e7d9
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Tue Nov 13 17:03:02 2018

[Mfill Android] Rename UserInfo::Field::is_password to is_obfuscated

This is another step to generalize the data sent to the front end for
the accessory sheet, to enable sending data for profiles and credit
cards.

Please check the linked bug for the desired final state as well as
the steps of this refactoring to be sent in follow-up CLs.

Bug: 902425
Change-Id: I240da77100748a337b889bff7b4be0a7c82404f4
Reviewed-on: https://chromium-review.googlesource.com/c/1330694
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Friedrich Horschig [CET] <fhorschig@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607624}
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewBinder.java
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/chrome/browser/password_manager/password_accessory_view_interface.h
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/components/autofill/core/browser/accessory_sheet_data.cc
[modify] https://crrev.com/7c346a904ab017c2f7b21f541ed338bd8503e7d9/components/autofill/core/browser/accessory_sheet_data.h

Sign in to add a comment