[rfc][icedtea-web] Check online detection even if checked before

[Jiri Vanek]

...snip...
>>>>
>>>>
>>>>
>>>> If -Xoffline is specified, I would like to keep current approach.
>>>>
>>>>
>>>> For the online detection, I'm really concerned about isOnline result (and
>>>> so
>>>> behaviour of system)
>>>>
>>>>
>>>> So brainstroming (valid only if no xOffline is specified):
>>>>
>>>> on begging assume environment is offline (no check was done)
>>>> The check on "is online" will be done always from now
>>>> If suddenly the resource is being avaiable online , switch to "online
>>>> verified" and consider
>>>> environment is online.
>>>> No more checks are done from here.
>>>>
>>>> with Xoffline - no check is done ever.
>>>>
>>>> What do you think?
>>>
>>> Hello,
>>>
>>> If I understood correctly, I think the change to make it behave like above
>>> would be simple:
>>>
>>> In JNLPRuntime:
>>>       public static void detectOnline(URL location) {
>>>           if (onlineDetected != null) {
>>>               return;
>>>           }
>>>
>>> change:  if (onlineDetected != null) {
>>
>> [*] If so, then with related Boolean -> boolean change.
>
> Sorry can you explain this comment a little more?


If the null will be never used, then it have to type of boolean, not Boolean.  And I think it will 
not be. the default value now will be false.

  [***] I have now realized - how slow it will be if the environment really will be offline? It will 
keep "pinging" all resources, and actually dying with timeouts. ... You will have to measures this 
on sme biger (all will start offline with current appracha and mostly sometimes dye later with 
runtimeexception)

There may be hiddden performance drop. (as opposite to current state, where is performance up when 
you plug out the cable)
>
>>
>>>       to:  if (onlineDetected == true) {
>>>
>>>
>>> However, I wonder if we need to make it so complicated.
>>>
>>> I see two situations:
>>>
>>> A: Xoffline : no check is done ever and we never download : I think
>>> ResourceTracker.initializeResource() will need some patching for this. But
>>> I already wrote one, just haven't sent it yet.
>>>
>>> B: 'Online' : for each resource, try online and if it fails, try cache,
>>> otherwise error. This is already coded safely.
>> I doubt.
>>
>>>
>>>     My question here is: What is benefit of doing isOnline check? Where else
>>>     is this used outside of initializing and downloading resources?
>>
>> Find usages is your friend.
>>
>> Especially the one in resourceTracekr is imho critical.
>
> Hahah, I think the opposite. The ones outside of ResourceTracker are great, but the one in ResourceTracker is not as good.

And how will you proceed without them? I doubt it will work.
>
>>>
>>>     When we use URLConnection object and try to initialize the resource,
>>>     this already does a check for us. As well, the code already deals with
>>>     the possible situations here.
>>
>> This is not working as smotthly as you expect. Please be careful with any
>> fixes here, otherwise you
>> will break offline capabilities ITW with effort achieved.
>
> Can you explain a little more? Possibly provide some examples and describe what you mean by offline capabilities?

[**] That every part of ITW works even if you unplug the utp cable and turn of WiFi. Its crucial 
part of javaws implementation, which was long time (before offline patch) missing. As benefit, also 
appelts works offline. And it is something What I also wont to keep.

>
>
>> 	
>>>
>>>     I do not think the isOnline check before trying to use URLConnection
>>>     object has a performance benefit that justifies the extra complexity in
>>>     code.
>>
>> No it does not. But make life much easier.
>>>
>>>
>>>
>>> Regards,
>>
>>
>>
>> Looking back to the orriginal patch which introduced offline capabilities:
>>
>> isOnlineDetected
>> isOnline
>>     and
>> detectOnline
>>
>>
>> and their's usages -
>> 4 (+3 comfortable)
>> 2
>>     and
>> 3
>>
>> it is not much, but the whole concept of those have to be revisited. If you
>> grant that all the itw
>> javaws and applets will work offline, than you can do whatever you need. But
>> then I would recommand
>> you to remove the offline capabilities patch, and implement the whole design
>> on your own.
>
> I'd rather not remove the patch :\

But it may be best way, Remove it, and try to reimplemnt the offline solution with the bug you found 
fixed.
>
>>
>>
>> I'm quite afraid to change individual parts of this patch. And I do not wont
>> some functions to
>> return random values.
>>
>>
>> Even the change of yours  [* above] may have side effects.  But It may be
>> what is seek. Fix for your
>> issue, and still keeping  all the behaviours I would like to have.
>
> I know for sure the [*] change will fix the issue I am having. I also strongly think that it will keep the behaviours you'd like to have,

Ok. So well.. lets try it. (but be aware of [***])

I would like to know Jacob's opinion about [*] [**] and [***] :)

 > but this would need a bit more review. But, if you can specify exactly what behaviours you are 
looking for, then it will be easier to see if this fix works or not.
>

explained  in [**]

> At the same time, I still feel a call to detectOnline() shouldn't return just because it's been called before. I think the only cost of the original patch is in performance and in my opinion it is extremely small. It keeps pretty much everything you want, fixes my issue, and has one drawback that I can see, which is performance. However, how big of a performance drop is this? I don't think it's even close to noticeable.
>
>
> Regards,
>
>>
  J.