New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
50% in 2018
Closed: Mar 21
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature

Blocked on:
issue 800686
issue 811307
issue 811312
issue 811317
issue 817370

Blocking:
issue 341477
issue 790993



Sign in to add a comment

Android implementation of password export

Project Member Reported by vabr@chromium.org, Nov 27 2017

Issue description

Design: https://docs.google.com/a/chromium.org/document/d/1miKr2x0PTNIKgt3RQeur51ICQ66uszlYAmFXJONbG_0/edit?usp=sharing

Mocks: go chrome-pwd-export-mocks-android (Google-internal only)
 

Comment 1 by vabr@chromium.org, Nov 27 2017

Adding screenshots for https://crrev.com/c/776681
menu-closed.png
80.8 KB View Download
menu-open.png
84.9 KB View Download
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28 2017

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

commit dcd1fe27c6a6d37766c55a0a26b6db7f655ecd39
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Nov 28 08:29:05 2017

[Android settings] Add a menu item to export passwords

This CL adds a menu item, behind an off-by-default feature, for
exporting passwords from Chrome's settings. The menu item
currently does not perform any action.

Design: https://docs.google.com/a/chromium.org/document/d/1miKr2x0PTNIKgt3RQeur51ICQ66uszlYAmFXJONbG_0/edit?usp=sharing

Mocks: go chrome-pwd-export-mocks-android (Google-internal only)

Screenshots:  https://crbug.com/788701#c1 

Bug:  788701 
Change-Id: I7e8b515596db4be061d494019a34220b9c019caf
Reviewed-on: https://chromium-review.googlesource.com/776681
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519607}
[add] https://crrev.com/dcd1fe27c6a6d37766c55a0a26b6db7f655ecd39/chrome/android/java/res/menu/save_password_preferences_action_bar_menu.xml
[modify] https://crrev.com/dcd1fe27c6a6d37766c55a0a26b6db7f655ecd39/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/dcd1fe27c6a6d37766c55a0a26b6db7f655ecd39/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/dcd1fe27c6a6d37766c55a0a26b6db7f655ecd39/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/dcd1fe27c6a6d37766c55a0a26b6db7f655ecd39/chrome/browser/android/chrome_feature_list.cc

Comment 3 by vabr@chromium.org, Nov 29 2017

Adding screencasts for https://crrev.com/c/797453
no-reauth.mp4
658 KB View Download
with-reauth.mp4
1.3 MB View Download

Comment 4 by battre@chromium.org, Nov 30 2017

Maybe we can explore at some point in the future how to send users to the screen where they can turn on their screen lock. I think that many users would not know what a screen lock is and where to configure it.

Comment 5 by vabr@chromium.org, Nov 30 2017

Thanks for the suggestion in #4. I filed bug 789912 to track that.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2017

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

commit c721c7cddaa61e28a360f5942682d7417fc7c5c9
Author: Vaclav Brozek <vabr@chromium.org>
Date: Thu Nov 30 14:30:29 2017

[Android password settings] Separate reauthentication helper

Currently the logic for reauthentication is scattered over these files:

PasswordEntryEditor.java
SavePasswordsPreferences.java

where SavePasswordsPreferences take care of resetting and own the
timer for reauthentication validity (because it should be tied to the
passwords settings lifetime), and PasswordEntryEditor contains logic
to decide about reauthentication steps based on that timer and on
other device state.

This alone is already a little disorganised. In a follow-up CL, one
more callsite for the reauth logic will be added, in
SavePasswordsPreferences.java, meaning that there would be
difficulties reusing the code currently in PasswordEntryEditor.java.

For that reason, this CL introduces a new class,
ReauthenticationManager.java, which encapsulates the logic. Because
the timer has been a static value, the new class is also composed of
static methods.

Bug:  788701 
Change-Id: Iea2007fdd9c89bff77595b2cf5ea8ac4e1501792
Reviewed-on: https://chromium-review.googlesource.com/797510
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520536}
[modify] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[modify] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java
[add] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java
[modify] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/java_sources.gni
[modify] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/junit/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragmentTest.java
[add] https://crrev.com/c721c7cddaa61e28a360f5942682d7417fc7c5c9/chrome/android/junit/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManagerTest.java

Comment 7 by vabr@chromium.org, Dec 1 2017

Screenshot for https://crrev.com/c/803955.
export-warning.png
94.2 KB View Download
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 5 2017

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

commit 999f75ab54fb2cf89c57263e91260dce08aa6dd3
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Dec 05 11:15:14 2017

