[rfc][icedtea-web][policyeditor] Fix for duplicate codebases when launched from dialog

Andrew Azores aazores at redhat.com
Tue Jun 17 20:30:57 UTC 2014


On 06/17/2014 04:08 PM, Jie Kang wrote:
> Hello,
>
>> Hi,
>>
>> When launching PolicyEditor from a security dialog, the applet's
>> codebase may be added to the UI twice. This only affects the list UI -
>> each entry shares the same checkbox state, custom permissions, etc., and
>> will be written to file as one entry. This happens partially because
>> there was some missing validation that the element doesn't already exist
>> before it is re-added to the list, and partially because in another
>> place where validation was done, the check was performed outside of a
>> SwingUtilities.invokeLater block which would later update the UI with
>> the new element, without re-checking its presence. This could cause a
>> race condition where the element was not yet in the list when it was
>> checked but could have already been added by the time it the invokeLater
>> ran. This patch fixes these two errors.
>>
>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1850
>>
>
> Patch looks good. 0.5.
>
> Maybe add javadoc that duplicate codebase urls aren't accepted?
>
> Also I just noticed but there is javadoc in PolicyEditor on the weakReference use. Not sure what the accepted style is in regards to comments vs. Javadoc but I think implementation details like why the weakReference is used would be more appropriate as a comment rather than part of Javadoc.
>

Ehh, being pedantic, but Javadoc *are* just comments. It's just that 
your IDE and potentially the javadoc tool, if you use it, decide to 
handle specific comments in a special manner ;). I can remove the one 
extra asterisk that makes it trigger the special handling, but I don't 
think it's really a big deal stylistically. None of it is public API 
anyway. The WeakReference is fairly questionable on why it's needed 
anyway so for people using an IDE, having it as javadoc allows that 
extra note on it to be visible just by eg hovering the cursor over any 
appearance of the identifier, not just by jumping to view the 
definition. So I'm in favour of just leaving it as-is.

Thanks,

-- 
Andrew Azores


More information about the distro-pkg-dev mailing list