[rfc][icedtea-web] offline support

Andrew Azores aazores at redhat.com
Tue Aug 5 15:04:17 UTC 2014


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,

-- 
Andrew A



More information about the distro-pkg-dev mailing list