Extend password export/import flags to Android

The connected base::Features are also used on Android, but the
about:flags page is set up to only allow changing it on desktop.

This CL adds the possibility to control those two base::Features
through about:flags on Android as well.

Bug:  788701 
Change-Id: Ib6ae924ed453c8ceda746746fb10659d718d18e0
Reviewed-on: https://chromium-review.googlesource.com/808105
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521662}
[modify] https://crrev.com/999f75ab54fb2cf89c57263e91260dce08aa6dd3/chrome/browser/about_flags.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 5 2017

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

commit cf177bac5e2eb43f3520532c4588e9594dbe9159
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Dec 05 11:39:00 2017

Fix invalid cast in SavePasswordsPreferences#resetList

The resetList method tries to call PreferenceGroup#removeAll on
TextMessagePreference (which is not a PreferenceGroup) if an update of
the settings happens while the last state had no saved passwords to
display.

This is a rare situation because people who have no saved passwords
are not very often inspecting their passwords settings, but can happen
in reality as well (e.g., when somebody inspects the page during early
moments of syncing and Chrome sync populates the page as the user
views it).

This CL introduces a special method to do the job of resetList in the
case there are no lists, and adds a test which executed the invalid
cast before the fix.

Bug:  788701 
Change-Id: I64ada510deb78cc7b8f22f085ef2f401e9bb76ba
Reviewed-on: https://chromium-review.googlesource.com/806165
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521667}
[modify] https://crrev.com/cf177bac5e2eb43f3520532c4588e9594dbe9159/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/cf177bac5e2eb43f3520532c4588e9594dbe9159/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 5 2017

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

commit ef6138f4969956e92b9e64bb1a60645b67c42ddd
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Dec 05 15:07:27 2017

Remove dead code SavePasswordsPreferences#onPreferenceChange

SavePasswordsPreferences never registers itself as an
OnPreferenceChangeListener so its onPreferenceChange method never gets
called. This CL removes that dead code.

Note that OnPreferenceChangeListener is still used in-line, so the
import is not removed.

Bug:  788701 
Change-Id: I87ae4f6be3e2dc4322fad56551338b0a8355e062
Reviewed-on: https://chromium-review.googlesource.com/808265
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521691}
[modify] https://crrev.com/ef6138f4969956e92b9e64bb1a60645b67c42ddd/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 6 2017

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

commit 0db267e941f1c227914d20cc272617188bd3fd04
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Dec 06 10:36:25 2017

Allow mocking PasswordUIView.java

Tests are needed for viewing passwords in settings. Those tests need
an easy way to mock PasswordUIView, which communicates with C++ in the
production version.

Therefore this CL does:
(1) Define an interface, PasswordManagerHandler, for the public methods of
    PasswordUIView to be mocked in the tests (coming in
    https://crrev.com/c/797453).
(2) Create PasswordManagerHandlerProvider which encapsulates the logic
    for managing observers, life-time and test-replacement of the
    PasswordManagerHandler implementations.
(3) Lift SavedPasswordEntry to top level for reasons explained in its
    class comment.

This CL should not change any functionality, so current test coverage
is assumed to be sufficient for the new code.
The new tests are postponed until https://crrev.com/c/797453, which
also introduces other test utilities to make writing them less of a
pain.

Bug:  788701 
Change-Id: I2b90a613065df50cad85de85ffa620a88df61a6e
Reviewed-on: https://chromium-review.googlesource.com/806155
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522059}
[add] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java/src/org/chromium/chrome/browser/PasswordManagerHandler.java
[modify] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java/src/org/chromium/chrome/browser/PasswordUIView.java
[add] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java/src/org/chromium/chrome/browser/SavedPasswordEntry.java
[modify] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[add] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandlerProvider.java
[modify] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/0db267e941f1c227914d20cc272617188bd3fd04/chrome/android/java_sources.gni

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 6 2017

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

commit 06c20788cc532a7565c4004b17789ba409751de1
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Dec 06 11:12:26 2017

[Android passwords settings] Add reauthentication for export

Mocks: go chrome-pwd-export-mocks-android (Google-internal only)

Screencast after the change:  https://crbug.com/788701#c3 

The CL adds a reauthentication step before the user can trigger
exporting passwords.

In addition to that, the CL also wraps the check for screen lock
availability in ReauthenticationManager.isScreenLockSetUp, to
make it possible to override for testing.

Also, this CL adds two tests for viewing passwords, because one
of earlier iterations of this CL broke something for the viewing
passwords feature and such tests would have caught it.

