Rename JavaObjectWeakGlobalRef::is_empty to avoid misuse.

android_webview/native/aw_contents_io_thread_client_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "android_webview/native/aw_contents_io_thread_client_impl.h"
 
 #include <map>
 #include <memory>
 #include <utility>
 
@@ skipping 255 matching lines @@
   // TODO: currently there is only one browser context so it is ok to
   // store in a global variable, in the future use browser_context to
   // obtain the correct instance.
   JavaObjectWeakGlobalRef temp(AttachCurrentThread(), jclient.obj());
   g_sw_instance_.Get() = temp;
 }
 
 // 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())

[timvolodine, 2016/11/07 17:13:01]

to be more precise, I was looking here and comparing
g_sw_instance_.Get().is_empty() with g_sw_instance_.Get().get(env).is_null() it
seemed they were equivalent modulo the comment about JNIEnv::NewLocalRef(obj)

[boliu, 2016/11/07 17:23:33]

They are not equivalent.

is_empty (now is_initialized) will return true even if the *referent* object has
been garbage collected; it's only checking whether the *reference* is
initialized with something non-null

so it's possible for is_empty/is_initialized to be true and get(env).is_null to
be false

[boliu, 2016/11/07 17:25:29]

darn it, got that backwards

it's possible for get(env).is_null to be true, and is_empty to be false

[timvolodine, 2016/11/07 20:16:15]

Just to follow-up: we chatted here a bit about this, ServiceWorkerClient
currently has singleton semantics so not much gc happening unless the user
clears it explicitly. In general it seems fine (and correct) to return null in
this case, we have code to handle null cases further down the road.

+    return std::unique_ptr<AwContentsIoThreadClient>();
+
   return std::unique_ptr<AwContentsIoThreadClient>(
       new AwContentsIoThreadClientImpl(false, java_delegate));
 }
 
 AwContentsIoThreadClientImpl::AwContentsIoThreadClientImpl(
     bool pending_association,
     const JavaRef<jobject>& obj)
   : pending_association_(pending_association),
     java_object_(obj) {
 }
@@ skipping 168 matching lines @@
   Java_AwContentsIoThreadClient_onReceivedHttpError(
       env, java_object_, web_request.jstring_url, web_request.is_main_frame,
       web_request.has_user_gesture, web_request.jstring_method,
       web_request.jstringArray_header_names,
       web_request.jstringArray_header_values, jstring_mime_type,
       jstring_encoding, status_code, jstring_reason,
       jstringArray_response_header_names, jstringArray_response_header_values);
 }
 
 } // namespace android_webview