[rfc][icedtea-web] policytool in itweb-settings

[Andrew Azores]

On 01/20/2014 07:27 AM, Jiri Vanek wrote:
> On 01/17/2014 10:08 PM, Andrew Azores wrote:
>> On 01/17/2014 11:52 AM, Jiri Vanek wrote:
>>>> This is why I was launching PolicyTool in a new process in the 
>>>> first place, actually. In all its
>>>> wisdom, PolicyTool exits the JVM when its window closes.
>>>
>>> It should not be so hard -
>>> By refelction get 'tw' field, and remove the  only (nasty words) 
>>> WindowListener it have - the one
>>> responsible for system.exit and nothing else:-/
>>>
>>> Unluckily, this listener is added in
>>> PolicyTool.main in tw.displayToolWindow(args);
>>>
>>> So you have to work on it a bit more. But again, nothing hard, and 
>>> no copy pasting;)
>>>
>>> Not sure if we will backport this, but probably yes.. Depends on 
>>> final compelxiity.
>>
>> 'tw' isn't a field though, it's just a local variable within main(), 
>> so we can't just grab it via
>> reflection.
>>
>> I've got the hackiest hack ever with reflection to get around this, 
>> and it is *seriously* ugly.
>> System.exit(0) is also called by PolicyTool indirectly in other 
>> places. I've removed the
>> ToolWindowListener so that hitting the window close button on 
>> PolicyTool doesn't exit the JVM, but
>> File>Exit still exits the JVM. This case comes from 
>> ToolDialog#userSaveContinue I believe, and I
>> don't see a way to work around this, really. We're going to need to 
>> do some extend/override voodoo
>> to get rid of this, unless we start introducing runtime code 
>> injection/modification in
>> itweb-settings control panel :) either that, or get some patches into 
>> the JDK to fix PolicyTool.
>>
>> The attached patch is the super crazy ugly hack that at least makes 
>> the window close button behave
>> nicely, but I feel like a monster for even having written this. Be 
>> warned.
>>
>> Oh, and try this - launch itweb-settings from terminal. Open 
>> policytool from itweb-settings. Close
>> policytool with the window close button. Press Ok in itweb-settings. 
>> The itweb-settings process
>> doesn't even exit! Wonderful! I'm not really sure what's keeping it 
>> open.
>>
>> Something else I noticed while testing this is that policytool 
>> prompts you if you want to save your
>> changes if you select File>Exit, but simply closing the window does 
>> not prompt you. This is without
>> the reflection patch applied. Ugh. PolicyTool is really in need of a 
>> serious overhaul...
>>
>
> Ouch. See my reply to patch.

Didn't see any mail, but we discussed on IRC. Here's a recap:
- The reflection hack is horrible. It doesn't quite work, and it's super 
ugly.
- The process not exiting is definitely a problem. We should not be 
playing around with reflection on the policytool like this.
- Calling PolicyTool.main() directly breaks the automated build(?). We 
don't want to make it a build dep, and it's too minor to bother with a 
configure step/flag(?).
- Solution: Runtime#exec() makes its return, with a fallback method of 
reflectively invoking PolicyTool.main() if for any reason the exec fails 
(non-POSIX system, executable not on PATH, etc)

So here's a patch implementing this solution, with lots of extra added 
documentation.

>>> ok. Ok to head, and please elaborate on backport to 1.4 it is worthy.
>>> Also please fix the closing of window asap (as new chngeset)
>>>
>>>
>>> Good work!
>>>  J.
>>
>> 1.4 doesn't have DirectoryValidator... what should we do here then? 
>> Also backport
>> DirectoryValidator? Or just let the 1.4 backport have less of a 
>> guarantee that the policy tool will
>> be able to successfully launch, and try to provide useful errors if not?
>>
>> Thanks,
>>
>
> Dependes on you. I'm for *not* backporting the directory validator, 
> and live with "maybe not readable file". But also I would just ensure 
> the parent dir creation.
>
> Thasnx!
>
>
> J.
>

I'm also not in favour of backporting directory validator, not if this 
is the only thing that's going to be using it. I'll introduce the 
backport to 1.4 for this soon.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: custompolicy-exec-with-fallback.patch
Type: text/x-patch
Size: 12855 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140120/ccffd323/custompolicy-exec-with-fallback-0001.patch