Bug:  788701 
Change-Id: Ibfa37192e26bc5797236ba9378aa1eb0f17f79cc
Reviewed-on: https://chromium-review.googlesource.com/797453
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522061}
[modify] https://crrev.com/06c20788cc532a7565c4004b17789ba409751de1/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[modify] https://crrev.com/06c20788cc532a7565c4004b17789ba409751de1/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java
[modify] https://crrev.com/06c20788cc532a7565c4004b17789ba409751de1/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java
[modify] https://crrev.com/06c20788cc532a7565c4004b17789ba409751de1/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/06c20788cc532a7565c4004b17789ba409751de1/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/06c20788cc532a7565c4004b17789ba409751de1/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 6 2017

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

commit 05edf9ed8accdd886adfe88ee93b2a3bdf3a1a25
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Dec 06 11:24:30 2017

[Android password settings] Display warning before export

This CL adds a warning to be displayed before the user can export
their passwords.

Mocks:
https://docs.google.com/presentation/d/1nIm5OmaOnb85ZAwMZPSVqHT0vkFbsRYZhOzbmznWU_c/edit#slide=id.g289b1efcd8_0_21

Note: the mock shows slightly different styling, but with the UX
designer we settled on using a standard dialog style already existing
in Chromium and checking separately whether that style needs fixing,
rather than introducing inconsistent styling among Chrome's dialogs.

Design: go chrome-pwd-export

Screenshot:  https://crbug.com/788701#c7 

Bug:  788701 
Change-Id: I3a2dbdc24429c5662fdc2b178630a8e45ba603a1
Reviewed-on: https://chromium-review.googlesource.com/803955
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522062}
[add] https://crrev.com/05edf9ed8accdd886adfe88ee93b2a3bdf3a1a25/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportWarningDialogFragment.java
[modify] https://crrev.com/05edf9ed8accdd886adfe88ee93b2a3bdf3a1a25/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/05edf9ed8accdd886adfe88ee93b2a3bdf3a1a25/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/05edf9ed8accdd886adfe88ee93b2a3bdf3a1a25/chrome/android/java_sources.gni
[modify] https://crrev.com/05edf9ed8accdd886adfe88ee93b2a3bdf3a1a25/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Comment 14 by vabr@chromium.org, Dec 6 2017

Screenshots for https://crrev.com/c/811186.
export-disabled.png
66.6 KB View Download
export-enabled.png
85.6 KB View Download
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 7 2017

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

commit 065cd806af680a383ec1794f08e27702c1816980
Author: Vaclav Brozek <vabr@chromium.org>
Date: Thu Dec 07 11:10:02 2017

[Android settings] Disable password export if there are no saved passwords

If the settings do not show any saved passwords, the export button in
the menu should still be present, but disabled.

Note that the existence of "blacklisted" entries, i.e., entries where
the user asked Chrome to never save a password, does not influence
the state of the menu item, because those entries are not exported.

Mocks: go chrome-pwd-export-mocks-android (currently slide 10,
Google-internal only)

General design:
https://docs.google.com/a/chromium.org/document/d/1miKr2x0PTNIKgt3RQeur51ICQ66uszlYAmFXJONbG_0/edit?usp=sharing

Bug:  788701 
Change-Id: I1f5ad12eef9ef6ec1c9b0529c5b1ef16941c5298
Reviewed-on: https://chromium-review.googlesource.com/811186
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522400}
[modify] https://crrev.com/065cd806af680a383ec1794f08e27702c1816980/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/065cd806af680a383ec1794f08e27702c1816980/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Comment 16 by vabr@chromium.org, Dec 7 2017

Blockedon: -341477
Blocking: 341477 790993

Comment 17 by vabr@chromium.org, Dec 7 2017

Status: Assigned (was: Started)
Status update:
One CL in review: https://crrev.com/c/809127
One CL in progress: https://crrev.com/c/793838
One more task pending -- adding the progress indicator before the export intent.

Will return in 2018 to complete this.
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 17 2018

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

commit 1b9e5f74c5a02ca050e0d20c2f6c93fedc759347
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jan 17 16:09:11 2018

Fix password export warning string on Android

The mocks [1] changed and dropped "passwords" from the warning string
before exporting passwords from Chrome settings on Android. This CL
updates the code.

[1]
https://docs.google.com/presentation/d/1nIm5OmaOnb85ZAwMZPSVqHT0vkFbsRYZhOzbmznWU_c/edit?ts=5a549b02#slide=id.g289b1efcd8_0_21

