Actually, regarding the problem with java.lang.reflect.Proxy in 2c, I think this is not a problem, since we never need to present the glue-layer Interfaces to apps:

app -> Support Library -> support_library_glue -> AwContents

The app passes an
abstract class android.webviewcompat.WebViewClient
to the Support Library. The Support Library holds on to this and uses it to implement

interface support_library_glue.WebViewClient

the support_library_glue holds on to its support_library_glue.WebViewClient and uses that to implement AwContentsClient (similarly to what WebViewContentsClientAdapter does in the current frameworks-glue-layer).



I think the issue we might run into using support_library_glue interfaces instead of abstract classes is that whenever we introduce a new callback in the support_library_glue, old versions of the support library will not know how to implement a default version of that method.

Yeah, we don't need to use Proxy anywhere I don't think.

We're going to need to use the caller's support library version to determine what callbacks/etc to make on them, the same way we currently use OS version for that.

Re #2: but how else do we implement any interfaces from the WebView classloader (e.g. support_library_glue.WebViewClient) within the Support Library? All interfaces/classes imported from support_library_glue into the Support Library will differ from the interfaces/classes loaded into the WebView classloader, since at run-time the ones imported into the Support library will have been loaded through the app classloader, no?

If we use AIDL then there's no need to do that in the first place..

Good point, I just want to scope out both solutions (and make sure I understand their limitations) - I'll have a closer look at the AIDL solution now.

Yeah, sorry, that's a good idea. I don't really know a lot about how Proxy works I'm afraid :)

I've been looking a at how to do an AIDL implementation of this, and there are at least a couple of drawbacks:
1. we use some classes which (IMO) would be infeasible to make Parcelable - such as Context. I guess we could work around this by using reflection in addition to AIDL (but then maybe we should just always use reflection).
2. The standard way of using AIDL is to define a Service, connect to that Service, and communicate with it using AIDL. I'm not sure whether we can use AIDL without also introducing a new Service (it seems unnecessary to introduce a new Service just to be able to use AIDL).
3. For each callback / object (e.g. WebViewClient) provided by an embedding app, we would either have to make the class defining that callback/object Parcelable, or take care to always call back through AIDL from the WebView code out into the Support Library code, to call the callback / use the object. For example: say an app implements WebViewClient.shouldOverrideUrlLoading. We would either make WebViewClient Parcelable (and thus force the app-implementation of WebViewClient to be Parcelable as well), or we store our WebViewClient in the Support Library code, and somehow reference that instance (e.g. using some unique ID) when calling through AIDL into WebView code - then when the WebView code wants to use the callback, it would call back through AIDL into the support library code, passing the WebViewClient reference (probably some unique ID). This feels fairly convoluted IMO :/ (though we could automate the creation of the AIDL code for passing IDs).

I have a naive question, and I am guessing answer is obvious but...: Why do we have to make Context parcelable. Isn't it already a frameworks class?

put another way: if we use AIDL, do we need to make all frameworks classes we use for communication a parcelable?

AIDL parcels everything because it's intended for IPC, so yes, you can *only* pass things that can be parceled. There's no mechanism to pass the actual objects, because that's impossible in an IPC context. So, yeah, this suggests my thought of using AIDL here isn't really suitable.

Some framework types are already parcelable, but this only makes sense for value objects really - there's no meaningful way to parcel a Context because a Context represents a set of resources belonging to the current process.

Coming back to question 2d in the original post:
handling cases where the WebView APK passes some class (created by the apk) to the Support Library, e.g. WebResourceRequest in
shouldOverrideUrlLoading(WebView view, WebResourceRequest request).

Ideally we would want to pass some class which both the support library (app classloader), and the support library glue (webview classloader) have loaded. Since WebResourceRequest is defined in the framework we don't want to use that. In this specific case of replacing WebResourceRequest we could just use an android.os.Bundle since all the members in WebResourceRequest are parcelable, but in general we might have to define some class representing WebResourceRequest in either the support library, or in the glue, and then use reflection from the side which doesn't define that WebResourceRequest class, to load it.

