Commit 9af53747 authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

[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/940226Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539832}
parent 23cce521
...@@ -69,5 +69,5 @@ public interface PasswordManagerHandler { ...@@ -69,5 +69,5 @@ public interface PasswordManagerHandler {
* *
* @param callback is called on completion, with the serialized passwords as argument. * @param callback is called on completion, with the serialized passwords as argument.
*/ */
void serializePasswords(Callback<String> callback); void serializePasswords(Callback<byte[]> callback);
} }
...@@ -72,7 +72,7 @@ public final class PasswordUIView implements PasswordManagerHandler { ...@@ -72,7 +72,7 @@ public final class PasswordUIView implements PasswordManagerHandler {
} }
@Override @Override
public void serializePasswords(Callback<String> callback) { public void serializePasswords(Callback<byte[]> callback) {
nativeHandleSerializePasswords(mNativePasswordUIViewAndroid, callback); nativeHandleSerializePasswords(mNativePasswordUIViewAndroid, callback);
} }
...@@ -116,5 +116,5 @@ public final class PasswordUIView implements PasswordManagerHandler { ...@@ -116,5 +116,5 @@ public final class PasswordUIView implements PasswordManagerHandler {
private native void nativeDestroy(long nativePasswordUIViewAndroid); private native void nativeDestroy(long nativePasswordUIViewAndroid);
private native void nativeHandleSerializePasswords( private native void nativeHandleSerializePasswords(
long nativePasswordUIViewAndroid, Callback<String> callback); long nativePasswordUIViewAndroid, Callback<byte[]> callback);
} }
...@@ -51,14 +51,12 @@ import org.chromium.chrome.browser.preferences.TextMessagePreference; ...@@ -51,14 +51,12 @@ import org.chromium.chrome.browser.preferences.TextMessagePreference;
import org.chromium.ui.text.SpanApplier; import org.chromium.ui.text.SpanApplier;
import org.chromium.ui.widget.Toast; import org.chromium.ui.widget.Toast;
import java.io.BufferedWriter; import java.io.BufferedOutputStream;
import java.io.File; import java.io.File;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStreamWriter;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
import java.nio.charset.Charset;
import java.util.Locale; import java.util.Locale;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
...@@ -334,10 +332,10 @@ public class SavePasswordsPreferences ...@@ -334,10 +332,10 @@ public class SavePasswordsPreferences
* for firing the share intent or displays an error. * for firing the share intent or displays an error.
* @param serializedPasswords A string with a CSV representation of the user's passwords. * @param serializedPasswords A string with a CSV representation of the user's passwords.
*/ */
private void shareSerializedPasswords(String serializedPasswords) { private void shareSerializedPasswords(byte[] serializedPasswords) {
AsyncTask<String, Void, ExportResult> task = new AsyncTask<String, Void, ExportResult>() { AsyncTask<byte[], Void, ExportResult> task = new AsyncTask<byte[], Void, ExportResult>() {
@Override @Override
protected ExportResult doInBackground(String... serializedPasswords) { protected ExportResult doInBackground(byte[]... serializedPasswords) {
assert serializedPasswords.length == 1; assert serializedPasswords.length == 1;
// Record the time it took to read and serialise the passwords. This excludes the // Record the time it took to read and serialise the passwords. This excludes the
// time to write them into a file, to be consistent with desktop (where writing is // time to write them into a file, to be consistent with desktop (where writing is
...@@ -382,9 +380,9 @@ public class SavePasswordsPreferences ...@@ -382,9 +380,9 @@ public class SavePasswordsPreferences
// they arrive. // they arrive.
mExportPreparationStart = System.currentTimeMillis(); mExportPreparationStart = System.currentTimeMillis();
PasswordManagerHandlerProvider.getInstance().getPasswordManagerHandler().serializePasswords( PasswordManagerHandlerProvider.getInstance().getPasswordManagerHandler().serializePasswords(
new Callback<String>() { new Callback<byte[]>() {
@Override @Override
public void onResult(String serializedPasswords) { public void onResult(byte[] serializedPasswords) {
shareSerializedPasswords(serializedPasswords); shareSerializedPasswords(serializedPasswords);
} }
}); });
...@@ -571,9 +569,9 @@ public class SavePasswordsPreferences ...@@ -571,9 +569,9 @@ public class SavePasswordsPreferences
* This method saves the contents of |serializedPasswords| into a temporary file and returns a * This method saves the contents of |serializedPasswords| into a temporary file and returns a
* sharing URI for it. In case of failure, returns EMPTY. It should only be run on the * sharing URI for it. In case of failure, returns EMPTY. It should only be run on the
* background thread of an AsyncTask, because it does I/O operations. * background thread of an AsyncTask, because it does I/O operations.
* @param serializedPasswords A string with serialized passwords in CSV format * @param serializedPasswords A byte array with serialized passwords in CSV format
*/ */
private ExportResult exportPasswordsIntoFile(String serializedPasswords) { private ExportResult exportPasswordsIntoFile(byte[] serializedPasswords) {
// First ensure that the PASSWORDS_CACHE_DIR cache directory exists. // First ensure that the PASSWORDS_CACHE_DIR cache directory exists.
File passwordsDir = File passwordsDir =
new File(ContextUtils.getApplicationContext().getCacheDir() + PASSWORDS_CACHE_DIR); new File(ContextUtils.getApplicationContext().getCacheDir() + PASSWORDS_CACHE_DIR);
...@@ -587,9 +585,9 @@ public class SavePasswordsPreferences ...@@ -587,9 +585,9 @@ public class SavePasswordsPreferences
return new ExportResult(e.getMessage()); return new ExportResult(e.getMessage());
} }
tempFile.deleteOnExit(); tempFile.deleteOnExit();
try (BufferedWriter tempWriter = new BufferedWriter(new OutputStreamWriter( try (BufferedOutputStream outputStream =
new FileOutputStream(tempFile), Charset.forName("UTF-8")))) { new BufferedOutputStream(new FileOutputStream(tempFile))) {
tempWriter.write(serializedPasswords); outputStream.write(serializedPasswords);
} catch (IOException e) { } catch (IOException e) {
return new ExportResult(e.getMessage()); return new ExportResult(e.getMessage());
} }
......
...@@ -107,7 +107,7 @@ public class SavePasswordsPreferencesTest { ...@@ -107,7 +107,7 @@ public class SavePasswordsPreferencesTest {
// This is set once {@link #serializePasswords()} is called. // This is set once {@link #serializePasswords()} is called.
@Nullable @Nullable
private Callback<String> mExportCallback; private Callback<byte[]> mExportCallback;
public void setSavedPasswords(ArrayList<SavedPasswordEntry> savedPasswords) { public void setSavedPasswords(ArrayList<SavedPasswordEntry> savedPasswords) {
mSavedPasswords = savedPasswords; mSavedPasswords = savedPasswords;
...@@ -117,7 +117,7 @@ public class SavePasswordsPreferencesTest { ...@@ -117,7 +117,7 @@ public class SavePasswordsPreferencesTest {
mSavedPasswordExeptions = savedPasswordExceptions; mSavedPasswordExeptions = savedPasswordExceptions;
} }
public Callback<String> getExportCallback() { public Callback<byte[]> getExportCallback() {
return mExportCallback; return mExportCallback;
} }
...@@ -162,7 +162,7 @@ public class SavePasswordsPreferencesTest { ...@@ -162,7 +162,7 @@ public class SavePasswordsPreferencesTest {
} }
@Override @Override
public void serializePasswords(Callback<String> callback) { public void serializePasswords(Callback<byte[]> callback) {
mExportCallback = callback; mExportCallback = callback;
} }
} }
...@@ -738,7 +738,7 @@ public class SavePasswordsPreferencesTest { ...@@ -738,7 +738,7 @@ public class SavePasswordsPreferencesTest {
reauthenticateAndRequestExport(); reauthenticateAndRequestExport();
// Pretend that passwords have been serialized to go directly to the intent. // Pretend that passwords have been serialized to go directly to the intent.
mHandler.getExportCallback().onResult("serialized passwords"); mHandler.getExportCallback().onResult(new byte[] {1, 2, 3});
// Before triggering the sharing intent chooser, stub it out to avoid leaving system UI open // Before triggering the sharing intent chooser, stub it out to avoid leaving system UI open
// after the test is finished. // after the test is finished.
...@@ -795,7 +795,7 @@ public class SavePasswordsPreferencesTest { ...@@ -795,7 +795,7 @@ public class SavePasswordsPreferencesTest {
}); });
// Pretend that passwords have been serialized to go directly to the intent. // Pretend that passwords have been serialized to go directly to the intent.
mHandler.getExportCallback().onResult("serialized passwords"); mHandler.getExportCallback().onResult(new byte[] {1, 2, 3});
// Before triggering the sharing intent chooser, stub it out to avoid leaving system UI open // Before triggering the sharing intent chooser, stub it out to avoid leaving system UI open
// after the test is finished. // after the test is finished.
...@@ -1006,7 +1006,7 @@ public class SavePasswordsPreferencesTest { ...@@ -1006,7 +1006,7 @@ public class SavePasswordsPreferencesTest {
.check(matches(isDisplayed())); .check(matches(isDisplayed()));
// Now pretend that passwords have been serialized. // Now pretend that passwords have been serialized.
mHandler.getExportCallback().onResult("serialized passwords"); mHandler.getExportCallback().onResult(new byte[] {1, 2, 3});
// After simulating the serialized passwords being received, check that the progress bar is // After simulating the serialized passwords being received, check that the progress bar is
// hidden. // hidden.
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <algorithm> #include <algorithm>
#include "base/android/callback_android.h" #include "base/android/callback_android.h"
#include "base/android/jni_array.h"
#include "base/android/jni_string.h" #include "base/android/jni_string.h"
#include "base/android/jni_weak_ref.h" #include "base/android/jni_weak_ref.h"
#include "base/android/scoped_java_ref.h" #include "base/android/scoped_java_ref.h"
...@@ -213,7 +214,11 @@ void PasswordUIViewAndroid::PostSerializedPasswords( ...@@ -213,7 +214,11 @@ void PasswordUIViewAndroid::PostSerializedPasswords(
} else { } else {
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
base::android::RunCallbackAndroid( base::android::RunCallbackAndroid(
callback, ConvertUTF8ToJavaString(env, serialized_passwords)); callback,
base::android::ToJavaByteArray(
env,
reinterpret_cast<const uint8_t*>(serialized_passwords.data()),
serialized_passwords.size()));
} }
break; break;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment