Rename JavaObjectWeakGlobalRef::is_empty to avoid misuse.

JavaObjectWeakGlobalRef::is_empty was being misused to attempt to check if the object referred to has been collected or not (see crbug.com/661444 for one case; others in this CL). The method is valuable in some cases that actually just want to check if the weak reference has been initialised, so rename it to is_uninitialized and add an explanatory comment to make it more obvious what to do with it. Fix a couple of existing uses that are incorrect or dubious, and rename the "okay" uses to call the new function. BUG= Review-Url: https://codereview.chromium.org/2485513003 Cr-Commit-Position: refs/heads/master@{#430417}
Denger-Network · Nov 7, 2016 · aef0403 · aef0403
1 parent 8c28653
commit aef0403
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 10 deletions.
diff --git a/android_webview/native/aw_contents_io_thread_client_impl.cc b/android_webview/native/aw_contents_io_thread_client_impl.cc
@@ -273,13 +273,12 @@ void AwContentsIoThreadClientImpl::SetServiceWorkerIoThreadClient(
 // static
 std::unique_ptr<AwContentsIoThreadClient>
 AwContentsIoThreadClient::GetServiceWorkerIoThreadClient() {
-  if (g_sw_instance_.Get().is_empty())
-    return std::unique_ptr<AwContentsIoThreadClient>();
-
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> java_delegate = g_sw_instance_.Get().get(env);
 
-  DCHECK(!java_delegate.is_null());
+  if (java_delegate.is_null())
+    return std::unique_ptr<AwContentsIoThreadClient>();
+
   return std::unique_ptr<AwContentsIoThreadClient>(
       new AwContentsIoThreadClientImpl(false, java_delegate));
 }

diff --git a/android_webview/native/popup_touch_handle_drawable.cc b/android_webview/native/popup_touch_handle_drawable.cc
@@ -17,7 +17,7 @@ PopupTouchHandleDrawable::PopupTouchHandleDrawable(
     float horizontal_padding_ratio)
     : java_ref_(env, obj)
     , drawable_horizontal_padding_ratio_(horizontal_padding_ratio) {
-  DCHECK(!java_ref_.is_empty());
+  DCHECK(obj);
 }
 
 PopupTouchHandleDrawable::~PopupTouchHandleDrawable() {

diff --git a/base/android/jni_weak_ref.h b/base/android/jni_weak_ref.h
@@ -29,7 +29,11 @@ class BASE_EXPORT JavaObjectWeakGlobalRef {
 
   base::android::ScopedJavaLocalRef<jobject> get(JNIEnv* env) const;
 
-  bool is_empty() const { return obj_ == nullptr; }
+  // Returns true if the weak reference has not been initialized to point at
+  // an object (or ḣas had reset() called).
+  // Do not call this to test if the object referred to still exists! The weak
+  // reference remains initialized even if the target object has been collected.
+  bool is_uninitialized() const { return obj_ == nullptr; }
 
   void reset();
 

diff --git a/content/browser/android/java/gin_java_bridge_dispatcher_host.cc b/content/browser/android/java/gin_java_bridge_dispatcher_host.cc
@@ -374,7 +374,7 @@ void GinJavaBridgeDispatcherHost::OnObjectWrapperDeleted(
     return;
   JavaObjectWeakGlobalRef ref =
       RemoveHolderAndAdvanceLocked(routing_id, &iter);
-  if (!ref.is_empty()) {
+  if (!ref.is_uninitialized()) {
     RemoveFromRetainedObjectSetLocked(ref);
   }
 }

diff --git a/content/browser/android/java/gin_java_method_invocation_helper.cc b/content/browser/android/java/gin_java_method_invocation_helper.cc
@@ -100,7 +100,7 @@ bool GinJavaMethodInvocationHelper::AppendObjectRef(
     if (iter == object_refs_.end()) {
       JavaObjectWeakGlobalRef object_ref(
           dispatcher->GetObjectWeakRef(object_id));
-      if (!object_ref.is_empty()) {
+      if (!object_ref.is_uninitialized()) {
         object_refs_.insert(std::make_pair(object_id, object_ref));
       }
     }

diff --git a/ui/platform_window/android/platform_ime_controller_android.cc b/ui/platform_window/android/platform_ime_controller_android.cc
@@ -26,7 +26,7 @@ PlatformImeControllerAndroid::~PlatformImeControllerAndroid() {
 
 void PlatformImeControllerAndroid::Init(JNIEnv* env,
                                         const JavaParamRef<jobject>& jobj) {
-  DCHECK(java_platform_ime_controller_android_.is_empty());
+  DCHECK(java_platform_ime_controller_android_.is_uninitialized());
   java_platform_ime_controller_android_ = JavaObjectWeakGlobalRef(env, jobj);
 }
 

diff --git a/ui/platform_window/android/platform_window_android.cc b/ui/platform_window/android/platform_window_android.cc
@@ -160,7 +160,7 @@ void PlatformWindowAndroid::ReleaseWindow() {
 // PlatformWindowAndroid, PlatformWindow implementation:
 
 void PlatformWindowAndroid::Show() {
-  if (!java_platform_window_android_.is_empty())
+  if (!java_platform_window_android_.is_uninitialized())
     return;
   JNIEnv* env = base::android::AttachCurrentThread();
   java_platform_window_android_ = JavaObjectWeakGlobalRef(