Bug:  788701 
Change-Id: Id61bf4e17267cd2c4694510c2a13318e21045e3f
Reviewed-on: https://chromium-review.googlesource.com/870593
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529759}
[modify] https://crrev.com/1b9e5f74c5a02ca050e0d20c2f6c93fedc759347/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 19 2018

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

commit a4d802ebb1600f07e797cc3d3f43f4175756e6d6
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Jan 19 20:54:00 2018

Serialise passwords for export on Android

This CL prepares the C++ binding for passing exported passwords to the
Java code controlling Chrome's settings. This consists of:

(1) Extending the PasswordUIViewAndroid C++ class to handle a call to
start serialising passwords, to spawn a task to copy and serialize the
passwords on a backend task runner, and to post the result back to
Java.

(2) Teaching the Java class SavePasswordsPreferences to trigger the
serialisation when the user taps on the Export menu item.

The changes in (1) specifically make life-time management of
PasswordUIViewAndroid a little more complex: care is take that the
class always survives the backend processing. This includes
introducing a state variable and postponing self-destruction
in Destroy().

The CL also adds a whole new unittest for PasswordUIViewAndroid and
one more scenario to the existing SavePasswordsPreferencesTest. The
code in either language is tested, but passing the information through
JNI is not, because it is not clear how that would be done.

The format changes to BUILD.gn (beyond introducing the new unittest)
are unrelated, but a result of the compulsory running of git cl format.

The CL does not introduce any processing of the serialised data in the
Java code yet (it is currently dropped on receipt). That will come in
future CLs.

Bug:  788701 
Change-Id: I32bbcb4bf62883d3154b3abf81ea7b076c05e37c
Reviewed-on: https://chromium-review.googlesource.com/809127
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530603}
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/android/java/src/org/chromium/chrome/browser/PasswordManagerHandler.java
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/android/java/src/org/chromium/chrome/browser/PasswordUIView.java
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/browser/android/password_ui_view_android.cc
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/browser/android/password_ui_view_android.h
[add] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/browser/android/password_ui_view_android_unittest.cc
[modify] https://crrev.com/a4d802ebb1600f07e797cc3d3f43f4175756e6d6/chrome/test/BUILD.gn

Project Member

Comment 20 by bugdroid1@chromium.org, Jan 22 2018

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

commit 965297e8184760b122fa63b2418a670662349934
Author: Vaclav Brozek <vabr@chromium.org>
Date: Mon Jan 22 19:08:57 2018

Clean up package structure for Android Passwords settings

Also, remove unnecessary "public" from an interface definition.

This is based on comments in
https://chromium-review.googlesource.com/c/chromium/src/+/809127/3/chrome/android/java/src/org/chromium/chrome/browser/PasswordManagerHandler.java#5

Bug:  788701 
Change-Id: Iaf6f06a06fc37663161a610bcb17ffe7f9258119
Reviewed-on: https://chromium-review.googlesource.com/868018
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530934}
[modify] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[rename] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandler.java
[modify] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandlerProvider.java
[rename] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordUIView.java
[modify] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[rename] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavedPasswordEntry.java
[modify] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/java_sources.gni
[modify] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/965297e8184760b122fa63b2418a670662349934/chrome/browser/BUILD.gn

Project Member

Comment 21 by bugdroid1@chromium.org, Jan 24 2018

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

commit e39e30c7fd03ac37e4ac0909ac937ae0def2d07c
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jan 24 12:27:06 2018

[Android settings] Fire an intent for exporting passwords

This CL adds the following functionality:
* When serialized passwords arrive from the native code,
  the UI code saves them into a cache file and creates
  a content:// URI for sharing them.
* When the user finishes the current export flow and the
  serialized passwords have arrived, Chrome will fire
  an intent chooser for a sharing intent to export the
  passwords.

Notable details:
* Adding a cache-path entry in file_paths.xml for
  passwords-related files.
* Persisting the content:// URI to survive Chrome being
  killed during the export flow (which involves an
  OS-level reauthentication activity).

Bug:  788701 
Change-Id: I0ceac669d7ac453aa2dc654ce08e97287d43d856
Reviewed-on: https://chromium-review.googlesource.com/793838
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531506}
[modify] https://crrev.com/e39e30c7fd03ac37e4ac0909ac937ae0def2d07c/chrome/android/java/res/xml/file_paths.xml
[modify] https://crrev.com/e39e30c7fd03ac37e4ac0909ac937ae0def2d07c/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/e39e30c7fd03ac37e4ac0909ac937ae0def2d07c/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Comment 22 by vabr@chromium.org, Jan 30 2018