As for 2e: I don't see how this (passing frameworks classes from outside android.webkit) could be an issue - if there is some feature that a specific class won't support at a specific Android version, then we (the WebView Support Library) shouldn't be able to support that feature anyway, since we can't communicate correctly with the framework anyway to make the feature work.

I think the only case I can think of as interesting for 2e is when Android introduces a feature in version X, and with it introduces some class which is passed through WebView APIs, but we want to support this feature in the support library for version Y (Y < X). I'm not sure what kind of such feature wouldn't require any communication with the rest of the framework though (outside android.webkit). If such a feature exists we would have to come up with our own version of that framework class, and pass it between support library and glue similarly to how we solve 2d.

aidl might be ok for data types (eg WebResourceRequest), but surely it won't work for things like WebViewClient, because I assume aidl makes a *copy* of the object, and for WebViewClient, we need to call methods on the original

and since it's trivial to turn a "data-only" type into one that's not data-only by adding more methods in the future, maybe it's not all that safe to use aidl for anything..

Yup, that's basically the same as comment 10. I don't think we're going to pursue using AIDL. It's possible to make that work by passing binders that can call back to the original object (lots of service APIs work this way and it's pretty transparent) but it's not really for the use case that we have.

hmm, the interface between support lib and apk will need to be kept stable. if it's all reflection in between, then we can't rely compiler doing this for us I think? :/

Yeah, it's probably going to be difficult to have this enforced by the compiler in that way :(

I'm not so sure about that - I was imagining that we could build our support library against he support_lib_glue layer normally, and then add a build-step where we convert the "normal" java calls into reflection calls instead.

I'm not entirely sure whether doing so is feasible yet, but I think it is definitely worth aiming for some kind of conversion step that would convert our code into reflection (whether or not that 'code' is normal Java code or some kind of template-code) - to avoid having to add that boiler-plate ourselves for each new method call, and to avoid bugs in the reflection itself.

One thing that came up during yesterday's sync meeting: apps can repackage/proguard their code so we have to make sure the support library works with apps doing so.

This could potentially affect the way we make calls between the support library and the Chromium glue layer. So we'll have to be careful with that implementation. More specifically, we probably can't use signatures of support library classes the webview apk is built against since those classes can be renamed in the actual app implementation.

If we do need to rely on naming/reflection/etc of our code we would need to provide a proguard flags file that can be used to preserve the parts that are important. Other libraries do this (play services at least?) though I'm not sure how automatic the mechanisms for it are (I'm not very familiar with how libraries are packaged and used in modern android development with gradle/android studio/etc).

Right, on second thought I think we should always be able to get the correct class-name at run-time - since we know which class we want to convert an object to (or which class we want to call into) we can just fetch its name at run-time instead of using any static names (we would have to be careful to ensure we are doing so though, probably add a test for this). Does this sound correct?

Hah, I did not realize you had already mentioned the proguarding thing here Torne :) (it came up in an email thread lately as well).

Anyways, the doc here:
https://docs.google.com/a/google.com/document/d/1wWAxd5EwZQBUReEqbnnyjpU475gK4VIzUTtS-oUgIQs/edit?usp=sharing
compares the two possible approaches. I'm leaning towards using InvocationHandlers and Proxies (even thought that's a more complex approach) because it very much hides implementation details of calling between support library and glue-layer better than the object+reflection approach (because we don't need reflection after passing an object across the boundary).

An alternative could be to avoid using reflection in the actual code, and then use code transformations to turn that code into Object + reflection calls.

Any thoughts on which approach to use?

It looks like using InvocationHandlers is the way to go here - if this turns out to be a performance issue (which should only happen for calls made very often), we can switch the affected calls to use reflection.