Issue 5223 - App is freezing sometimes

adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/engine/Engine.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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package org.adblockplus.sbrowser.contentblocker.engine;
-
-import android.content.Context;
-import android.content.Intent;
-import android.content.SharedPreferences;
-import android.content.pm.PackageInfo;
-import android.content.pm.PackageManager;
-import android.content.pm.ResolveInfo;
-import android.net.ConnectivityManager;
-import android.net.NetworkInfo;
-import android.net.Uri;
-import android.os.AsyncTask;
-import android.os.Handler;
-import android.os.Looper;
-import android.preference.PreferenceManager;
-import android.text.TextUtils;
-import android.util.Log;
-
-import org.adblockplus.adblockplussbrowser.R;
 
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLEncoder;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.regex.Pattern;
 
+import org.adblockplus.adblockplussbrowser.R;
+
+import android.content.Context;
+import android.content.Intent;
+import android.content.SharedPreferences;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
+import android.content.pm.ResolveInfo;
+import android.net.ConnectivityManager;
+import android.net.NetworkInfo;
+import android.net.Uri;
+import android.os.Handler;
+import android.os.Looper;
+import android.preference.PreferenceManager;
+import android.text.TextUtils;
+import android.util.Log;
+
 public final class Engine
 {
   private static final String TAG = Engine.class.getSimpleName();
 
   // TODO make use of this regex's
   public static final Pattern RE_SUBSCRIPTION_HEADER = Pattern.compile(
       "\\[Adblock(?:\\s*Plus\\s*([\\d\\.]+)?)?\\]", Pattern.CASE_INSENSITIVE);
   public static final Pattern RE_FILTER_META = Pattern.compile("^\\s*!\\s*(\\w+)\\s*:\\s*(.*)");
   public static final Pattern RE_FILTER_ELEMHIDE = Pattern
       .compile("^([^\\/\\*\\|\\@\"!]*?)#(\\@)?(?:([\\w\\-]+|\\*)((?:\\([\\w\\-]+(?:[$^*]?=[^\\(\\)\"]*)?\\))*)|#([^{}]+))$");
@@ skipping 32 matching lines @@
   private static final int OLDEST_SAMSUNG_INTERNET_5_VERSIONCODE = 500000000;
 
   private final ReentrantLock accessLock = new ReentrantLock();
   private DefaultSubscriptions defaultSubscriptions;
   private Subscriptions subscriptions;
   private JSONPrefs jsonPrefs;
   private AppInfo appInfo;
   private LinkedBlockingQueue<EngineEvent> engineEvents = new LinkedBlockingQueue<EngineEvent>();
   private Thread handlerThread;
   private Downloader downloader;
+  private SubscriptionUpdateCallback subscriptionUpdateCallback;
   private final Context serviceContext;
   private boolean wasFirstRun = false;
   private long nextUpdateBroadcast = Long.MAX_VALUE;
 
   private Engine(final Context context)
   {
     this.serviceContext = context;
   }
 
   public String getPrefsDefault(final String key)
@@ skipping 57 matching lines @@
     }
     catch (PackageManager.NameNotFoundException e)
     {
       // Should never happen, as checkAAStatusAndProceed() should not be called if the user
       // has no compatible SBrowser installed. Nevertheless we have to handle the Exception.
       Log.d(TAG, "No compatible Samsung Browser found.", e);
       return false;
     }
   }
 
+  public void setSubscriptionUpdateCallback(final SubscriptionUpdateCallback subscriptionUpdateCallback)
+  {
+    this.subscriptionUpdateCallback = subscriptionUpdateCallback;
+  }
+
   public void requestUpdateBroadcast()
   {
-    this.lock();
-    try
-    {
-      this.nextUpdateBroadcast = System.currentTimeMillis() + BROADCAST_COMBINATION_DELAY_MILLIS;
-    }
-    finally
-    {
-      this.unlock();
-    }
+    this.nextUpdateBroadcast = System.currentTimeMillis() + BROADCAST_COMBINATION_DELAY_MILLIS;
   }
 
   private void writeFileAndSendUpdateBroadcast()
   {
     createAndWriteFile();
 
     runOnUiThread(new Runnable()
     {
       @Override
       public void run()
@@ skipping 36 matching lines @@
             return false;
         }
       }
       return false;
     }
     return current.isConnected();
   }
 
   public List<SubscriptionInfo> getListedSubscriptions()
   {
+    return this.subscriptions.getSubscriptions(this);
+  }
+
+  public void changeSubscriptionState(final String id, final boolean enabled)
+  {
+    if (this.subscriptionUpdateCallback != null)
+    {
+      this.subscriptionUpdateCallback.subscriptionUpdateRequested(enabled);
+    }
+    this.engineEvents.add(new ChangeEnabledStateEvent(id, enabled));
+  }
+
+  public void subscriptionStateChanged() {

[anton, 2017/06/26 14:45:54]

"{" should be on the next line.
LGTM in general

+    if (this.subscriptionUpdateCallback != null)
+    {
+      this.subscriptionUpdateCallback.subscriptionUpdatedApplied();
+    }
+  }
+
+  void downloadFinished(final String id, final int responseCode, final String response,
+      final Map<String, String> headers)
+  {
+    this.engineEvents.add(new DownloadFinishedEvent(id, responseCode, response, headers));
+  }
+
+  private void createAndWriteFile()
+  {
     this.lock();
     try
     {
-      return this.subscriptions.getSubscriptions(this);
+      Log.d(TAG, "Writing filters...");
+      final File filterFile = this.subscriptions.createAndWriteFile();
+      writeWhitelistedWebsites(this.serviceContext, filterFile);
+
+      final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this.serviceContext);
+      final String key = this.serviceContext.getString(R.string.key_cached_filter_path);
+      prefs.edit().putString(key, filterFile.getAbsolutePath()).commit();
+
+      Log.d(TAG, "Cleaning up cache...");
+      final File dummyFile = getDummyFilterFile(this.serviceContext);
+      final File[] cacheDirFiles = getFilterCacheDir(this.serviceContext).listFiles();
+      if (cacheDirFiles != null)
+      {
+        for (final File file : cacheDirFiles)
+        {
+          if (!file.equals(dummyFile) && !file.equals(filterFile))
+          {
+            Log.d(TAG, "Deleting file:" + file);
+            file.delete();
+          }
+        }
+      }
+    }
+    catch (IOException e)
+    {
+      Log.e(TAG, "Failed to write filters", e);
     }
     finally
     {
       this.unlock();
     }
