[rfc][icedtea-web] offline support

[Andrew Azores]

On 08/06/2014 08:43 AM, Jiri Vanek wrote:
> On 08/05/2014 05:04 PM, Andrew Azores wrote:
>> On 08/05/2014 11:01 AM, Jiri Vanek wrote:
>>> On 08/05/2014 04:50 PM, Andrew Azores wrote:
>>>> On 08/04/2014 11:34 AM, Jiri Vanek wrote:
>>>>> +    public static boolean isOnline() {
>>>>> +        if (isOfflineForced()) {
>>>>> +            return !isOfflineForced();
>>>>> +        }
>>>>> +        return onlineDetected;
>>>>> +    }
>>>>
>>>> This hunk is a little bit silly IMO :) that first return can just 
>>>> be "return false".
>>>
>>> will be fixed:
>>>
>>>  public static boolean isOnline() {
>>>         if (isOfflineForced()) {
>>>             return false;
>>>         }
>>>         return isOnlineDetected();
>>>     }
>>>
>>> (there was hidden NPE)
>>>
>>>>
>>>> I'm also not a fan (and I'm sure you're not surprised here) of 
>>>> onlineDetected being a Boolean, but
>>>> since the null value is only ever used in internal state and all 
>>>> the outward facing accessors use
>>>> plain boolean, I guess it's okay.
>>>
>>> I thought you will be complaining and I had this internall usage as 
>>> ace in sleeve.
>>>>
>>>> Otherwise... I can't find much else to complain about. It seems to 
>>>> work fine and I generally like
>>>> the patch. There is one thing I'm wondering about though. Using 
>>>> this patch with a large application
>>>> (in this case Elluminate), the application still takes quite some 
>>>> time to start up. Is this simply
>>>> because of how many local JARs there are to verify? Or maybe 
>>>> ResourceTracker is still sending HEAD
>>>> requests to try to check if the JARs are up to date... ?
>>>
>>> Yes. If you enable debug/show console. You wills see that *all* this 
>>> time is consumed by
>>> cerverifier.  Once offline is detected, no attempts  to verify  
>>> cahce are done. It should not
>>> touch network at all. However, there are few url.openConnections in 
>>> ITW about which usages I'm not
>>> sure what to do. I will investigate them later and try to replace 
>>> them (during testing, I have not
>>> seen them used when implementing this patch)
>>>
>>> I have run Elluminate during testing:)
>>>
>>> I have jsut found this method:
>>>
>>> /**
>>>      * Return true if the Environment is Offline
>>>      */
>>>     @Override
>>>     public boolean isOffline() {
>>>
>>>         URL url = findFirstURLFromJNLPFile();
>>>         URLConnection conn = null;
>>>         try {
>>>             conn = url.openConnection();
>>>             conn.getInputStream().close();
>>>             return false;
>>>         } catch (IOException exception) {
>>>             return true;
>>>         } finally {
>>>             if (conn != null && conn instanceof HttpURLConnection) {
>>>                 ((HttpURLConnection) conn).disconnect();
>>>             }
>>>         }
>>>     }
>>>
>>>
>>> I will shorten it via new is offline detection:
>>>
>>>
>>>     @Override
>>>     public boolean isOffline() {
>>>
>>>         URL url = findFirstURLFromJNLPFile();
>>>         JNLPRuntime.detectOnline(url);
>>>         return !JNLPRuntime.isOnline();
>>>     }
>>>
>>>
>>> Ok for head? I will prepare 1.5 version (much shorter) later today.
>>>
>>>
>>> J.
>>
>> All looks/sounds good to me, ok for HEAD.
>>
>> Thanks,
>>
>
> Here is 1.5 version. Head's one did not applied at all so I wrote it 
> again. I hope I did not messed it.
> In this version the Xservice is untouched, Xoflfine is missing and so 
> allowOflfine can not be avoided.
> The forceOffline was kept for debuging purposes and is missing the setter
>
> Otherwise it *should* be same.
>
> J.

Looks OK to me.

>
> Once this is (no rush!) approved, I would like to freeze branch and 
> sent tarball to you and Lukas and Jie for testing.  Andrew, although 
> you are officially leaving 8th, will you be able to lead them ? I'm 
> afraid the burden of 1.5 testing is completely in Toronto this time.
>
> JJ.

Sure.

Thanks,

-- 
Andrew A