Skip to content

Commit

Permalink
Pass JavaRef to Java methods in android_webview.
Browse files Browse the repository at this point in the history
Update code in android_webview to use JavaRef when calling Java methods
via JNI, instead of passing bare jobject. Various function parameter
types are converted from jobject to JavaRef to enable calls to obj()
higher up the call chain to be removed.

BUG=506850

Review-Url: https://codereview.chromium.org/2295693004
Cr-Commit-Position: refs/heads/master@{#416234}
  • Loading branch information
tornewuff authored and Commit bot committed Sep 2, 2016
1 parent f5a2901 commit 1c12b81
Show file tree
Hide file tree
Showing 17 changed files with 58 additions and 63 deletions.
4 changes: 2 additions & 2 deletions android_webview/native/aw_autofill_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ AwAutofillClient::AwAutofillClient(WebContents* contents)
Java_AwAutofillClient_create(env, reinterpret_cast<intptr_t>(this)));

AwContents* aw_contents = AwContents::FromWebContents(web_contents_);
aw_contents->SetAwAutofillClient(delegate.obj());
java_ref_ = JavaObjectWeakGlobalRef(env, delegate.obj());
aw_contents->SetAwAutofillClient(delegate);
java_ref_ = JavaObjectWeakGlobalRef(env, delegate);
}

AwAutofillClient::~AwAutofillClient() {
Expand Down
8 changes: 3 additions & 5 deletions android_webview/native/aw_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ void AwContents::InitAutofillIfNecessary(bool enabled) {
AutofillManager::DISABLE_AUTOFILL_DOWNLOAD_MANAGER);
}

void AwContents::SetAwAutofillClient(jobject client) {
void AwContents::SetAwAutofillClient(const JavaRef<jobject>& client) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
Expand Down Expand Up @@ -399,8 +399,7 @@ void GenerateMHTMLCallback(ScopedJavaGlobalRef<jobject>* callback,
JNIEnv* env = AttachCurrentThread();
// Android files are UTF8, so the path conversion below is safe.
Java_AwContents_generateMHTMLCallback(
env, ConvertUTF8ToJavaString(env, path.AsUTF8Unsafe()), size,
callback->obj());
env, ConvertUTF8ToJavaString(env, path.AsUTF8Unsafe()), size, *callback);
}
} // namespace

Expand Down Expand Up @@ -1147,8 +1146,7 @@ void InvokeVisualStateCallback(const JavaObjectWeakGlobalRef& java_ref,
ScopedJavaLocalRef<jobject> obj = java_ref.get(env);
if (obj.is_null())
return;
Java_AwContents_invokeVisualStateCallback(env, obj, callback->obj(),
request_id);
Java_AwContents_invokeVisualStateCallback(env, obj, *callback, request_id);
}
} // namespace

Expand Down
2 changes: 1 addition & 1 deletion android_webview/native/aw_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ class AwContents : public FindHelper::Listener,
void SetSaveFormData(bool enabled);

// Sets the java client
void SetAwAutofillClient(jobject client);
void SetAwAutofillClient(const base::android::JavaRef<jobject>& client);

