[rfc][icedtea-web] allow access to jars' attributes from jnlpfile class

[Jiri Vanek]

Small brekage, fix on fix....
On 11/08/2013 01:24 PM, Jiri Vanek wrote:
> On 11/07/2013 05:02 PM, Andrew Azores wrote:
>> On 11/07/2013 10:19 AM, Jiri Vanek wrote:
>>> As jnlp file is argument passed to various security dialogues, it should have access to attributes.
>>> This is continuation of
>>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2013-October/025070.html
>>>
>>> I do not have good feelings from patch,  but jnlpfile x jnlp classloader encapsulation ... or
>>> "encapsulation" is what it is :(
>>>
>>> As the result this patch is implementing
>>> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/manifest.html#app_name
>>>
>>> Also I have found
>>> http://icedtea.classpath.org/hg/icedtea-web/file/dcd51951d507/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java#l831
>>>
>>> So I would like to ask if this condition is correct. IMO not. There should be :
>>>  if (launchDesc != null) {
>>>     mainClass = launchDesc.getMainClass();
>>>  }
>>>
>>> Or am I missing something?
>>
>> I don't know, your suggestion seems to make sense but the JNLPClassLoader is just such a mess to
>> deal with that I'm not sure I really can okay this change :S. If you can produce some test cases to
>> show that this still works and doesn't cause problems...
>
> I will send this as separate patch. And cc Adam who introduced those lines.
>
>>
>>> Also there is a lot of "if (mainClass == null){work}if (mainClass == null){work} " as speed up
>>> there should be some "if (mainClass == null ) return"  hmm?
>>
>> I haven't looked deeply into it but I think this might break applets where they have a library JAR
>> and a plain .class for the main-class in the codebase, depending on where you're thinking of putting
>> it. Where do you mean to put this? And would it really make things much faster?
>
> Checked again, it is not worthy.
>>
>>>
>>>
>>> J.
>>
>> Little nits:
>> PluginBridge#getTitle :
>> - Spacing around '!=' in the first null check perhaps?
>> - Spelling in comment. "specification recommends main class instead of HTML parameter"
>>
>> ResourceDesc#getMainJar :
>> - 'else' on same line as closing brace of the 'if', just to be consistent with other code
>>
>> Other than that, I think it looks good - so long as the other two new attributes get implemented as
>> well :)
>
> Above fixed, also littl ebut improved (some logic moved from pluginBridge to jnlpfile)
>
> As this should go to 1.4 , please double check before aprove.
>
>
> J.
>
>
> 2013-11-08  Jiri Vanek  <jvanek at redhat.com>
>
>      Enabled access to manifests' attributes from JNLPFile class
>      Implemented http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/manifest.html#app_name
>      * netx/net/sourceforge/jnlp/JNLPFile.java: Added (manifestsAttributes) field.
>      Added (ManifestsAttributes) inner class, to encapsulate access to attributes.
>      (getTitle) can handle manifests too.
>      * netx/net/sourceforge/jnlp/PluginBridge.java: is following app_name recommendations.
>      * netx/net/sourceforge/jnlp/ResourcesDesc.java: (getMainJAR) made more granular
>      * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: (init) inject itself
>      to file's ManifestsAttributes. (checkForAttributeInJars) renamed field
>      mainClassInThisJar to attributeInThisJar. Added getter for mainClass.
>      * netx/net/sourceforge/jnlp/security/CertWarningPane.java: bracketing cleanup.
>      * tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java: new test to
>      check new functionalites
>      * tests/netx/unit/net/sourceforge/jnlp/runtime/ResourcesDescTest.java: same

-------------- next part --------------
diff -r 705ee12ba503 netx/net/sourceforge/jnlp/JNLPFile.java
--- a/netx/net/sourceforge/jnlp/JNLPFile.java	Fri Nov 08 13:14:52 2013 +0100
+++ b/netx/net/sourceforge/jnlp/JNLPFile.java	Fri Nov 08 14:04:26 2013 +0100
@@ -326,9 +326,6 @@
             return manifestTitle;
         }
         String mainClass = getManifestsAttributes().getMainClass();
-        if (mainClass == null){
-            return "Unnown application title";
-        }
         return mainClass;        
     }
     
diff -r 705ee12ba503 tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java
--- a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java	Fri Nov 08 13:14:52 2013 +0100
+++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java	Fri Nov 08 14:04:26 2013 +0100
@@ -99,7 +99,7 @@
 
         Assert.assertNull(jnlpFile.getTitleFromJnlp());
         Assert.assertNull(jnlpFile.getTitleFromManifest());
-        Assert.assertNotNull(jnlpFile.getTitle());//some dummy value
+        Assert.assertNull(jnlpFile.getTitle());
 
         setTitle(jnlpFile);
 
@@ -111,7 +111,7 @@
 
         Assert.assertNull(jnlpFile.getTitleFromJnlp());
         Assert.assertNull(jnlpFile.getTitleFromManifest());
-        Assert.assertNotNull(jnlpFile.getTitle());//some dummy value
+        Assert.assertNull(jnlpFile.getTitle());
 
         final JNLPClassLoader classLoader = new JNLPClassLoader(jnlpFile, UpdatePolicy.ALWAYS);