Pass method parameters as JavaParamRef in android_webview.

Pass method parameters as JavaParamRef in android_webview.

Pass all object parameters to JNI methods in JavaParamRef<> wrappers.
This matches previous changes made to do this for JNI non-method
functions.

BUG=506850
Committed: https://crrev.com/5728d85d64e4e6ff62c520cb2ddcf17cf17e7aaa
Cr-Commit-Position: refs/heads/master@{#363198}

[Torne, 2015-12-03 14:56:30]

torne@chromium.org changed reviewers:
+ sgurun@chromium.org

[Torne, 2015-12-03 14:56:30]

All changes scripted and git cl format'ed other than things I've specifically
called out below (pay most attention to these) :)

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents.cc:238:
AwContentsIoThreadClientImpl::Associate(web_contents_.get(),
manual change here: no need to construct a temporary ScopedJavaLocalRef any
more, can just pass the existing JavaRef.

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents_client_bridge.cc (right):

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents_client_bridge.cc:111: void
AwContentsClientBridge::ProceedSslError(JNIEnv* env,
manual change: converted this file directly to JavaRef, as with header file.
This required a few manual changes to use .is_null() and .obj() in appropriate
places.

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents_client_bridge.h (right):

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents_client_bridge.h:59: void
ProceedSslError(JNIEnv* env,
manual change: converted this file directly to JavaRef, to avoid doing horrible
things in the unit test code. JavaParamRef is just an intermediate step in the
migration (it allows implicit conversion back to jobject), so skipping it is
okay, I fixed the relevant code to not rely on the conversion.

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents_client_bridge_unittest.cc (right):

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents_client_bridge_unittest.cc:132:
bridge_->ProvideClientCertificateResponse(
manual change here to pass parameters as JavaRef instead of jobject.

[sgurun-gerrit only, 2015-12-03 22:21:38]

lgtm

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents.cc:238:
AwContentsIoThreadClientImpl::Associate(web_contents_.get(),
On 2015/12/03 14:56:30, Torne wrote:
> manual change here: no need to construct a temporary ScopedJavaLocalRef any
> more, can just pass the existing JavaRef.

yes. Although I did not see any reason for creating the temporary when it was
first written -- one could as well pass it as jobject.

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents_client_bridge.h (right):

https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents_client_bridge.h:59: void
ProceedSslError(JNIEnv* env,
On 2015/12/03 14:56:30, Torne wrote:
> manual change: converted this file directly to JavaRef, to avoid doing
horrible
> things in the unit test code. JavaParamRef is just an intermediate step in the
> migration (it allows implicit conversion back to jobject), so skipping it is
> okay, I fixed the relevant code to not rely on the conversion.

I think java_ref is not necessary for anything except
provideclientcertificateresponse. but either way should be ok.

[Torne, 2015-12-04 11:30:50]

On 2015/12/03 22:21:38, sgurun wrote:
> lgtm
> 
>
https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
> File android_webview/native/aw_contents.cc (right):
> 
>
https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
> android_webview/native/aw_contents.cc:238:
> AwContentsIoThreadClientImpl::Associate(web_contents_.get(),
> On 2015/12/03 14:56:30, Torne wrote:
> > manual change here: no need to construct a temporary ScopedJavaLocalRef any
> > more, can just pass the existing JavaRef.
> 
> yes. Although I did not see any reason for creating the temporary when it was
> first written -- one could as well pass it as jobject.

..because the whole point of having JavaRef is to pass java objects around using
it to be sure what the lifetimes of objects are. Nobody is supposed to be
passing things as jobject anywhere.

>
https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
> File android_webview/native/aw_contents_client_bridge.h (right):
> 
>
https://codereview.chromium.org/1492703004/diff/1/android_webview/native/aw_c...
> android_webview/native/aw_contents_client_bridge.h:59: void
> ProceedSslError(JNIEnv* env,
> On 2015/12/03 14:56:30, Torne wrote:
> > manual change: converted this file directly to JavaRef, to avoid doing
> horrible
> > things in the unit test code. JavaParamRef is just an intermediate step in
the
> > migration (it allows implicit conversion back to jobject), so skipping it is
> > okay, I fixed the relevant code to not rely on the conversion.
> 
> I think java_ref is not necessary for anything except
> provideclientcertificateresponse. but either way should be ok.

Everything will be converted to JavaRef in the end, I just don't want to do all
the work for it now. Find-and-replace on a whole file where needed is easier
than fixing individual cases.

[Torne, 2015-12-04 11:30:57]

The CQ bit was checked by torne@chromium.org

[commit-bot: I haz the power, 2015-12-04 11:33:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492703004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492703004/1

[commit-bot: I haz the power, 2015-12-04 11:48:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-04 11:48:32]

Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)

[Torne, 2015-12-04 13:42:42]

The CQ bit was checked by torne@chromium.org

[Torne, 2015-12-04 13:42:43]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org
Link to the patchset: https://codereview.chromium.org/1492703004/#ps20001
(title: "forgot to test compile unit tests before uploading, whoops")

[commit-bot: I haz the power, 2015-12-04 13:42:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492703004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492703004/20001

[commit-bot: I haz the power, 2015-12-04 14:10:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-04 14:11:24]

Description was changed from

==========
Pass method parameters as JavaParamRef in android_webview.

Pass all object parameters to JNI methods in JavaParamRef<> wrappers.
This matches previous changes made to do this for JNI non-method
functions.

BUG=506850
==========

to

==========
Pass method parameters as JavaParamRef in android_webview.

Pass all object parameters to JNI methods in JavaParamRef<> wrappers.
This matches previous changes made to do this for JNI non-method
functions.

BUG=506850

Committed: https://crrev.com/5728d85d64e4e6ff62c520cb2ddcf17cf17e7aaa
Cr-Commit-Position: refs/heads/master@{#363198}
==========

[commit-bot: I haz the power, 2015-12-04 14:11:24]

Patchset 2 (id:??) landed as
https://crrev.com/5728d85d64e4e6ff62c520cb2ddcf17cf17e7aaa
Cr-Commit-Position: refs/heads/master@{#363198}