-  }
-
-  public void changeSubscriptionState(final String id, final boolean enabled)
-  {
-    this.engineEvents.add(new ChangeEnabledStateEvent(id, enabled));
-  }
-
-  void downloadFinished(final String id, final int responseCode, final String response,
-      final Map<String, String> headers)
-  {
-    this.engineEvents.add(new DownloadFinishedEvent(id, responseCode, response, headers));
-  }
-
-  private void createAndWriteFile()
-  {
-    new CreateAndWriteFileAsyncTask().execute(serviceContext, subscriptions);

[diegocarloslima, 2017/05/25 21:20:45]

I might be wrong, but it seems that the freezing issue isn't related to the need
of running an AsyncTask for writing the filters. From what I could see, this
method is always called by a background thread. The problem seems to lie in the
lock() / unlock() calls. They're being called in the main thread on
Engine.getListedSubscriptions() and Engine.requestUpdateBroadcast(), which
should never happen. You might need to investigate if removing these calls
produces any side effect and if we really need to make the main thread wait for
some of these background operation, in which case we should show a progress
dialog instead of freezing the main thread

[jens, 2017/06/01 15:32:08]

Acknowledged.

   }
 
   public static void runOnUiThread(final Runnable runnable)
   {
     new Handler(Looper.getMainLooper()).post(runnable);
   }
 
   public boolean isAcceptableAdsEnabled()
   {
     this.lock();
@@ skipping 473 matching lines @@
       Log.d(TAG, headers.toString());
       this.downloader.enqueueDownload(this.createDownloadURL(sub), sub.getId(), headers);
     }
   }
 
   public void connectivityChanged()
   {
     this.downloader.connectivityChanged();
   }
 
-  private static class CreateAndWriteFileAsyncTask extends AsyncTask<Object, Void, Void>
-  {
-    private static final String TAG = CreateAndWriteFileAsyncTask.class.getSimpleName();
-
-    @Override
-    protected Void doInBackground(Object... params)
-    {
-      final Context serviceContext = (Context) params[0];
-      final Subscriptions subscriptions = (Subscriptions) params[1];
-
-      createAndWriteFile(serviceContext, subscriptions);
-      return null;
-    }
-
-    private void createAndWriteFile(Context serviceContext, Subscriptions subscriptions)
-    {
-      try
-      {
-        Log.d(TAG, "Writing filters...");
-        final File filterFile = subscriptions.createAndWriteFile();
-        writeWhitelistedWebsites(serviceContext, filterFile);
-
-        final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(serviceContext);
-        final String key = serviceContext.getString(R.string.key_cached_filter_path);
-        prefs.edit().putString(key, filterFile.getAbsolutePath()).commit();
-
-        Log.d(TAG, "Cleaning up cache...");
-        final File dummyFile = getDummyFilterFile(serviceContext);
-        final File[] cacheDirFiles = getFilterCacheDir(serviceContext).listFiles();
-        if (cacheDirFiles != null)
-        {
-          for (final File file : cacheDirFiles)
-          {
-            if (!file.equals(dummyFile) && !file.equals(filterFile))
-            {
-              Log.d(TAG, "Deleting file:" + file);
-              file.delete();
-            }
-          }
-        }
-      }
-      catch (IOException e)
-      {
-        Log.e(TAG, "Failed to write filters", e);
-      }
-    }
-
-    private void writeWhitelistedWebsites(Context context, File filterFile) throws IOException
-    {
-      Log.d(TAG, "Writing whitelisted websites...");
-      final SharedPreferences prefs =
-          PreferenceManager.getDefaultSharedPreferences(context.getApplicationContext());
-      final String key = context.getString(R.string.key_whitelisted_websites);
-
-      final Set<String> whitelistedWebsites = new TreeSet<>();
-      whitelistedWebsites.addAll(prefs.getStringSet(key, Collections.<String>emptySet()));
-
-      try (BufferedWriter w = new BufferedWriter(
-          new OutputStreamWriter(new FileOutputStream(filterFile, true), Engine.CHARSET_UTF_8))) {
-        for (final String url : whitelistedWebsites) {
-          try {
-            final URI uri = new URI(url);
-            final String host = uri.getHost() != null ? uri.getHost() : uri.getPath();
-            w.write("@@||" + host + "^$document");
-            w.write('\n');
-          } catch (URISyntaxException e) {
-            Log.w(TAG, "Failed to parse whitelisted website: " + url);
-          }
-        }
-      }
-    }
-
-    private static File getDummyFilterFile(Context context)
-    {
-      return new File(getFilterCacheDir(context), "dummy.txt");
-    }
-
-    private static File getFilterCacheDir(Context context)
-    {
-      return new File(context.getCacheDir(), "subscriptions");
-    }
+  public interface SubscriptionUpdateCallback
+  {
+    void subscriptionUpdateRequested(boolean enabled);
+    void subscriptionUpdatedApplied();
   }
 }