[RFC][icedtea-web] Fix PluginObjectStore.contains(Object object)
Adam Domurad
adomurad at redhat.com
Fri Aug 10 12:26:22 PDT 2012
> Hi,
>
> the check for "== null" in contains(Object object) looks incorrect.
Agreed.
>
> with kind regards
> thomas
>
> # HG changeset patch
> # Parent 10e8a5fc9b6ad5572a89297a1a22380f8f535938
> diff -r 10e8a5fc9b6a -r f85c1da3698c
> plugin/icedteanp/java/sun/applet/PluginObjectStore.java
> --- a/plugin/icedteanp/java/sun/applet/PluginObjectStore.java Thu
> Aug 09 17:44:52 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginObjectStore.java Thu
> Aug 09 17:58:02 2012 +0200
> @@ -45,9 +45,9 @@
> enum PluginObjectStore {
> INSTANCE;
>
> - private HashMap<Integer, Object> objects = new HashMap<Integer,
> Object>();
> - private HashMap<Integer, Integer> counts = new HashMap<Integer,
> Integer>();
> - private HashMap<Object, Integer> identifiers = new
> HashMap<Object, Integer>();
> + private final Map<Integer, Object> objects = new HashMap<Integer,
> Object>();
> + private final Map<Integer, Integer> counts = new HashMap<Integer,
> Integer>();
> + private final Map<Object, Integer> identifiers = new
> HashMap<Object, Integer>();
> private final Object lock = new Object();
>
> private boolean wrapped = false;
> @@ -64,20 +64,21 @@
> }
>
> public Integer getIdentifier(Object object) {
> + if (object == null)
> + return 0;
> +
> synchronized(lock) {
> - if (object == null)
> - return 0;
> return identifiers.get(object);
> }
> }
>
> public boolean contains(Object object) {
> - synchronized(lock) {
> - if (object == null)
> + if (object != null)
> + synchronized(lock) {
> return identifiers.containsKey(object);
> + }
Looking at the only usage of this method:
PluginAppletSecurity line 1064:
if (throwable != null && store.contains(throwable))
The current version is essentially a no-op. Good catch.
One opinionated nit, I prefer the look of:
if (object != null) {
synchronized(lock) {
return identifiers.containsKey(object);
}
}
> + return false;
>
> - return false;
> - }
> }
>
> public boolean contains(int identifier) {
>
Thanks for the patches! Needs a ChangeLog, but other than that and the
small nit, OK for HEAD & 1.3.
More information about the distro-pkg-dev
mailing list