Issue 5223 - App is freezing sometimes

adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java

Index: adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java
===================================================================
--- a/adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java
+++ b/adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java
@@ -12,16 +12,34 @@
  * 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;
@@ -37,34 +55,16 @@
 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 org.adblockplus.sbrowser.contentblocker.MainPreferences;
-
-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*(.*)");
@@ -275,50 +275,17 @@
   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
-    {
-      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();
-    }
+    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()
@@ -801,9 +768,92 @@
       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");
+    }
+  }
 }