Commit dcd07902 authored by Ryan Daum's avatar Ryan Daum Committed by Commit Bot

[chromecast] Fix Android gesture callback scoping

The callback wrapper being local stack allocated means the
asynchronous callback invocation may fail if invoked after the method
returns; this has been causing crash/JNI failure. Switch
to a unique_ptr and move it along.

Additionally the callback argument itself may go out of scope, so
needs to be switched to the correctly scoped JNI handle.

Bug: internal b/162882383
Test: manual on device
Change-Id: I4bb4c87531e9a4b099f61ad9133ad1b3922de4be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364559
Commit-Queue: Ryan Daum <rdaum@chromium.org>
Reviewed-by: default avatarDaniel Nicoara <dnicoara@chromium.org>
Reviewed-by: default avatarSimeon Anfinrud <sanfin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800086}
parent 336ff538
......@@ -10,7 +10,6 @@
#include "base/android/jni_string.h"
#include "base/android/scoped_java_ref.h"
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "chromecast/browser/jni_headers/CastContentWindowAndroid_jni.h"
#include "content/public/browser/web_contents.h"
#include "ui/events/keycodes/keyboard_code_conversion_android.h"
......@@ -48,7 +47,7 @@ class GestureConsumedCallbackWrapper {
JNIEnv* env,
const base::android::JavaParamRef<jobject>& callback)
: env_(env), callback_(callback) {
class_ = base::android::ScopedJavaLocalRef<jclass>(
class_ = base::android::ScopedJavaGlobalRef<jclass>(
base::android::GetClass(env_, kGestureConsumedCallbackClassName));
callback_method_id_ =
base::android::MethodID::Get<base::android::MethodID::TYPE_INSTANCE>(
......@@ -57,6 +56,11 @@ class GestureConsumedCallbackWrapper {
")V");
}
GestureConsumedCallbackWrapper& operator=(
const GestureConsumedCallbackWrapper&) = delete;
GestureConsumedCallbackWrapper(const GestureConsumedCallbackWrapper&) =
delete;
void Invoke(bool handled) {
jboolean handled_value(handled);
env_->CallVoidMethod(callback_.obj(), callback_method_id_, handled_value);
......@@ -64,11 +68,9 @@ class GestureConsumedCallbackWrapper {
private:
JNIEnv* env_;
const base::android::JavaParamRef<jobject>& callback_;
base::android::ScopedJavaLocalRef<jclass> class_;
const base::android::ScopedJavaGlobalRef<jobject> callback_;
base::android::ScopedJavaGlobalRef<jclass> class_;
jmethodID callback_method_id_;
DISALLOW_COPY_AND_ASSIGN(GestureConsumedCallbackWrapper);
};
} // namespace
......@@ -178,15 +180,16 @@ void CastContentWindowAndroid::ConsumeGesture(
const base::android::JavaParamRef<jobject>& jcaller,
int gesture_type,
const base::android::JavaParamRef<jobject>& callback) {
GestureConsumedCallbackWrapper wrapper(env, callback);
auto wrapper =
std::make_unique<GestureConsumedCallbackWrapper>(env, callback);
if (delegate_) {
delegate_->ConsumeGesture(
static_cast<GestureType>(gesture_type),
base::BindOnce(&GestureConsumedCallbackWrapper::Invoke,
base::Unretained(&wrapper)));
std::move(wrapper)));
return;
}
wrapper.Invoke(false);
wrapper->Invoke(false);
}
void CastContentWindowAndroid::OnVisibilityChange(
......
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