Usage of new API, cleanups (reduced)

src/org/adblockplus/android/AdblockPlus.java

Index: src/org/adblockplus/android/AdblockPlus.java
diff --git a/src/org/adblockplus/android/AdblockPlus.java b/src/org/adblockplus/android/AdblockPlus.java
index 61d9cb7ad056b118f47aeccbfe6104749925f8f7..5ce5a8d9f44372a822174bf630f94d5cbeafc3e1 100755
--- a/src/org/adblockplus/android/AdblockPlus.java
+++ b/src/org/adblockplus/android/AdblockPlus.java
@@ -33,6 +33,8 @@ import java.util.regex.Pattern;
 
 import org.adblockplus.android.updater.AlarmReceiver;
 
+import com.github.rjeschke.neetutils.Strings;
+
 import android.app.ActivityManager;
 import android.app.ActivityManager.RunningServiceInfo;
 import android.app.AlarmManager;
@@ -55,7 +57,7 @@ import android.util.Log;
 
 public class AdblockPlus extends Application
 {
-  private final static String TAG = "Application";
+  private final static String TAG = Utils.getTag(AdblockPlus.class);
 
   private final static Pattern RE_JS = Pattern.compile(".*\\.js$", Pattern.CASE_INSENSITIVE);
   private final static Pattern RE_CSS = Pattern.compile(".*\\.css$", Pattern.CASE_INSENSITIVE);
@@ -64,9 +66,13 @@ public class AdblockPlus extends Application
   private final static Pattern RE_HTML = Pattern.compile(".*\\.html?$", Pattern.CASE_INSENSITIVE);
 
   /**
+   * Update notification id.
+   */
+  public final static int UPDATE_NOTIFICATION_ID = R.string.app_name + 1;

[Felix Dahlke, 2014/04/16 15:24:25]

Should be "static final", not "final" static, according to the Android
conventions (and Sun too IIRC). Also below.

[René Jeschke, 2014/04/16 17:51:47]

Done.

+  /**
    * Broadcasted when filtering is enabled or disabled.
    */
-  public static final String BROADCAST_FILTERING_CHANGE = "org.adblockplus.android.filtering.status";
+  public final static String BROADCAST_FILTERING_CHANGE = "org.adblockplus.android.filtering.status";
   /**
    * Broadcasted when subscription status changes.
    */
@@ -75,12 +81,10 @@ public class AdblockPlus extends Application
    * Broadcasted when filter match check is performed.
    */
   public final static String BROADCAST_FILTER_MATCHES = "org.adblockplus.android.filter.matches";
-
   /**
    * Cached list of recommended subscriptions.
    */
   private Subscription[] subscriptions;
-
   /**
    * Indicates whether filtering is enabled or not.
    */
@@ -92,8 +96,8 @@ public class AdblockPlus extends Application
 
   private static class ReferrerMappingCache extends LinkedHashMap<String, String>
   {
-    private static final long serialVersionUID = 1L;
-    private static final int MAX_SIZE = 5000;
+    private final static long serialVersionUID = 1L;
+    private final static int MAX_SIZE = 5000;
 
     public ReferrerMappingCache()
     {
@@ -101,13 +105,13 @@ public class AdblockPlus extends Application
     }
 
     @Override
-    protected boolean removeEldestEntry(Map.Entry<String, String> eldest)
+    protected boolean removeEldestEntry(final Map.Entry<String, String> eldest)
     {
-      return size() > MAX_SIZE;
+      return this.size() > MAX_SIZE;

[Felix Dahlke, 2014/04/16 15:24:25]

Same as in other files, also below.

[René Jeschke, 2014/04/16 17:51:47]

Done.

     }
   };
 
-  private ReferrerMappingCache referrerMapping = new ReferrerMappingCache();
+  private final ReferrerMappingCache referrerMapping = new ReferrerMappingCache();
 
   /**
    * Returns pointer to itself (singleton pattern).
@@ -122,10 +126,10 @@ public class AdblockPlus extends Application
     int buildNumber = -1;
     try
     {
-      PackageInfo pi = getPackageManager().getPackageInfo(getPackageName(), 0);
+      final PackageInfo pi = this.getPackageManager().getPackageInfo(this.getPackageName(), 0);
       buildNumber = pi.versionCode;
     }
-    catch (NameNotFoundException e)
+    catch (final NameNotFoundException e)
     {
       // ignore - this shouldn't happen
       Log.e(TAG, e.getMessage(), e);
@@ -136,26 +140,17 @@ public class AdblockPlus extends Application
   /**
    * Opens Android application settings
    */
-  public static void showAppDetails(Context context)
+  public static void showAppDetails(final Context context)
   {
-    String packageName = context.getPackageName();
-    Intent intent = new Intent();
-    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.GINGERBREAD)
-    {
-      // above 2.3
-      intent.setAction(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
-      Uri uri = Uri.fromParts("package", packageName, null);
-      intent.setData(uri);
-    }
-    else
-    {
-      // below 2.3
-      final String appPkgName = (Build.VERSION.SDK_INT == Build.VERSION_CODES.FROYO ? "pkg" : "com.android.settings.ApplicationPkgName");
-      intent.setAction(Intent.ACTION_VIEW);
-      intent.setClassName("com.android.settings", "com.android.settings.InstalledAppDetails");
-      intent.putExtra(appPkgName, packageName);
-    }
+    final String packageName = context.getPackageName();
+
+    final Intent intent = new Intent();
+    intent.setAction(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
+
+    final Uri uri = Uri.fromParts("package", packageName, null);
+    intent.setData(uri);
     intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+
     context.startActivity(intent);
   }
 
@@ -164,48 +159,45 @@ public class AdblockPlus extends Application
    */
   public static String getDeviceName()
   {
-    String manufacturer = Build.MANUFACTURER;
-    String model = Build.MODEL;
+    final String manufacturer = Build.MANUFACTURER;
+    final String model = Build.MODEL;
     if (model.startsWith(manufacturer))
-      return capitalize(model);
+    {

[Felix Dahlke, 2014/04/16 15:24:25]

Same as in other files, also below.

[René Jeschke, 2014/04/16 17:51:47]

Done.

+      return Utils.capitalizeString(model);
+    }
     else
-      return capitalize(manufacturer) + " " + model;
+    {
+      return Utils.capitalizeString(manufacturer) + " " + model;
+    }
   }
 
-  public static void appendRawTextFile(Context context, StringBuilder text, int id)
+  public static void appendRawTextFile(final Context context, final StringBuilder text, final int id)
   {
-    InputStream inputStream = context.getResources().openRawResource(id);
-    InputStreamReader in = new InputStreamReader(inputStream);
-    BufferedReader buf = new BufferedReader(in);
+    // TODO: What about closing the resources?
+    final InputStream inputStream = context.getResources().openRawResource(id);
+    final InputStreamReader in = new InputStreamReader(inputStream);
+    final BufferedReader buf = new BufferedReader(in);
     String line;
     try
     {
       while ((line = buf.readLine()) != null)
+      {
         text.append(line);
+      }
     }
-    catch (IOException e)
+    catch (final IOException e)
     {
+      // TODO: How about real logging?
       e.printStackTrace();
     }
   }
 
-  private static String capitalize(String s)
-  {
-    if (s == null || s.length() == 0)
-      return "";
-    char first = s.charAt(0);
-    if (Character.isUpperCase(first))
-      return s;
-    else
-      return Character.toUpperCase(first) + s.substring(1);
-  }
-
   /**
    * Checks if device has a WiFi connection available.
    */
-  public static boolean isWiFiConnected(Context context)
+  public static boolean isWiFiConnected(final Context context)
   {
-    ConnectivityManager connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);

[Felix Dahlke, 2014/04/16 15:24:25]

Why remove the space after the cast? More common/clear that way IMO. Same below.

+    final ConnectivityManager connectivityManager = (ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);
     NetworkInfo networkInfo = null;
     if (connectivityManager != null)
     {
@@ -221,12 +213,15 @@ public class AdblockPlus extends Application
    */
   public boolean isServiceRunning()
   {
-    ActivityManager manager = (ActivityManager) getSystemService(ACTIVITY_SERVICE);
+    final ActivityManager manager = (ActivityManager)this.getSystemService(ACTIVITY_SERVICE);
+
     // Actually it returns not only running services, so extra check is required
-    for (RunningServiceInfo service : manager.getRunningServices(Integer.MAX_VALUE))
+    for (final RunningServiceInfo service : manager.getRunningServices(Integer.MAX_VALUE))
     {
       if (service.service.getClassName().equals(ProxyService.class.getCanonicalName()) && service.pid > 0)
+      {
         return true;
+      }
     }
     return false;
   }
@@ -236,14 +231,14 @@ public class AdblockPlus extends Application
    */
   public boolean checkWriteExternalPermission()
   {
-    String permission = "android.permission.WRITE_EXTERNAL_STORAGE";
-    int res = checkCallingOrSelfPermission(permission);
+    final String permission = "android.permission.WRITE_EXTERNAL_STORAGE";
+    final int res = this.checkCallingOrSelfPermission(permission);
     return res == PackageManager.PERMISSION_GRANTED;
   }
 
   public boolean isFirstRun()
   {
-    return abpEngine.isFirstRun();
+    return this.abpEngine.isFirstRun();
   }
 
   /**
@@ -251,9 +246,12 @@ public class AdblockPlus extends Application
    */
   public Subscription[] getRecommendedSubscriptions()
   {
-    if (subscriptions == null)
-      subscriptions = abpEngine.getRecommendedSubscriptions();
-    return subscriptions;
+    // TODO: Why don't we re-check?
+    if (this.subscriptions == null)
+    {
+      this.subscriptions = this.abpEngine.getRecommendedSubscriptions();
+    }
+    return this.subscriptions;
   }
 
   /**
@@ -261,7 +259,7 @@ public class AdblockPlus extends Application
    */
   public Subscription[] getListedSubscriptions()
   {
-    return abpEngine.getListedSubscriptions();
+    return this.abpEngine.getListedSubscriptions();
   }
 
   /**
@@ -270,14 +268,9 @@ public class AdblockPlus extends Application
    * @param url
    *          URL of subscription to add
    */
-  public void setSubscription(String url)
+  public void setSubscription(final String url)
   {
-    Subscription[] subscriptions = abpEngine.getListedSubscriptions();
-    for (Subscription subscription : subscriptions)
-    {
-      abpEngine.removeSubscription(subscription.url);
-    }
-    abpEngine.addSubscription(url);
+    this.abpEngine.setSubscription(url);
   }
 
   /**
@@ -285,40 +278,36 @@ public class AdblockPlus extends Application
    */
   public void refreshSubscriptions()
   {
-    Subscription[] subscriptions = abpEngine.getListedSubscriptions();
-    for (Subscription subscription : subscriptions)
-    {
-      abpEngine.refreshSubscription(subscription.url);
-    }
+    this.abpEngine.refreshSubscriptions();
   }
 
   /**
    * Enforces subscription status update.
    *
-   * @param url Subscription url
+   * @param url
+   *          Subscription url

[Felix Dahlke, 2014/04/16 15:24:25]

Why this weird break? Same below.

    */
-  public void actualizeSubscriptionStatus(String url)
+  public void updateSubscriptionStatus(final String url)
   {
-    abpEngine.actualizeSubscriptionStatus(url);
+    this.abpEngine.updateSubscriptionStatus(url);
   }
 
-
   /**
    * Enables or disables Acceptable Ads
    */
-  public void setAcceptableAdsEnabled(boolean enabled)
+  public void setAcceptableAdsEnabled(final boolean enabled)
   {
-    abpEngine.setAcceptableAdsEnabled(enabled);
+    this.abpEngine.setAcceptableAdsEnabled(enabled);
   }
 
   public String getAcceptableAdsUrl()
   {
-    final String documentationLink = abpEngine.getDocumentationLink();
-    final String locale = getResources().getConfiguration().locale.toString().replace("_", "-");
+    final String documentationLink = this.abpEngine.getDocumentationLink();
+    final String locale = this.getResources().getConfiguration().locale.toString().replace("_", "-");
     return documentationLink.replace("%LINK%", "acceptable_ads").replace("%LANG%", locale);
   }
 
-  public void setNotifiedAboutAcceptableAds(boolean notified)
+  public void setNotifiedAboutAcceptableAds(final boolean notified)
   {
     final SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(this);
     final Editor editor = preferences.edit();
@@ -335,25 +324,29 @@ public class AdblockPlus extends Application
   /**
    * Returns ElemHide selectors for domain.
    *
-   * @param domain The domain
+   * @param domain
+   *          The domain
    * @return A list of CSS selectors
    */
   public String[] getSelectorsForDomain(final String domain)
   {
-    /* We need to ignore element hiding rules here to work around two bugs:
+    /*
+     * We need to ignore element hiding rules here to work around two bugs:
+     *
      * 1. CSS is being injected even when there's an exception rule with $elemhide
+     *
      * 2. The injected CSS causes blank pages in Chrome for Android
      *
-     * Starting with 1.1.2, we ignored element hiding rules after download anyway, to keep the
-     * memory usage down. Doing this with libadblockplus is trickier, but would be the clean
-     * solution. */
+     *
+     * Starting with 1.1.2, we ignored element hiding rules after download anyway, to keep the memory usage down. Doing this with libadblockplus is

[Felix Dahlke, 2014/04/16 15:24:25]

As before, please don't make code that adheres to the 100 column rule violate
it. Same below.

[René Jeschke, 2014/04/16 17:51:47]

Done.

+     * trickier, but would be the clean solution.
+     */
     return null;
-/*
-    if (!filteringEnabled)
-      return null;
-
-    return abpEngine.getSelectorsForDomain(domain);
-*/
+    /*

[Felix Dahlke, 2014/04/16 15:24:25]

We don't want to leave commented out code around, let's either leave it alone or
get rid of it.

[René Jeschke, 2014/04/16 17:51:47]

Done.

+     * if (!filteringEnabled) return null;
+     *
+     * return abpEngine.getSelectorsForDomain(domain);
+     */
   }
 
   /**
@@ -370,59 +363,80 @@ public class AdblockPlus extends Application
    * @return true if matched filter was found
    * @throws Exception
    */
-  public boolean matches(String url, String query, String referrer, String accept)
+  public boolean matches(final String url, final String query, final String referrer, final String accept)
   {
     final String fullUrl = !"".equals(query) ? url + "?" + query : url;
     if (referrer != null)
-      referrerMapping.put(fullUrl, referrer);
+    {
+      this.referrerMapping.put(fullUrl, referrer);
+    }
 
-    if (!filteringEnabled)
+    if (!this.filteringEnabled)
+    {
       return false;
+    }
 
     String contentType = null;
 
     if (accept != null)
     {
       if (accept.contains("text/css"))
+      {
         contentType = "STYLESHEET";
+      }
       else if (accept.contains("image/*"))
+      {
         contentType = "IMAGE";
+      }
       else if (accept.contains("text/html"))
+      {
         contentType = "SUBDOCUMENT";
+      }
     }
 
     if (contentType == null)
     {
       if (RE_JS.matcher(url).matches())
+      {
         contentType = "SCRIPT";
+      }
       else if (RE_CSS.matcher(url).matches())
+      {
         contentType = "STYLESHEET";
+      }
       else if (RE_IMAGE.matcher(url).matches())
+      {
         contentType = "IMAGE";
+      }
       else if (RE_FONT.matcher(url).matches())
+      {
         contentType = "FONT";
+      }
       else if (RE_HTML.matcher(url).matches())
+      {
         contentType = "SUBDOCUMENT";
+      }
     }
     if (contentType == null)
+    {
       contentType = "OTHER";
+    }
 
-    final List<String> referrerChain = buildReferrerChain(referrer);
+    final List<String> referrerChain = this.buildReferrerChain(referrer);
     Log.d("Referrer chain", fullUrl + ": " + referrerChain.toString());
-    String[] referrerChainArray = referrerChain.toArray(new String[referrerChain.size()]);
-    return abpEngine.matches(fullUrl, contentType, referrerChainArray);
+    final String[] referrerChainArray = referrerChain.toArray(new String[referrerChain.size()]);
+    return this.abpEngine.matches(fullUrl, contentType, referrerChainArray);
   }
 
   private List<String> buildReferrerChain(String url)
   {
     final List<String> referrerChain = new ArrayList<String>();
-    // We need to limit the chain length to ensure we don't block indefinitely if there's
-    // a referrer loop.
+    // We need to limit the chain length to ensure we don't block indefinitely if there's a referrer loop.
     final int maxChainLength = 10;
     for (int i = 0; i < maxChainLength && url != null; i++)
     {
       referrerChain.add(0, url);
-      url = referrerMapping.get(url);
+      url = this.referrerMapping.get(url);
     }
     return referrerChain;
   }
@@ -432,28 +446,27 @@ public class AdblockPlus extends Application
    */
   public boolean isFilteringEnabled()
   {
-    return filteringEnabled;
+    return this.filteringEnabled;
   }
 
   /**
    * Enables or disables filtering.
    */
-  public void setFilteringEnabled(boolean enable)
+  public void setFilteringEnabled(final boolean enable)
   {
-    filteringEnabled = enable;
-    sendBroadcast(new Intent(BROADCAST_FILTERING_CHANGE).putExtra("enabled", filteringEnabled));
+    this.filteringEnabled = enable;
+    this.sendBroadcast(new Intent(BROADCAST_FILTERING_CHANGE).putExtra("enabled", this.filteringEnabled));
   }
 
   /**
-   * Starts ABP engine. It also initiates subscription refresh if it is enabled
-   * in user settings.
+   * Starts ABP engine. It also initiates subscription refresh if it is enabled in user settings.
    */
   public void startEngine()
   {
-    if (abpEngine == null)
+    if (this.abpEngine == null)
     {
-      File basePath = getFilesDir();
-      abpEngine = new ABPEngine(this, basePath.getAbsolutePath());
+      final File basePath = this.getFilesDir();
+      this.abpEngine = ABPEngine.create(AdblockPlus.getApplication(), ABPEngine.generateAppInfo(this), basePath.getAbsolutePath());
     }
   }
 
@@ -462,10 +475,10 @@ public class AdblockPlus extends Application
    */
   public void stopEngine()
   {
-    if (abpEngine != null)
+    if (this.abpEngine != null)
     {
-      abpEngine.release();
-      abpEngine = null;
+      this.abpEngine.dispose();
+      this.abpEngine = null;
       Log.i(TAG, "stopEngine");
     }
   }
@@ -475,20 +488,18 @@ public class AdblockPlus extends Application
    */
   public void checkUpdates()
   {
-    abpEngine.checkUpdates();
+    this.abpEngine.checkForUpdates();
   }
 
   /**
-   * Sets Alarm to call updater after specified number of minutes or after one
-   * day if
-   * minutes are set to 0.
+   * Sets Alarm to call updater after specified number of minutes or after one day if minutes are set to 0.
    *
    * @param minutes
    *          number of minutes to wait
    */
-  public void scheduleUpdater(int minutes)
+  public void scheduleUpdater(final int minutes)
   {
-    Calendar updateTime = Calendar.getInstance();
+    final Calendar updateTime = Calendar.getInstance();
 
     if (minutes == 0)
     {
@@ -499,17 +510,17 @@ public class AdblockPlus extends Application
       // ...next day
       updateTime.add(Calendar.HOUR_OF_DAY, 24);
       // Spread out the “mass downloading” for 6 hours
-      updateTime.add(Calendar.MINUTE, (int) (Math.random() * 60 * 6));
+      updateTime.add(Calendar.MINUTE, (int)(Math.random() * 60 * 6));
     }
     else
     {
       updateTime.add(Calendar.MINUTE, minutes);
     }
 
-    Intent updater = new Intent(this, AlarmReceiver.class);
-    PendingIntent recurringUpdate = PendingIntent.getBroadcast(this, 0, updater, PendingIntent.FLAG_CANCEL_CURRENT);
+    final Intent updater = new Intent(this, AlarmReceiver.class);
+    final PendingIntent recurringUpdate = PendingIntent.getBroadcast(this, 0, updater, PendingIntent.FLAG_CANCEL_CURRENT);
     // Set non-waking alarm
-    AlarmManager alarms = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
+    final AlarmManager alarms = (AlarmManager)this.getSystemService(Context.ALARM_SERVICE);
     alarms.set(AlarmManager.RTC, updateTime.getTimeInMillis(), recurringUpdate);
   }
 
@@ -522,33 +533,36 @@ public class AdblockPlus extends Application
     // Check for crash report
     try
     {
-      InputStreamReader reportFile = new InputStreamReader(openFileInput(CrashHandler.REPORT_FILE));
-      final char[] buffer = new char[0x1000];
-      StringBuilder out = new StringBuilder();
+      final InputStreamReader reportFile = new InputStreamReader(this.openFileInput(CrashHandler.REPORT_FILE));
+      final char[] buffer = new char[4096];

[Felix Dahlke, 2014/04/16 15:24:25]

Why this? Hex is not that hard to read.

[René Jeschke, 2014/04/16 17:51:47]

Because is totally does not make any sense here. Well, still.

+      final StringBuilder out = new StringBuilder();
       int read;
+

[Felix Dahlke, 2014/04/16 15:24:25]

Empty lines are also arguably an unrelated change, same below.

[René Jeschke, 2014/04/16 17:51:47]

Done.

       do
       {
         read = reportFile.read(buffer, 0, buffer.length);
         if (read > 0)
+        {
           out.append(buffer, 0, read);
+        }
       }
       while (read >= 0);
-      String report = out.toString();
-      if (!"".equals(report))
+
+      final String report = out.toString();
+      if (!Strings.isEmpty(report))
       {
         final Intent intent = new Intent(this, CrashReportDialog.class);
         intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
         intent.putExtra("report", report);
-        startActivity(intent);
+        this.startActivity(intent);
       }
     }
-    catch (FileNotFoundException e)
+    catch (final FileNotFoundException e)
     {
       // ignore
     }
-    catch (IOException e)
+    catch (final IOException e)
     {
-      // TODO Auto-generated catch block
       Log.e(TAG, e.getMessage(), e);
     }
 
@@ -556,6 +570,6 @@ public class AdblockPlus extends Application
     Thread.setDefaultUncaughtExceptionHandler(new CrashHandler(this));
 
     // Initiate update check
-    scheduleUpdater(0);
+    this.scheduleUpdater(0);
   }
 }