Bug 246068 - [Net] Add a facility to set a custom proxy provider
Summary: [Net] Add a facility to set a custom proxy provider
Status: RESOLVED DUPLICATE of bug 257443
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.5   Edit
Hardware: PC All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Pawel Pogorzelski CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-03 06:57 EDT by Pawel Pogorzelski CLA
Modified: 2009-03-11 05:54 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pawel Pogorzelski CLA 2008-09-03 06:57:18 EDT
Build ID: M20080827-2000

Steps To Reproduce:
There is a design issue with the jWinHttp-1.0.0.dll library that provides native proxy information on Windows platform. The library is just a thin wrapper that maps Java calls to WinHttp API calls. It doesn't contain a lot of logic.

This is a contrary to the implementation of the native library on GNOME. In this case libgnomeproxy-1.0.0.so maps OS structures to Java classes which significantly simplifies library use.

This should be unified, that is native libraries should implement the same header file. This would give less platform dependent code on the Java side. Also, more appropriate/natural implementation of the funcionality on Windows platform could be provided.

More information:
Comment 1 Pawel Pogorzelski CLA 2008-09-03 07:00:35 EDT
CCing Stefan since he provided the first implementation of the library.
Comment 2 Stefan Liebig CLA 2008-09-03 09:04:56 EDT
The decision that led to my design is very simple:
Do as least as possible within native code (except you like programming in c/c++ more than java ;-)
Comment 3 Pawel Pogorzelski CLA 2008-09-03 09:38:52 EDT
Honestly, JNI programming isn't my favourite way of spending time too :) But anyway, Stefan do you agree that the initial design should be changed?

There is also a drawback. More native code introduces more chances of errors on JNI side which could even led to JVM crash. This could be reduced to minimum through more elaborate testing of the native code. I think that the simple and flexible Java side code is worth implementing the change.
Comment 4 Stefan Liebig CLA 2008-09-04 06:57:18 EDT
My understanding of the original design is that the ProxyProvider class is the API. Any provider has just to implement that thing - no matter how. And I think this is a good decision because it gives implementers the most flexibility.

I am very sure if we would move all the Java Code dealing with the PAC and WPAD and proxy exception lists (this is the: do not use proxies for) into c/c++ that this will be a long and painful path.
Comment 5 Pawel Pogorzelski CLA 2008-09-05 04:42:55 EDT
Stefan, IProxyService is an API and the rest of the code just provides implementation for that interface.

My point is that if we use platform dependent code lets do it in native code. This is just a design issue. Refactoring the code would provide clean separation - common logic in Java code and OS specific stuff in native libraries. Current approach doesn't provide such separation.

Also, why shouldn't we implements OS specific stuff in a more appropriate way that is given by native languages. For example is it better to call getLastError() (Windows SDK function) through JNI or hide it in native library? IMO the latter is better.

As much common platform independent Java code as possible and clean points where native libraries could plug in is the way to go. Are you convinced now, Stefan?
Comment 6 Stefan Liebig CLA 2008-09-05 05:48:48 EDT
Pawel, I completely understand this design decision.

I am just saying, that the win32 side of the story is much more "complex" than the linux/gnome side. And that is is much easier to do that "complex" stuff in java instead of c/c++.

So, you got my +1 for refactoring it.
Comment 7 Pawel Pogorzelski CLA 2008-09-05 06:24:52 EDT
OK, thanks for the comment Stefan. I could misunderstood the comment 4, now everything is clear.
Comment 8 Pawel Pogorzelski CLA 2008-09-05 10:44:04 EDT
Refactored native libraries have to provide information about the proxy source (for example Windows IE Setting, Linux environment variables or GNOME settings). This was introduced in bug 246072 but has to be moved to native code when fixing this bug.
Comment 9 Pawel Pogorzelski CLA 2008-11-24 13:21:58 EST
Fix for this bug will have to make sure that new proxy providers use constants from IProxyData. See bug 249733 for details.
Comment 10 Mark A. Ziesemer CLA 2008-12-04 09:54:32 EST
Regarding all the comments regarding native vs. Java code:

+1 to Stefan's comment #2 and comment #4.  I think keeping as much code in the product's primary language (in this case, Java!) makes it easier to debug when needed.  It'll probably be more receptive for community support as well, e.g. receiving patches.

I would contend that JNI should only be used when necessary (no Java API available), and possibly for any performance-critical operations that Java doesn't already handle well (very rare today).  The requirement and lack of an existing API here does require some JNI, but I don't see any performance advantages.

As long as the platform-specific code is factored into separate packages / folders, etc., what is the disadvantage to using a little extra Java code to adapt it to the API in use?  The various platforms work different, so it should be expected that their native code will need to be different.

As there are minor changes / additions to the API, I think it would definitely be easier to first have a chance to bring the implementations up-to-date using Java, rather than having to dive straight into JNI for multiple platforms.


See also bug 257443.  It could be ideal to support providers by extension points.  This could also allow each platform-specific implementation to be provided by its own package - and make it clear whether or not platform-specific support is available.
Comment 11 Pawel Pogorzelski CLA 2008-12-04 10:32:58 EST
Guys, got your points. But even if we keep the existing approach (native libraries as thin wrappers on OS API) some steps has to be performed to clearly separate platform specific/optional code. Then a facility to set a custom proxy provider could be introduced more easily.

Changing abstract to:
[Net] Add a facility to set a custom proxy provider
Comment 12 Pawel Pogorzelski CLA 2009-01-21 08:03:34 EST
Fixing this bug will allow bug 106586 to be fixed by supplying a custom proxy provider. Bug 106586 is about proxy settings per target host.
Comment 13 Pawel Pogorzelski CLA 2009-03-11 05:54:14 EDT

*** This bug has been marked as a duplicate of bug 257443 ***