[icedtea-web] RFC: Patch to support plugin classloader sharing

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-03-04 16:56]:
> On 03/04/2011 03:55 PM, Deepak Bhole wrote:
<snip>
> >
> 
> Just to summarize (and to make sure I understand this): the key here
> is to create another ClassLoader. This ClassLoader only contains the
> codebase (can there be more than one?) and is only used as a last
> resort. This avoids contacting the server needlessly.
> 

Yep, that is absolutely correct!

> >ChangeLog:
> >2011-03-04  Deepak Bhole<dbhole at redhat.com>
> >
<snip>
> >+                        baseLoader.merge(loader);
> >+                }
> >                  loader = baseLoader;
> >              }
> >
> 
> This logic is getting quite convoluted. What's the purpose of this
> hunk (and the one before this)?
>

Yeah I am not a big fan of it either, but the else was necessary.

The original if says:

<If there is no loader or the loader with the same key as this one is for
another JNLP file>:

1. Create a new loader (CL)
2. Creation of CL may create additional extension loaders (jnlps can have
exts), so if any were created, ccumulate those, and merge it all into one.

The above logic is only valid for JNLPs, as plugins have no concept of
extension loaders. Even for applets from the same page, creation of the
initial loader does no trigger any additional loader creation. So I
added an else logic which says:

Else, if loader for given key (page) is not null:
1. Create a new loader for this applet
2. Merge contents of new loader into existing loader that exists for this
page


I think the "or the loader with the same key..." condition can probably be
removed, but it is outside the scope of this patch. I will take a look
at it later.

> >@@ -529,7 +541,7 @@
> >       * loaded from the codebase are not cached.
> >       */
> >      public void enableCodeBase() {
<snip>
> >+        }
> >+    }
> 
> Much better than those crazy reflection calls :)
> 

:)

> >  }
> >diff -r 64b9d3a8239c netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
> >--- a/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java	Thu Mar 03 17:56:00 2011 -0500

<snip>
> >+++ b/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java	Fri Mar 04 15:46:36 2011 -0500
> >@@ -199,8 +199,16 @@
> >                   appletPanels.removeElement(p);
> 
> Other than the change in JNLPClassLoader.getInstance, this looks fine.
> 

Great, thanks for reviewing this so quickly!

Cheers,
Deepak