void SetJsOnlineProperty(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
Expand Down
12 changes: 7 additions & 5 deletions android_webview/native/aw_contents_background_thread_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,21 @@

#include "jni/AwContentsBackgroundThreadClient_jni.h"

using base::android::JavaRef;

namespace android_webview {

// static
base::android::ScopedJavaLocalRef<jobject>
AwContentsBackgroundThreadClient::shouldInterceptRequest(
JNIEnv* env,
jobject obj,
jstring url,
const JavaRef<jobject>& obj,
const JavaRef<jstring>& url,
jboolean isMainFrame,
jboolean hasUserGesture,
jstring method,
jobjectArray requestHeaderNames,
jobjectArray requestHeaderValues) {
const JavaRef<jstring>& method,
const JavaRef<jobjectArray>& requestHeaderNames,
const JavaRef<jobjectArray>& requestHeaderValues) {
return Java_AwContentsBackgroundThreadClient_shouldInterceptRequestFromNative(
env, obj, url, isMainFrame, hasUserGesture, method, requestHeaderNames,
requestHeaderValues);
Expand Down
10 changes: 5 additions & 5 deletions android_webview/native/aw_contents_background_thread_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ class AwContentsBackgroundThreadClient {
public:
static base::android::ScopedJavaLocalRef<jobject> shouldInterceptRequest(
JNIEnv* env,
jobject obj,
jstring url,
const base::android::JavaRef<jobject>& obj,
const base::android::JavaRef<jstring>& url,
jboolean isMainFrame,
jboolean hasUserGesture,
jstring method,
jobjectArray requestHeaderNames,
jobjectArray requestHeaderValues);
const base::android::JavaRef<jstring>& method,
const base::android::JavaRef<jobjectArray>& requestHeaderNames,
const base::android::JavaRef<jobjectArray>& requestHeaderValues);
};

}
Expand Down
5 changes: 3 additions & 2 deletions android_webview/native/aw_contents_client_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ void RecordClientCertificateKey(

} // namespace

AwContentsClientBridge::AwContentsClientBridge(JNIEnv* env, jobject obj)
AwContentsClientBridge::AwContentsClientBridge(JNIEnv* env,
const JavaRef<jobject>& obj)
: java_ref_(env, obj) {
DCHECK(obj);
DCHECK(!obj.is_null());
Java_AwContentsClientBridge_setNativeContentsClientBridge(
env, obj, reinterpret_cast<intptr_t>(this));
}
Expand Down
3 changes: 2 additions & 1 deletion android_webview/native/aw_contents_client_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ namespace android_webview {
// any references.
class AwContentsClientBridge : public AwContentsClientBridgeBase {
public:
AwContentsClientBridge(JNIEnv* env, jobject obj);
AwContentsClientBridge(JNIEnv* env,
const base::android::JavaRef<jobject>& obj);
~AwContentsClientBridge() override;

// AwContentsClientBridgeBase implementation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void AwContentsClientBridgeTest::SetUp() {
ASSERT_TRUE(net::android::RegisterJni(env_));
jbridge_.Reset(env_,
Java_MockAwContentsClientBridge_getAwContentsClientBridge(env_).obj());
bridge_.reset(new AwContentsClientBridge(env_, jbridge_.obj()));
bridge_.reset(new AwContentsClientBridge(env_, jbridge_));
selected_cert_ = nullptr;
cert_selected_callbacks_ = 0;
cert_request_info_ = new net::SSLCertRequestInfo;
Expand Down
12 changes: 4 additions & 8 deletions android_webview/native/aw_contents_io_thread_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,10 @@ std::unique_ptr<AwWebResourceResponse> RunShouldInterceptRequest(
"shouldInterceptRequest");
ScopedJavaLocalRef<jobject> ret =
AwContentsBackgroundThreadClient::shouldInterceptRequest(
env,
obj.obj(),
web_request.jstring_url.obj(),
web_request.is_main_frame,
web_request.has_user_gesture,
web_request.jstring_method.obj(),
web_request.jstringArray_header_names.obj(),
web_request.jstringArray_header_values.obj());
env, obj, 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);
return std::unique_ptr<AwWebResourceResponse>(
ret.is_null() ? nullptr : new AwWebResourceResponseImpl(ret));
}
Expand Down
2 changes: 1 addition & 1 deletion android_webview/native/aw_contents_statics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace {
void ClientCertificatesCleared(ScopedJavaGlobalRef<jobject>* callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
JNIEnv* env = AttachCurrentThread();
Java_AwContentsStatics_clientCertificatesCleared(env, callback->obj());
Java_AwContentsStatics_clientCertificatesCleared(env, *callback);
}

void NotifyClientCertificatesChanged() {
Expand Down
2 changes: 1 addition & 1 deletion android_webview/native/aw_message_port_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ void AwMessagePortServiceImpl::OnMessageChannelCreated(
if (obj.is_null())
return;
Java_AwMessagePortService_onMessageChannelCreated(env, obj, *port1, *port2,
ports->obj());
*ports);
}

// Adds a new port to the message port service.
Expand Down
10 changes: 5 additions & 5 deletions android_webview/native/aw_pdf_exporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@
#include "printing/units.h"

using base::android::JavaParamRef;
using base::android::JavaRef;
using base::android::ScopedJavaLocalRef;

namespace android_webview {

AwPdfExporter::AwPdfExporter(JNIEnv* env,
jobject obj,
const JavaRef<jobject>& obj,
content::WebContents* web_contents)
: java_ref_(env, obj),
web_contents_(web_contents) {
DCHECK(obj);
: java_ref_(env, obj), web_contents_(web_contents) {
DCHECK(!obj.is_null());
Java_AwPdfExporter_setNativeAwPdfExporter(
env, obj, reinterpret_cast<intptr_t>(this));
}
Expand Down Expand Up @@ -59,7 +59,7 @@ int MilsToDots(int val, int dpi) {
} // anonymous namespace

void AwPdfExporter::InitPdfSettings(JNIEnv* env,
jobject obj,
const JavaRef<jobject>& obj,
printing::PrintSettings& settings) {
int dpi = Java_AwPdfExporter_getDpi(env, obj);
int width = Java_AwPdfExporter_getPageWidth(env, obj);
Expand Down
4 changes: 2 additions & 2 deletions android_webview/native/aw_pdf_exporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace android_webview {
class AwPdfExporter {
public:
AwPdfExporter(JNIEnv* env,
jobject obj,
const base::android::JavaRef<jobject>& obj,
content::WebContents* web_contents);

~AwPdfExporter();
Expand All @@ -36,7 +36,7 @@ class AwPdfExporter {

private:
void InitPdfSettings(JNIEnv* env,
jobject obj,
const base::android::JavaRef<jobject>& obj,
printing::PrintSettings& settings);
void DidExportPdf(int fd, bool success);

Expand Down
20 changes: 10 additions & 10 deletions android_webview/native/aw_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ AwSettings::~AwSettings() {

JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> scoped_obj = aw_settings_.get(env);
jobject obj = scoped_obj.obj();
if (!obj) return;
Java_AwSettings_nativeAwSettingsGone(env, obj,
if (scoped_obj.is_null())
return;
Java_AwSettings_nativeAwSettingsGone(env, scoped_obj,
reinterpret_cast<intptr_t>(this));
}

Expand Down Expand Up @@ -122,10 +122,10 @@ void AwSettings::UpdateEverything() {
JNIEnv* env = base::android::AttachCurrentThread();
CHECK(env);
ScopedJavaLocalRef<jobject> scoped_obj = aw_settings_.get(env);
jobject obj = scoped_obj.obj();
if (!obj) return;
if (scoped_obj.is_null())
return;
// Grab the lock and call UpdateEverythingLocked.
Java_AwSettings_updateEverything(env, obj);
Java_AwSettings_updateEverything(env, scoped_obj);
}

void AwSettings::UpdateEverythingLocked(JNIEnv* env,
Expand Down Expand Up @@ -260,11 +260,11 @@ void AwSettings::PopulateWebPreferences(WebPreferences* web_prefs) {
JNIEnv* env = base::android::AttachCurrentThread();
CHECK(env);
ScopedJavaLocalRef<jobject> scoped_obj = aw_settings_.get(env);
jobject obj = scoped_obj.obj();
if (!obj) return;
if (scoped_obj.is_null())
return;
// Grab the lock and call PopulateWebPreferencesLocked.
Java_AwSettings_populateWebPreferences(
env, obj, reinterpret_cast<jlong>(web_prefs));
Java_AwSettings_populateWebPreferences(env, scoped_obj,
reinterpret_cast<jlong>(web_prefs));
}

void AwSettings::PopulateWebPreferencesLocked(JNIEnv* env,
Expand Down
14 changes: 6 additions & 8 deletions android_webview/native/aw_web_contents_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,15 @@ void AwWebContentsDelegate::RunFileChooser(
DCHECK_EQ(FileChooserParams::Open, params.mode);
}
Java_AwWebContentsDelegate_runFileChooser(
env, java_delegate.obj(), render_frame_host->GetProcess()->GetID(),
env, java_delegate, render_frame_host->GetProcess()->GetID(),
render_frame_host->GetRoutingID(), mode_flags,
ConvertUTF16ToJavaString(
env, base::JoinString(params.accept_types, base::ASCIIToUTF16(",")))
.obj(),
params.title.empty() ? NULL
: ConvertUTF16ToJavaString(env, params.title).obj(),
env, base::JoinString(params.accept_types, base::ASCIIToUTF16(","))),
params.title.empty() ? nullptr
: ConvertUTF16ToJavaString(env, params.title),
params.default_file_name.empty()
? NULL
: ConvertUTF8ToJavaString(env, params.default_file_name.value())
.obj(),
? nullptr
: ConvertUTF8ToJavaString(env, params.default_file_name.value()),
params.capture);
}

Expand Down
9 changes: 4 additions & 5 deletions android_webview/native/input_stream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,12 @@ bool InputStreamImpl::Read(net::IOBuffer* dest, int length, int* bytes_read) {

int remaining_length = length;
char* dest_write_ptr = dest->data();
jbyteArray buffer = buffer_.obj();
*bytes_read = 0;

while (remaining_length > 0) {
const int max_transfer_length = std::min(remaining_length, kBufferSize);
const int transfer_length = Java_InputStreamUtil_read(
env, jobject_, buffer, 0, max_transfer_length);
env, jobject_, buffer_, 0, max_transfer_length);
if (transfer_length == kExceptionThrownStatusCode)
return false;

Expand All @@ -105,7 +104,7 @@ bool InputStreamImpl::Read(net::IOBuffer* dest, int length, int* bytes_read) {
continue;

DCHECK_GE(max_transfer_length, transfer_length);
DCHECK_GE(env->GetArrayLength(buffer), transfer_length);
DCHECK_GE(env->GetArrayLength(buffer_.obj()), transfer_length);

// This check is to prevent a malicious InputStream implementation from
// overrunning the |dest| buffer.
Expand All @@ -114,8 +113,8 @@ bool InputStreamImpl::Read(net::IOBuffer* dest, int length, int* bytes_read) {

// Copy the data over to the provided C++ IOBuffer.
DCHECK_GE(remaining_length, transfer_length);
env->GetByteArrayRegion(buffer, 0, transfer_length,
reinterpret_cast<jbyte*>(dest_write_ptr));
env->GetByteArrayRegion(buffer_.obj(), 0, transfer_length,
reinterpret_cast<jbyte*>(dest_write_ptr));
if (ClearException(env))
return false;

Expand Down
2 changes: 1 addition & 1 deletion android_webview/native/input_stream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class InputStreamImpl : public InputStream {
~InputStreamImpl() override;

// Gets the underlying Java object. Guaranteed non-NULL.
jobject jobj() const { return jobject_.obj(); }
const base::android::JavaRef<jobject>& jobj() const { return jobject_; }

// InputStream implementation.
bool BytesAvailable(int* bytes_available) const override;
Expand Down

0 comments on commit 1c12b81

Please sign in to comment.