Status: Started (was: Assigned)
Uploading a screenshot for https://crrev.com/c/881019.
passwords-progress.png
75.2 KB View Download
Project Member

Comment 23 by bugdroid1@chromium.org, Jan 31 2018

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

commit b76be05fb3716ff2d3a72fef7503dd9bb3d47422
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jan 31 18:24:16 2018

[Android passwords settings] Add a progress bar

This CL adds a progress bar to the password export flow.

The progress bar is shown if the user already completed
reauthentication and export confirmation but the passwords
are still being serialized in the background. The user
can cancel the whole export with a Cancel button next
to the progress bar. The screenshot is at
 https://crbug.com/788701#c22 , the mocks at
go chrome-pwd-export-mocks-android (Google only).

This CL also merges the existing flags mExportOptionSuspended,
mExportRequested and mExportConfirmed into a single mExportState
to improve the clarity of the whole process.

Bug:  788701 
Change-Id: I1857b135df255124a1507139095f36eca7c3dc3d
Reviewed-on: https://chromium-review.googlesource.com/881019
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533335}
[add] https://crrev.com/b76be05fb3716ff2d3a72fef7503dd9bb3d47422/chrome/android/java/res/layout/passwords_progress_dialog.xml
[add] https://crrev.com/b76be05fb3716ff2d3a72fef7503dd9bb3d47422/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ProgressBarDialogFragment.java
[modify] https://crrev.com/b76be05fb3716ff2d3a72fef7503dd9bb3d47422/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/b76be05fb3716ff2d3a72fef7503dd9bb3d47422/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/b76be05fb3716ff2d3a72fef7503dd9bb3d47422/chrome/android/java_sources.gni
[modify] https://crrev.com/b76be05fb3716ff2d3a72fef7503dd9bb3d47422/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 31 2018

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

commit 5620d24a32541ef1f1a393112abef3ea66e396ed
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jan 31 22:19:12 2018

Enable password export on Android

With https://crrev.com/c/881019 landing, all the necessary pieces are
there.

Bug:  788701 , 790993
Change-Id: I1a582c08eb86681afdb9adebcf6b0ebc38e93293
Reviewed-on: https://chromium-review.googlesource.com/895306
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533439}
[modify] https://crrev.com/5620d24a32541ef1f1a393112abef3ea66e396ed/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/5620d24a32541ef1f1a393112abef3ea66e396ed/components/password_manager/core/common/password_manager_features.cc

Comment 25 by vabr@chromium.org, Feb 2 2018

Uploading screenshots for an upcoming CL with error reporting.
drive.png
84.6 KB View Download
ioerror.png
85.9 KB View Download
Project Member

Comment 26 by bugdroid1@chromium.org, Feb 8 2018

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

commit 07d48c5b47ca21dbad88750d5de827fd65e9c658
Author: Vaclav Brozek <vabr@chromium.org>
Date: Thu Feb 08 22:36:16 2018

[Android settings] Display errors when exporting passwords

This CL adds a dialog to the passwords settings page on Android to
show errors encountered during exporting. The dialog also allows the
user to retry exporting or navigate to a help page, depending on
context.

Mocks: go chrome-pwd-export-mocks-android (Google only)

Screenshots:  https://crbug.com/788701#c25 

Bug:  788701 
Change-Id: Ibd2852803f03eb16cefa5ae04df1ecedbe51188f
Reviewed-on: https://chromium-review.googlesource.com/899363
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535552}
[add] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/java/res/layout/passwords_error_dialog.xml
[add] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportErrorDialogFragment.java
[modify] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportWarningDialogFragment.java
[modify] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/java_sources.gni
[modify] https://crrev.com/07d48c5b47ca21dbad88750d5de827fd65e9c658/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

Comment 27 by vabr@chromium.org, Feb 13 2018

Blockedon: 811307

Comment 28 by vabr@chromium.org, Feb 13 2018

Blockedon: 811317

Comment 29 by vabr@chromium.org, Feb 13 2018

Blockedon: 811312

Comment 30 by vabr@chromium.org, Feb 20 2018

Blockedon: 800686
Project Member

Comment 31 by bugdroid1@chromium.org, Feb 21 2018

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

commit 6de1635cf86dfa91ff57fe82966750ab48559e8c
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Feb 21 10:51:13 2018

Add metrics for password export on Android, part 1

