Fix uses of GetByteArrayElements() in WebCoreFrameBridge

Currently, we call GetByteArrayElements() to get a pointer to the data, then
modify it, then fail to call ReleaseByteArrayElements. This means that if
GetByteArrayElements() happens to take a copy of the data, we'll never sync the
modified data back to the JNI object.

This change updates the code to use SetByteArrayRegion() which handles
releasing any copies that are made by JNI.

Also use ScopedLocalRef where possible.

Bug: 5442710
Change-Id: Ic8691eb18cac746ee2a2086017ffb9b5558755bd

1 files changed, 12 insertions, 18 deletions

diff --git a/Source/WebKit/android/jni/WebCoreFrameBridge.cpp b/Source/WebKit/android/jni/WebCoreFrameBridge.cpp
index 4350eb6..19740e2 100644
--- a/Source/WebKit/android/jni/WebCoreFrameBridge.cpp
+++ b/Source/WebKit/android/jni/WebCoreFrameBridge.cpp
@@ -984,7 +984,7 @@ WebFrame::didReceiveAuthenticationChallenge(WebUrlLoaderClient* client, const st
 #endif
 
 void
-WebFrame::reportSslCertError(WebUrlLoaderClient* client, int cert_error, const std::string& cert, const std::string& url)
+WebFrame::reportSslCertError(WebUrlLoaderClient* client, int error, const std::string& cert, const std::string& url)
 {
 #ifdef ANDROID_INSTRUMENT
     TimeCounterAuto counter(TimeCounter::JavaCallbackTimeCounter);
@@ -995,16 +995,14 @@ WebFrame::reportSslCertError(WebUrlLoaderClient* client, int cert_error, const s
         return;
     int jHandle = reinterpret_cast<int>(client);
 
+    // Don't copy the null terminator.
     int len = cert.length();
-    jbyteArray jCert = env->NewByteArray(len);
-    jbyte* bytes = env->GetByteArrayElements(jCert, NULL);
-    cert.copy(reinterpret_cast<char*>(bytes), len);
+    ScopedLocalRef<jbyteArray> jCert(env, env->NewByteArray(len));
+    env->SetByteArrayRegion(jCert.get(), 0, len, reinterpret_cast<const jbyte*>(cert.c_str()));
 
-    jstring jUrl = env->NewStringUTF(url.c_str());
+    ScopedLocalRef<jstring> jUrl(env, env->NewStringUTF(url.c_str()));
 
-    env->CallVoidMethod(javaFrame.get(), mJavaFrame->mReportSslCertError, jHandle, cert_error, jCert, jUrl);
-    env->DeleteLocalRef(jCert);
-    env->DeleteLocalRef(jUrl);
+    env->CallVoidMethod(javaFrame.get(), mJavaFrame->mReportSslCertError, jHandle, error, jCert.get(), jUrl.get());
     checkException(env);
 }
 
@@ -1059,12 +1057,10 @@ WebFrame::didReceiveData(const char* data, int size) {
     if (!javaFrame.get())
         return;
 
-    jbyteArray jData = env->NewByteArray(size);
-    jbyte* bytes = env->GetByteArrayElements(jData, NULL);
-    memcpy(reinterpret_cast<char*>(bytes), data, size);
+    ScopedLocalRef<jbyteArray> jData(env, env->NewByteArray(size));
+    env->SetByteArrayRegion(jData.get(), 0, size, reinterpret_cast<const jbyte*>(data));
 
-    env->CallVoidMethod(javaFrame.get(), mJavaFrame->mDidReceiveData, jData, size);
-    env->DeleteLocalRef(jData);
+    env->CallVoidMethod(javaFrame.get(), mJavaFrame->mDidReceiveData, jData.get(), size);
     checkException(env);
 }
 
@@ -1096,13 +1092,11 @@ void WebFrame::setCertificate(const std::string& cert)
         return;
 
     int len = cert.length();
-    jbyteArray jCert = env->NewByteArray(len);
-    jbyte* bytes = env->GetByteArrayElements(jCert, NULL);
-    cert.copy(reinterpret_cast<char*>(bytes), len);
+    ScopedLocalRef<jbyteArray> jCert(env, env->NewByteArray(len));
+    env->SetByteArrayRegion(jCert.get(), 0, len, reinterpret_cast<const jbyte*>(cert.c_str()));
 
-    env->CallVoidMethod(javaFrame.get(), mJavaFrame->mSetCertificate, jCert);
+    env->CallVoidMethod(javaFrame.get(), mJavaFrame->mSetCertificate, jCert.get());
 
-    env->DeleteLocalRef(jCert);
     checkException(env);
 }
 #endif