Issue 5053 - Release JsValues explicitly

libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 13 matching lines @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
 import org.adblockplus.libadblockplus.AppInfo;
 import org.adblockplus.libadblockplus.Filter;
 import org.adblockplus.libadblockplus.FilterChangeCallback;
 import org.adblockplus.libadblockplus.FilterEngine;
 import org.adblockplus.libadblockplus.FilterEngine.ContentType;
 import org.adblockplus.libadblockplus.JsEngine;
+import org.adblockplus.libadblockplus.JsValue;
 import org.adblockplus.libadblockplus.LogSystem;
 import org.adblockplus.libadblockplus.ShowNotificationCallback;
 import org.adblockplus.libadblockplus.Subscription;
 import org.adblockplus.libadblockplus.UpdateAvailableCallback;
 import org.adblockplus.libadblockplus.UpdateCheckDoneCallback;
 
 import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.os.Build.VERSION;
@@ skipping 181 matching lines @@
   public boolean isElemhideEnabled()
   {
     return this.elemhideEnabled;
   }
 
   private static org.adblockplus.libadblockplus.android.Subscription convertJsSubscription(final Subscription jsSubscription)
   {
     final org.adblockplus.libadblockplus.android.Subscription subscription =
       new org.adblockplus.libadblockplus.android.Subscription();
 
-    subscription.title = jsSubscription.getProperty("title").toString();
-    subscription.url = jsSubscription.getProperty("url").toString();
+    JsValue jsTitle = jsSubscription.getProperty("title");

[diegocarloslima, 2017/03/30 14:19:29]

If JsValue implemented AutoCloseable, we could avoid some finally blocks, by
using try-with-resources

[anton, 2017/03/30 14:34:15]

i've been thinking about it and it makes no sense -
 the user can just forget to use `try-with-resources` just like forget to invoke
`dispose()`

is it correct?

[diegocarloslima, 2017/03/30 14:47:18]

Yeah, he can forget on both approaches. The advantage of using
try-with-resources is that we won't need a boilerplate code 'finally {
object.dispose()}' block every time we want to dispose a locally created
variable, making the code cleaner. I would even suggest to make Disposable
extend AutoCloseable, so every Disposable child can be used in a
try-with-resources block. Since this change might be kinda unrelated to this
issue, we can discuss further and do it in a separate issue.

+    try
+    {
+      subscription.title = jsTitle.toString();
+    }
+    finally
+    {
+      jsTitle.dispose();
+    }
+
+    JsValue jsUrl = jsSubscription.getProperty("url");
+    try
+    {
+      subscription.url = jsUrl.toString();
+    }
+    finally
+    {
+      jsUrl.dispose();
+    }
 
     return subscription;
   }
 
   private static org.adblockplus.libadblockplus.android.Subscription[] convertJsSubscriptions(
     final List<Subscription> jsSubscriptions)
   {
     final org.adblockplus.libadblockplus.android.Subscription[] subscriptions =
       new org.adblockplus.libadblockplus.android.Subscription[jsSubscriptions.size()];
 
     for (int i = 0; i < subscriptions.length; i++)
     {
       subscriptions[i] = convertJsSubscription(jsSubscriptions.get(i));
     }
 
     return subscriptions;
   }
 
   public org.adblockplus.libadblockplus.android.Subscription[] getRecommendedSubscriptions()
   {
-    return convertJsSubscriptions(this.filterEngine.fetchAvailableSubscriptions());
+    List<Subscription> subscriptions = this.filterEngine.fetchAvailableSubscriptions();
+    try
+    {
+      return convertJsSubscriptions(subscriptions);
+    }
+    finally
+    {
+      for (Subscription eachSubscription : subscriptions)
+      {
+        eachSubscription.dispose();
+      }
+    }
   }
 
   public org.adblockplus.libadblockplus.android.Subscription[] getListedSubscriptions()
   {
-    return convertJsSubscriptions(this.filterEngine.getListedSubscriptions());
+    List<Subscription> subscriptions = this.filterEngine.getListedSubscriptions();
+    try
+    {
+      return convertJsSubscriptions(subscriptions);
+    }
+    finally
+    {
+      for (Subscription eachSubscription : subscriptions)
+      {
+        eachSubscription.dispose();
+      }
+    }
   }
 
   public void clearSubscriptions()
   {
     for (final Subscription s : this.filterEngine.getListedSubscriptions())
     {
-      s.removeFromList();
+      try
+      {
+        s.removeFromList();
+      }
+      finally
+      {
+        s.dispose();
+      }
     }
   }
 
   public void setSubscription(final String url)
   {
     clearSubscriptions();
 
     final Subscription sub = this.filterEngine.getSubscription(url);
     if (sub != null)
     {
-      sub.addToList();
+      try
+      {
+        sub.addToList();
+      }
+      finally
+      {
+        sub.dispose();
+      }
     }
   }
 
   public void setSubscriptions(Collection<String> urls)
   {
     clearSubscriptions();
 
     for (String eachUrl : urls)
     {
       final Subscription sub = this.filterEngine.getSubscription(eachUrl);
       if (sub != null)
       {
-        sub.addToList();
+        try
+        {
+          sub.addToList();
+        }
+        finally
+        {
+          sub.dispose();
+        }
       }
     }
   }
 
   public void refreshSubscriptions()
   {
     for (final Subscription s : this.filterEngine.getListedSubscriptions())
     {
-      s.updateFilters();
+      try
+      {
+        s.updateFilters();
+      }
+      finally
+      {
+        s.dispose();
+      }
     }
   }
 
   public boolean isAcceptableAdsEnabled()
   {
     final String url = getAcceptableAdsSubscriptionURL();
     List<Subscription> subscriptions = this.filterEngine.getListedSubscriptions();
-    for (Subscription eachSubscription : subscriptions)
+    try
     {
-      if (eachSubscription.getProperty("url").toString().equals(url))
+      for (Subscription eachSubscription : subscriptions)
       {
-        return true;
+        JsValue jsUrl = eachSubscription.getProperty("url");
+        try
+        {
+          if (jsUrl.toString().equals(url))
+          {
+            return true;
+          }
+        }
+        finally
+        {
+          jsUrl.dispose();
+        }
+      }
+      return false;
+    }
+    finally
+    {
+      if (subscriptions != null)
+      {
+        for (Subscription eachSubscription : subscriptions)
+        {
+          eachSubscription.dispose();
+        }
       }
     }
-    return false;
   }
 
   public void setEnabled(final boolean enabled)
   {
     this.enabled = enabled;
   }
 
   public boolean isEnabled()
   {
     return enabled;
   }
 
   public String getAcceptableAdsSubscriptionURL()
   {
-    return this.filterEngine.getPref("subscriptions_exceptionsurl").toString();
+    JsValue jsPref = this.filterEngine.getPref("subscriptions_exceptionsurl");
+    try
+    {
+      return jsPref.toString();
+    }
+    finally
+    {
+      jsPref.dispose();
+    }
   }
 
   public void setAcceptableAdsEnabled(final boolean enabled)
   {
     final String url = getAcceptableAdsSubscriptionURL();
     final Subscription sub = this.filterEngine.getSubscription(url);
     if (sub != null)
     {
-      if (enabled)
+      try
       {
-        sub.addToList();
+        if (enabled)
+        {
+          sub.addToList();
+        }
+        else
+        {
+          sub.removeFromList();
+        }
       }
-      else
+      finally
       {
-        sub.removeFromList();
+        sub.dispose();
       }
     }
   }
 
   public String getDocumentationLink()
   {
-    return this.filterEngine.getPref("documentation_link").toString();
+    JsValue jsPref = this.filterEngine.getPref("documentation_link");
+    try
+    {
+      return jsPref.toString();
+    }
+    finally
+    {
+      jsPref.dispose();
+    }
   }
 
   public boolean matches(final String fullUrl, final ContentType contentType, final String[] referrerChainArray)
   {
     if (!enabled)
     {
       return false;
     }
 
     final Filter filter = this.filterEngine.matches(fullUrl, contentType, referrerChainArray);
 
     if (filter == null)
     {
       return false;
     }
 
-    // hack: if there is no referrer, block only if filter is domain-specific
-    // (to re-enable in-app ads blocking, proposed on 12.11.2012 Monday meeting)
-    // (documentUrls contains the referrers on Android)
     try
     {
-      if (referrerChainArray.length == 0 && (filter.getProperty("text").toString()).contains("||"))
+      // hack: if there is no referrer, block only if filter is domain-specific
+      // (to re-enable in-app ads blocking, proposed on 12.11.2012 Monday meeting)
+      // (documentUrls contains the referrers on Android)
+      try
       {
-        return false;
+        JsValue jsText = filter.getProperty("text");
+        try
+        {
+          if (referrerChainArray.length == 0 && (jsText.toString()).contains("||"))
+          {
+            return false;
+          }
+        }
+        finally
+        {
+          jsText.dispose();
+        }
       }
-    } catch (NullPointerException e) {
+      catch (NullPointerException e)
+      {
+      }
+
+      return filter.getType() != Filter.Type.EXCEPTION;
     }
-
-    return filter.getType() != Filter.Type.EXCEPTION;
+    finally
+    {
+      filter.dispose();
+    }
   }
 
   public boolean isDocumentWhitelisted(final String url, final String[] referrerChainArray)
   {
     return this.filterEngine.isDocumentWhitelisted(url, referrerChainArray);
   }
 
   public boolean isDomainWhitelisted(final String url, final String[] referrerChainArray)
   {
     if (whitelistedDomains == null)
@@ skipping 65 matching lines @@
   public void setWhitelistedDomains(List<String> domains)
   {
     this.whitelistedDomains = domains;
   }
 
   public List<String> getWhitelistedDomains()
   {
     return whitelistedDomains;
   }
 }