Commit 2ac4c6ca authored by Torne (Richard Coles)'s avatar Torne (Richard Coles) Committed by Commit Bot

Modernize JavaRef and its subclasses.

Modernize the interface for JavaRef and its subclasses - make better use
of C++11 features, and mark existing confusing/easily misused APIs as
deprecated. Also. fully reformat the file while it's being changed this
much.

Notable changes:
 - JavaRef can now be converted to bool in bool contexts, so "if (obj)"
   works and "!obj.is_null()" is no longer necessary.
 - ScopedJavaLocalRef<T>::Adopt() now exists to make it more explicit
   that the caller is adopting an existing reference, not creating a new
   one. This should only be used when dealing with return values from
   JNIEnv methods.
 - Conversion constructors/assignments now exist, allowing for example
   JavaRef<jstring> to be assigned to ScopedJavaLocalRef<jobject>.
 - Local references can be constructed or assigned from global
   references and vice versa without needing to use Reset().
 - Confusing forms of Reset() that take different parameters, or behave
   differently from, construction/assignment have been deprecated.

The new API is intended to be somewhat similar to scoped_refptr, which
is the closest native equivalent.

Bug: 519562
Test: base_unittests
Change-Id: I8a989dd04b357e83bc2a28a6ec9b7e14495ccbd2
Reviewed-on: https://chromium-review.googlesource.com/c/1262115
Commit-Queue: Richard Coles <torne@chromium.org>
Reviewed-by: default avataragrieve <agrieve@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598800}
parent 95fb8fdb
This diff is collapsed.
......@@ -8,6 +8,9 @@
#include "base/android/jni_string.h"
#include "testing/gtest/include/gtest/gtest.h"
#define EXPECT_SAME_OBJECT(a, b) \
EXPECT_TRUE(env->IsSameObject(a.obj(), b.obj()))
namespace base {
namespace android {
......@@ -69,13 +72,110 @@ TEST_F(ScopedJavaRefTest, Conversions) {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jstring> str = ConvertUTF8ToJavaString(env, "string");
ScopedJavaGlobalRef<jstring> global(str);
// Contextual conversions to bool should be allowed.
EXPECT_TRUE(str);
EXPECT_FALSE(JavaRef<jobject>());
// All the types should convert from nullptr, even JavaRef.
{
JavaRef<jstring> null_ref(nullptr);
EXPECT_FALSE(null_ref);
ScopedJavaLocalRef<jobject> null_local(nullptr);
EXPECT_FALSE(null_local);
ScopedJavaGlobalRef<jarray> null_global(nullptr);
EXPECT_FALSE(null_global);
}
// Local and global refs should {copy,move}-{construct,assign}.
// Moves should leave the source as null.
{
ScopedJavaLocalRef<jstring> str2(str);
EXPECT_SAME_OBJECT(str2, str);
ScopedJavaLocalRef<jstring> str3(std::move(str2));
EXPECT_SAME_OBJECT(str3, str);
EXPECT_FALSE(str2);
ScopedJavaLocalRef<jstring> str4;
str4 = str;
EXPECT_SAME_OBJECT(str4, str);
ScopedJavaLocalRef<jstring> str5;
str5 = std::move(str4);
EXPECT_SAME_OBJECT(str5, str);
EXPECT_FALSE(str4);
}
{
ScopedJavaGlobalRef<jstring> str2(global);
EXPECT_SAME_OBJECT(str2, str);
ScopedJavaGlobalRef<jstring> str3(std::move(str2));
EXPECT_SAME_OBJECT(str3, str);
EXPECT_FALSE(str2);
ScopedJavaGlobalRef<jstring> str4;
str4 = global;
EXPECT_SAME_OBJECT(str4, str);
ScopedJavaGlobalRef<jstring> str5;
str5 = std::move(str4);
EXPECT_SAME_OBJECT(str5, str);
EXPECT_FALSE(str4);
}
// As above but going from jstring to jobject.
{
ScopedJavaLocalRef<jobject> obj2(str);
EXPECT_SAME_OBJECT(obj2, str);
ScopedJavaLocalRef<jobject> obj3(std::move(obj2));
EXPECT_SAME_OBJECT(obj3, str);
EXPECT_FALSE(obj2);
ScopedJavaLocalRef<jobject> obj4;
obj4 = str;
EXPECT_SAME_OBJECT(obj4, str);
ScopedJavaLocalRef<jobject> obj5;
obj5 = std::move(obj4);
EXPECT_SAME_OBJECT(obj5, str);
EXPECT_FALSE(obj4);
}
{
ScopedJavaGlobalRef<jobject> obj2(global);
EXPECT_SAME_OBJECT(obj2, str);
ScopedJavaGlobalRef<jobject> obj3(std::move(obj2));
EXPECT_SAME_OBJECT(obj3, str);
EXPECT_FALSE(obj2);
ScopedJavaGlobalRef<jobject> obj4;
obj4 = global;
EXPECT_SAME_OBJECT(obj4, str);
ScopedJavaGlobalRef<jobject> obj5;
obj5 = std::move(obj4);
EXPECT_SAME_OBJECT(obj5, str);
EXPECT_FALSE(obj4);
}
// Explicit copy construction or assignment between global<->local is allowed,
// but not implicit conversions.
{
ScopedJavaLocalRef<jstring> new_local(global);
EXPECT_SAME_OBJECT(new_local, str);
new_local = global;
EXPECT_SAME_OBJECT(new_local, str);
ScopedJavaGlobalRef<jstring> new_global(str);
EXPECT_SAME_OBJECT(new_global, str);
new_global = str;
EXPECT_SAME_OBJECT(new_local, str);
static_assert(!std::is_convertible<ScopedJavaLocalRef<jobject>,
ScopedJavaGlobalRef<jobject>>::value,
"");
static_assert(!std::is_convertible<ScopedJavaGlobalRef<jobject>,
ScopedJavaLocalRef<jobject>>::value,
"");
}
// Converting between local/global while also converting to jobject also works
// because JavaRef<jobject> is the base class.
{
ScopedJavaGlobalRef<jobject> global_obj(str);
ScopedJavaLocalRef<jobject> local_obj(global);
const JavaRef<jobject>& obj_ref1(str);
const JavaRef<jobject>& obj_ref2(global);
EXPECT_TRUE(env->IsSameObject(obj_ref1.obj(), obj_ref2.obj()));
EXPECT_TRUE(env->IsSameObject(global_obj.obj(), obj_ref2.obj()));
EXPECT_SAME_OBJECT(obj_ref1, obj_ref2);
EXPECT_SAME_OBJECT(global_obj, obj_ref2);
}
global.Reset(str);
const JavaRef<jstring>& str_ref = str;
......@@ -99,7 +199,7 @@ TEST_F(ScopedJavaRefTest, RefCounts) {
EXPECT_EQ(1, g_local_refs);
EXPECT_EQ(2, g_global_refs);
ScopedJavaLocalRef<jstring> str2(env, str.Release());
auto str2 = ScopedJavaLocalRef<jstring>::Adopt(env, str.Release());
EXPECT_EQ(1, g_local_refs);
{
ScopedJavaLocalRef<jstring> str3(str2);
......
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