[rfc][icedtea-web] Security dialogs wait for PolicyEditor to finish parsing

[Andrew Azores]

On 03/24/2014 12:01 PM, Jiri Vanek wrote:
> On 03/24/2014 04:46 PM, Andrew Azores wrote:
>> On 03/24/2014 11:11 AM, Jiri Vanek wrote:
>>> On 03/24/2014 03:44 PM, Andrew Azores wrote:
>>>> On 03/24/2014 10:40 AM, Jiri Vanek wrote:
>>>>> On 03/24/2014 03:01 PM, Andrew Azores wrote:
>>>>>> Hi,
>>>>>>
>>>>>> PolicyEditor loads and saves policy files async, obviously. 
>>>>>> However, this means that opening a new
>>>>>> PolicyEditor instance on an existing policy file and 
>>>>>> programmatically adding a new codebase to the
>>>>>> editor does not have a well-defined order. This can be 
>>>>>> problematic because PolicyEditor
>>>>>> highlights/selects the last added codebase, so eg in the scenario 
>>>>>> where the editor is being
>>>>>> launched
>>>>>> from a security dialog, we want to be sure that the current 
>>>>>> applet's codebase is the one selected
>>>>>> when the editor appears. This patch adds a method to PolicyEditor 
>>>>>> that indicates whether the
>>>>>> editor
>>>>>> is currently reading/writing a file, and adds a loop to the two 
>>>>>> security dialogs so that the new
>>>>>> codebase is not added and the editor made visible until it 
>>>>>> completes its IO.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>
>>>>> Hm... I dont think this is correct approach. Why are you so 
>>>>> strongly against modal dialogue?
>>>>>
>>>>>
>>>>> J.
>>>>
>>>> I'm not... making PolicyEditor modal will come later. This patch is 
>>>> just about making sure that when
>>>> you launch PolicyEditor from a security dialog, the current 
>>>> applet's codebase is guaranteed to be
>>>> the selected one when the editor appears. This and modality are 
>>>> completely separate.
>>>>
>>>> Thanks,
>>>>
>>> uff... wouldnt be much more easy to make the load/save in sync?  I 
>>> do not see any reason why it is
>>> in new thread Thread(save/load).start() ....
>>>
>>>
>>> J.
>>
>> I really don't want to be doing it sync on the UI thread. This makes 
>> the application less
>> responsive, and is generally bad practice. Not worth the benefit of 
>> making it easier to guarantee
>> the programmatically added new codebases come after. There are better 
>> ways to do it async than
>> manually spawning a new Thread, though, yes. eg SwingWorker as you 
>> said on IRC, or Executors and
>> Runnables as I suggested. Still, these potential enhancements to the 
>> multithreading model would
>> still have the codebase ordering problem.
>
> I agree thath do it in gui thread is wrong. But I disagree with 
> current aproach and new suggested locking.
>
> Correct aporach is:
> invoke thread (swingworker)
>   disable gui or show dialog with progressbar or soem wait please
>    (so gui will be "responsoble")
> wait for thread.
>
>
> Would be correct approach.

Okay, this is an enhancement in that it uses the nice SwingWorker 
abstraction and provides a visual indicator of progress for the IO 
operation, but I don't see how it really solves the problem. If by "wait 
for thread" you mean Thread#join or similar, then we're again just 
blocking the EDT, so it won't actually be responsive until the IO is 
done. In that case we might as well just have the EDT do the IO 
directly. If we don't block the EDT and continue to let the IO complete 
async, albeit with visual progress indication and nicely abstracted, 
then there is still the problem of addNewCodebase being called 
externally and not having a well-defined order compared to the 
addNewCodebase calls made by the thread performing the IO.

>
>
> However - the polici files *are* small. So the burden of *doing it 
> IN*  AWT is imho acceptable.
>
>
> J.
>
>

Thanks,

-- 
Andrew A