Commit ccacbe95 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Android][TouchToFill] Check and recreate Android view before usage

Relying on GetWindowAndroid() returning a non-null value causes crashes
in various instances. With this CL, the window is null-checked before
use an creation is deferred to when it's actually used.
If creation doesn't succeed on first try (e.g. because the window isn't
yet attached), a consecutive call might succeed.

This approach also has the advantage that the interface remains clean
and doesn't require handling the rare Android failure explicitly. This
is acceptable since failing to create the surface means, that the UI
isn't ready to show any information anyway or anymore.

Bug: 1049090
Change-Id: Iebc556e33226151cbc4fb8060653290ae0e22dc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037573
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739347}
parent 32c30787
...@@ -48,20 +48,23 @@ UiCredential ConvertJavaCredential(JNIEnv* env, ...@@ -48,20 +48,23 @@ UiCredential ConvertJavaCredential(JNIEnv* env,
} // namespace } // namespace
TouchToFillViewImpl::TouchToFillViewImpl(TouchToFillController* controller) TouchToFillViewImpl::TouchToFillViewImpl(TouchToFillController* controller)
: controller_(controller) { : controller_(controller) {}
java_object_ = Java_TouchToFillBridge_create(
AttachCurrentThread(), reinterpret_cast<intptr_t>(this),
controller_->GetNativeView()->GetWindowAndroid()->GetJavaObject());
}
TouchToFillViewImpl::~TouchToFillViewImpl() { TouchToFillViewImpl::~TouchToFillViewImpl() {
Java_TouchToFillBridge_destroy(AttachCurrentThread(), java_object_); if (java_object_internal_) {
// Don't create an object just for destruction.
Java_TouchToFillBridge_destroy(AttachCurrentThread(),
java_object_internal_);
}
} }
void TouchToFillViewImpl::Show( void TouchToFillViewImpl::Show(
const GURL& url, const GURL& url,
IsOriginSecure is_origin_secure, IsOriginSecure is_origin_secure,
base::span<const password_manager::UiCredential> credentials) { base::span<const password_manager::UiCredential> credentials) {
auto java_object = GetOrCreateJavaObject();
if (!java_object)
return;
// Serialize the |credentials| span into a Java array and instruct the bridge // Serialize the |credentials| span into a Java array and instruct the bridge
// to show it together with |url| to the user. // to show it together with |url| to the user.
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
...@@ -80,7 +83,7 @@ void TouchToFillViewImpl::Show( ...@@ -80,7 +83,7 @@ void TouchToFillViewImpl::Show(
} }
Java_TouchToFillBridge_showCredentials( Java_TouchToFillBridge_showCredentials(
env, java_object_, ConvertUTF8ToJavaString(env, url.spec()), env, java_object, ConvertUTF8ToJavaString(env, url.spec()),
is_origin_secure.value(), credential_array); is_origin_secure.value(), credential_array);
} }
...@@ -105,3 +108,17 @@ void TouchToFillViewImpl::OnManagePasswordsSelected(JNIEnv* env) { ...@@ -105,3 +108,17 @@ void TouchToFillViewImpl::OnManagePasswordsSelected(JNIEnv* env) {
void TouchToFillViewImpl::OnDismiss(JNIEnv* env) { void TouchToFillViewImpl::OnDismiss(JNIEnv* env) {
OnDismiss(); OnDismiss();
} }
base::android::ScopedJavaGlobalRef<jobject>
TouchToFillViewImpl::GetOrCreateJavaObject() {
if (java_object_internal_) {
return java_object_internal_;
}
if (controller_->GetNativeView() == nullptr ||
controller_->GetNativeView()->GetWindowAndroid() == nullptr) {
return nullptr; // No window attached (yet or anymore).
}
return java_object_internal_ = Java_TouchToFillBridge_create(
AttachCurrentThread(), reinterpret_cast<intptr_t>(this),
controller_->GetNativeView()->GetWindowAndroid()->GetJavaObject());
}
...@@ -38,8 +38,14 @@ class TouchToFillViewImpl : public TouchToFillView { ...@@ -38,8 +38,14 @@ class TouchToFillViewImpl : public TouchToFillView {
void OnDismiss(JNIEnv* env); void OnDismiss(JNIEnv* env);
private: private:
// Returns either the fully initialized java counterpart of this bridge or
// a is_null() reference if the creation failed. By using this method, the
// bridge will try to recreate the java object if it failed previously (e.g.
// because there was no native window available).
base::android::ScopedJavaGlobalRef<jobject> GetOrCreateJavaObject();
TouchToFillController* controller_ = nullptr; TouchToFillController* controller_ = nullptr;
base::android::ScopedJavaGlobalRef<jobject> java_object_; base::android::ScopedJavaGlobalRef<jobject> java_object_internal_;
}; };
#endif // CHROME_BROWSER_TOUCH_TO_FILL_ANDROID_TOUCH_TO_FILL_VIEW_IMPL_H_ #endif // CHROME_BROWSER_TOUCH_TO_FILL_ANDROID_TOUCH_TO_FILL_VIEW_IMPL_H_
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