[fyi] icedtea-web - making browsers tests more stable - softkiller, session backup and it integration to firefox

[Adam Domurad]

On 11/23/2012 06:02 AM, Jiri Vanek wrote:
> Hi!
>
> Pushed.
>
> I'm also attaching new integration patch for your softkiller, which 
> you will need later, when you will smuggle softkiller to ITW :)
> Please note especially last two lines of attached patch:
> -            //ProcessAssasin.closeWindows(s);
> +            ProcessAssasin.closeWindows(s);
>
> Those will enable the softkiller for firefox. Other integrations are 
> done.
>
> Although to have it enabled it now was making no real harm, it was 
> spamming the output with exceptions.
>
> J.
>
> On 11/21/2012 02:48 PM, Pavel Tisnovsky wrote:
>> Hi Jiri,
>>
>> First part:
>>   - I can't say anything about the softkiller itself ;-), let's have 
>> another pair of eyes to look at it
>>   - Makefile changes looks ok
>>
>> Second part:
>>   - All license headers are now indented by one more char in the diff 
>> - I'm not sure how it happened
>>     but it don't have to be part of this change
>>
>>   - I'm not sure if those new variables should or must be volatile 
>> because they are changed and read
>>     form two or more threads w/o synchronization:
>> +    //signifies that assasin have been summoned
>> +    private boolean killing = false;
>> +    //signifies that assasin have done its job
>> +    private boolean killed = false;
>>
>>   - Support for more listeners - OK
>>
>>   - listener.lineReaded()... AFAIK "readed" should be written as 
>> "read" because it is irregular verb ;)
>>
>>   - Following code is a bit strange, some copy&  paste error? :)
>> +        } catch (NullPointerException ex) {
>> +            ex.printStackTrace();
>> +        } //do not want to bother output with terminations
>> +        //mostly compaling when assassin kill the process about 
>> StreamClosed
>> +        //do not want to bother output with terminations
>> +        //mostly compaling when assassin kill the process about 
>> StreamClosed
>>
>> +    public static void closeWindow(String pid) throws Exception {
>> +        List<String>  ll = new ArrayList<String>(4);<-- should be 2 
>> here?
>> +        ll.add(ServerAccess.getInstance().getDir().getParent() + 
>> "/softkiller");
>> +        ll.add(pid);
>>
>> - +    //signifies that assasin have been summoned - haha we are 
>> going to transform IT-web into a game :D
>>
>> - diff -r c61b2db7d32f 
>> tests/test-extensions/net/sourceforge/jnlp/ProcessWrapper.java
>>    would be better to have some javadoc with explanation what this 
>> class does
>>
>> - huh?
>> +        f.delete();
>> +        ;
>> +        f.mkdir();
>>
>> In overall your big patch seems to be correct and giving that tests 
>> are now more stable
>> I think it's ok to push it to HEAD.
>>
>> Cheers,
>> Pavel
>>
>>
>>
>> ----- Jiri Vanek<jvanek at redhat.com>  wrote:
>>> Hi!
>>>
>>> This is another effort to make browser tests more stable.
>>>
>>> First part - softkiller - is Pavels attempt to close window "like 
>>> cross is clicked". Hmm It is working... I'm still not sure how. 
>>> Firefox looks to endure longer without safe mode, but eg opera can 
>>> not survive it. And eg streams from browsers are flushed more 
>>> properly. But also some lags are here which make it simetimes 
>>> unstable. It also contains its integration to ITW make processes.
>>> /me  is not taking any responsibility for this part :)
>>>
>>>
>>> Second part is oriented to usage of softkiller where possible and to 
>>> backuping and restoring of FF sessions(by backuping/restoring 
>>> profiles).
>>>
>>> To do this I had to do quite large refactoring, so the 
>>> "executeProcess" logic will possible to accept implementations of 
>>> some Interfaces, and then will be able to launch the interface's 
>>> methods.
>>> All the executeProcess have been moved to ProcessWraper class. (old 
>>> methods in ServerAccess like executeJavaws are now creating 
>>> ProcessWraper instance). All browsers now Implements 
>>> ReactableProcess, which can be called
>>> by executeProcess beforeProcess, afterProcess,beforeKill and 
>>> afterKill. Implementation is done just for firefox, but I believe it 
>>> will be helpful when other processes eill nedd some tunning. Usage 
>>> of interface's methods in firefox for backuping/restoring session is 
>>> quite strightforward then
>>>
>>>
>>> Any hints (especially to this early design) welcomed!
>>>
>>> J.
>>
>

Hi, lending some extra eyes.

The C portion looked good to me (very correctness-conscious too :). I 
cannot comment too deeply on the correctness of the API usage but it all 
seems in order.

Happy hacking,
- Adam