[Webkit-unassigned] [Bug 57859] Add implementations of JavaInstance, JavaClass, JavaField and JavaMethod that simply proxy calls to a listener.

Thu Apr 7 11:22:37 PDT 2011

https://bugs.webkit.org/show_bug.cgi?id=57859

--- Comment #11 from Andrei Popescu <andreip at google.com>  2011-04-07 11:22:37 PST ---
Hi Alexey,

Thanks for the comments!

(In reply to comment #10)
> I think that implementation of remote Java objects should either be common with other remote objects, or it should at least be similar to how out of process NPAPI plug-ins are implemented.
>

Since we haven't adopted WebKit2 but we do want remote Java objects, would it be possible for us to make the implementation of remote Java objects be common with other remote objects? As far as I can tell, that would not work for us, so perhaps the best thing to do is your second choice: follow a somewhat similar pattern but keep them different?

> The former can be challenging, especially since some of out of process NPAPI plug-in code is not in open source. But the latter is something that should be considered if we don't want our heads explode.
> 

Heh, agreed :)

> I can see several differences with NPAPI here:
> 
> 1) WebKit IPC is implemented in WebKit, not in WebCore, so NPAPI remote objects also live in WebKit. With WebKit2, that is now cross-platform of course (for all platforms that adopt WebKit2). This distinction may seem minor, but I don't see what other platforms would do with proxy Java classes that are being added to WebCore here. And if it's only for Android, it should live in Android WebKit.
> 

Ok so how about we just leave the JavaInstance be an interface whose header lives in WebCore/bridge/v8? We can then have the proxy class live in WebKit?

> 2) NPAPI objects don't have a common class for local and remote objects. I don't see any reason for Java to have a common class other than code sharing - but inheritance is an anti-pattern for code sharing. Since these objects behave so differently, I think that JavaInstance and ProxyJavaInstance should be separate JSC::Bindings::Instance subclasses. This objection is more about bug 55383, in fact.
> 

But I think they can't be subclasses of JSC::Bindings::Instance since the Instance class is JSC-specific and we use V8. We inject our instances via NPAPI, which is just a V8-specific detail. This is why we have two JavaInstance classes: one in bridge/jsc and one in bridge/v8. 

> 3) If I'm reading this patch correctly, it uses yet another level of inheritance to actually implement IPC (e.g. invokeMethod is pure virtual). This use of inheritance to supply an implementation is against common WebKit practices - we would have a separate class to provide implementation. See e.g. the relationship between ProxyInstance and NetscapePluginInstanceProxy in WebKit/mac.
> 
> I'm not saying that out of process NPAPI implementation in WebKit/mac is necessarily the ideal, but it's the most modern code we have for Instance subclasses. If you prefer to do something differently, we should at least agree that the same would be better for NPAPI in the future.

If I understand things right:  since the V8-specific JavaInstance isn't really an Instance subclass, is this the right goal here? Instead, we can have the JavaInstance be a pure interface and have the implementations live in WebKit...

Thanks,
Andrei

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.