[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