This CL adds the following metrics to the code for exporting passwords
from Android settings:
* PasswordManager.TimeReadingExportedPasswords
* PasswordManager.ExportPasswordToCSVResult

It also extends the enumeration for the latter to capture some
Android-specific errors.

More details about the metrics are in the design doc [1].

(Remaining TODOs for metrics, not addressed here:
Reporting PasswordManager.ExportedPasswordsPerUserInCSV and the user
actions.)

[1]
https://docs.google.com/document/d/1miKr2x0PTNIKgt3RQeur51ICQ66uszlYAmFXJONbG_0/edit?ts=5a313898#heading=h.chwovfmlf7pk

Bug:  788701 
Change-Id: Ide5b1c908e7bd7a1240e1114487dab3636a5d315
Reviewed-on: https://chromium-review.googlesource.com/909108
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538059}
[modify] https://crrev.com/6de1635cf86dfa91ff57fe82966750ab48559e8c/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/6de1635cf86dfa91ff57fe82966750ab48559e8c/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/6de1635cf86dfa91ff57fe82966750ab48559e8c/components/password_manager/core/browser/password_manager_metrics_util.h
[modify] https://crrev.com/6de1635cf86dfa91ff57fe82966750ab48559e8c/tools/metrics/histograms/enums.xml

Comment 32 by vabr@chromium.org, Feb 28 2018

Blockedon: 817370
Project Member

Comment 33 by bugdroid1@chromium.org, Feb 28 2018

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

commit 9af53747bc71a3b8e183da795182bdf20e06817e
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Feb 28 15:47:32 2018

[Android]: Pass serialised passwords from native as a byte array

Chrome passwords settings on Android allow the user to export
passwords. The Java settings code asks the C++ code to serialise and
send over the passwords. The serialised result is in UTF-8 within C++
but gets converted into UTF-16 for Java and then back to UTF-8 on
writing to a cache file.

This CL changes the data type from String to byte array on Java side.
This eliminates the converstion to UTF-16 and back.

Note 1: This was pointed out in
https://chromium-review.googlesource.com/c/chromium/src/+/926527/2/chrome/browser/android/password_ui_view_android.cc#222.

Note 2: This might get further simplified if  https://crbug.com/817293 
gets implemented, but that's not happening in M66.

Bug:  788701 
Change-Id: I0799b2c5f6d7e43e9b7449322d300fb9e9d82c54
Reviewed-on: https://chromium-review.googlesource.com/940226
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539832}
[modify] https://crrev.com/9af53747bc71a3b8e183da795182bdf20e06817e/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandler.java
[modify] https://crrev.com/9af53747bc71a3b8e183da795182bdf20e06817e/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordUIView.java
[modify] https://crrev.com/9af53747bc71a3b8e183da795182bdf20e06817e/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/9af53747bc71a3b8e183da795182bdf20e06817e/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/9af53747bc71a3b8e183da795182bdf20e06817e/chrome/browser/android/password_ui_view_android.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Feb 28 2018

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

commit 32427f84360e52cc5c88a62a65edccbd1e77249a
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Feb 28 21:44:02 2018

Add metrics for password export on Android, part 2

This CL adds PasswordManager.ExportedPasswordsPerUserInCSV to the code
for exporting passwords from Android settings.

More details about the metrics are in the design doc [1].

(Remaining TODO for metrics, not addressed here: user actions.)

[1]
https://docs.google.com/document/d/1miKr2x0PTNIKgt3RQeur51ICQ66uszlYAmFXJONbG_0/edit?ts=5a313898#heading=h.chwovfmlf7pk

Bug:  788701 
Change-Id: I844f7506590d505b1d2e4671e21f5d363d73a2e7
Reviewed-on: https://chromium-review.googlesource.com/926527
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539946}
[add] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ByteArrayIntCallback.java
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandler.java
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordUIView.java
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/android/java_sources.gni
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/browser/BUILD.gn
[add] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/browser/android/byte_array_int_callback.cc
[add] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/browser/android/byte_array_int_callback.h
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/browser/android/password_ui_view_android.cc
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/browser/android/password_ui_view_android.h
[modify] https://crrev.com/32427f84360e52cc5c88a62a65edccbd1e77249a/chrome/browser/android/password_ui_view_android_unittest.cc

Labels: -M-65 M-66
Status: Fixed (was: Started)
As for the TODO from #34 above, user actions were removed from the plan.

So the code work on this feature is finished. Later bug fixes will get separate bugs if needed, and technical debt is tracked by  issue 817292 .

Sign in to add a comment