Create file for serialised passwords in PasswordUIViewAndroid instead of SavePasswordsPreferences |
||
Issue descriptionThis is a follow-up to issue 817293 . When Chrome on Android exports passwords, the Java UI code creates the target file for exported passwords first, then calls the C++ backend to fill it with passwords. This unnecessarily splits the I/O operations at two places, results in more different error handlers and potentially causes a StrictModeDiskReadViolation for the I/O operations done from Java. Instead, Java should just provide the target directory path, then the C++ backend should create the file in it and fill it in one go. This way: * Any errors coming from the file creation will be reported by the already existing error reporting present for the writing errors. * The I/O will happen in one place, properly checked for running on a suitable thread.
,
Oct 2
Hm, now I realised the StrictModeDiskReadViolation comes from using android.content.ContextWrapper.getCacheDir. That's not something to be addressed on this bug, and it seems out of Chrome's control. In general, StrictModeDiskReadViolation occurrences are tracked in bug 398264.
,
Oct 2
Adding a figure to help with review of https://crrev.com/c/1256813
,
Oct 2
Just for my own record, dumping links to a discussion about custom callbacks on the internal team: go/alrnp, and to an older approved CL which added a 2-value callback: https://crrev.com/c/926527
,
Oct 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74 commit ddb5f9f89cb68eed764f8d0a7fca4af238a53d74 Author: Vaclav Brozek <vabr@chromium.org> Date: Sun Oct 28 14:08:56 2018 [Password export][Android] Create file in native code When Chrome exports passwords, it creates a temporary file, serializes the passwords into it and shares it through an intent. So far, creating the file happened from the Java code, while writing to it from the native C++ backend. This required gathering and reporting I/O errors from two places. This CL moves the file creation next to the writing code in the native C++ backend. As a side effect to achieve the goal, this CL also adds a two-argument callback for the communication across Java bindings, because Callback.java only accepts a single argument. The added callback has fixed argument types (and int and a string), because generalising seems like a significant effort and there is no clear demand for that. OTOH, the new callback ends up in base, next to the single-arg callback, so that it is visible to developers in the future. Bug: 891218 Change-Id: Ida685baa72456f8631356f219ea80ad7db195db5 Reviewed-on: https://chromium-review.googlesource.com/c/1256813 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#603376} [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/base/BUILD.gn [add] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/base/android/int_string_callback.cc [add] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/base/android/int_string_callback.h [add] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/base/android/java/src/org/chromium/base/IntStringCallback.java [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportFlow.java [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandler.java [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordUIView.java [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/browser/android/password_ui_view_android.cc [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/browser/android/password_ui_view_android.h [modify] https://crrev.com/ddb5f9f89cb68eed764f8d0a7fca4af238a53d74/chrome/browser/android/password_ui_view_android_unittest.cc
,
Oct 29
|
||
►
Sign in to add a comment |
||
Comment 1 by vabr@chromium.org
, Oct 2