New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 826988 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 781764



Sign in to add a comment

dupeMethod uses the wrong ClassLoader

Project Member Reported by ntfschr@chromium.org, Mar 29 2018

Issue description

createInvocationHandlerFor() is documented [1] as delegating method calls to the |delegate| parameter, looking up methods in that object's ClassLoader (more precisely: methods on the corresponding BoundaryInterface defined in delegate's ClassLoader).

However, dupeMethod() is implemented such that it uses Class.forName(String) [2], which looks in the "currentLoader," defined as the ClassLoader for the current class (in this case, the version of BoundaryInterfaceReflectionUtil from either the app-side or chromium-side, depending where it's called from).

This means that invoking createInvocationHandlerFor on the chromium side for an app-defined object will return a bad InvocationHandler, since dupeMethod() will return chromium-side Methods instead of the app-side Methods that the app-side object actually has.

This bug should be invisible when creating an InvocationHandler on the same side as the object is defined (which is what gsennton@ has done so far), but will be a RuntimeException when creating an InvocationHandler on the opposite side (which we need for WebViewClientCompat).

[1] https://cs.chromium.org/chromium/src/android_webview/support_library/boundary_interfaces/src/org/chromium/support_lib_boundary/util/BoundaryInterfaceReflectionUtil.java?l=51&rcl=19fcd2685b4af798be5d736627881c1a8c091320
[2] https://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#forName(java.lang.String)

 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6aed4c0fe3d8eeb3ae7f2bdd32cc6433cdf929c

commit c6aed4c0fe3d8eeb3ae7f2bdd32cc6433cdf929c
Author: Nate Fischer <ntfschr@chromium.org>
Date: Thu Mar 29 21:36:13 2018

AW: use correct ClassLoader in dupeMethod

This fixes an issue with the support library where we would sometimes
create an InvocationHandler and look up methods using the wrong
ClassLoader. This ensures we always use the correct ClassLoader.

dupeMethod() is only used here, so it's safe to change the signature.

See the crbug for more information.

Bug:  826988 
Test: Manual, did this in my demo CL at http://crrev/c/965883
Change-Id: I7ca7bb90aec81a6a30ce4d6393cf40ee54d40c19
Reviewed-on: https://chromium-review.googlesource.com/985631
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546961}
[modify] https://crrev.com/c6aed4c0fe3d8eeb3ae7f2bdd32cc6433cdf929c/android_webview/support_library/boundary_interfaces/src/org/chromium/support_lib_boundary/util/BoundaryInterfaceReflectionUtil.java

Status: Fixed (was: Assigned)
No manual verification necessary.
Status: Verified (was: Fixed)
Bulk edit: marking stale 'fixed' bugs as 'verified' since they don't need verification at this point.

Sign in to add a comment