Issue 2853 - Settings changes are sometimes not saved if the user quits the app

mobile/android/thirdparty/org/adblockplus/browser/AddOnBridge.java

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 33 matching lines @@
   private static final String TAG = "AdblockBrowser.AddOnBridge";
   private static final String REQUEST_NAME = "AdblockPlus:Api";
   // Handler+HandlerThread for posting delayed re-tries without interfering with
   // other threads (e.g. the UI or Gecko thread)
   private static final HandlerThread PRIVATE_HANDLER_THREAD;
   private static final Handler PRIVATE_HANDLER;
   // Global handler, for e.g. UI tasks
   private static final HandlerThread GLOBAL_HANDLER_THREAD;
   private static final Handler GLOBAL_HANDLER;
   // Sometimes, the app is killed before the extension is able to save all changes regarding
-  // AddOnBridge requests. Given that, we need to store the uncompleted requests on SharedPrefs,
+  // AddOnBridge requests. Given that, we need to store the pending requests on SharedPrefs,
   // so we can resend them to the extension once the app restarts
   // See https://issues.adblockplus.org/ticket/2853
   private static final AddOnEventListener ADD_ON_EVENT_LISTENER = new AddOnEventListener();
   private static final String ON_FILTERS_LOAD_EVENT = "Abb:OnFiltersLoad";
   private static final String ON_FILTERS_SAVE_EVENT = "Abb:OnFiltersSave";
   private static final List<AddOnRequest> PENDING_REQUESTS = new ArrayList<>();

[Felix Dahlke, 2017/01/30 15:37:53]

You've renamed this from "uncomplete" to "pending" (I prefer the latter FWIW),
but there are still some references to "uncompleted" requests in the code. Those
should be changed as well IMO.

[diegocarloslima, 2017/01/30 17:12:43]

Acknowledged.

-  private static final String UNCOMPLETED_REQUESTS_PREFS_KEY = "UNCOMPLETED_REQUESTS_PREFS_KEY";
+  private static final String PENDING_REQUESTS_PREFS_KEY = "PENDING_REQUESTS_PREFS_KEY";
 
   private static SharedPreferences sharedPrefs;
   private static boolean filtersLoaded;
 
   static
   {
     PRIVATE_HANDLER_THREAD = new HandlerThread("abp-private-handler");
     PRIVATE_HANDLER_THREAD.setDaemon(true);
     PRIVATE_HANDLER_THREAD.start();
     PRIVATE_HANDLER = new Handler(PRIVATE_HANDLER_THREAD.getLooper());
 
     GLOBAL_HANDLER_THREAD = new HandlerThread("abp-global-handler");
     GLOBAL_HANDLER_THREAD.setDaemon(true);
     GLOBAL_HANDLER_THREAD.start();
     GLOBAL_HANDLER = new Handler(GLOBAL_HANDLER_THREAD.getLooper());
   }
 
   public static void init(Context context)
   {
     sharedPrefs = context.getSharedPreferences(AddOnBridge.class.getName(), Context.MODE_PRIVATE);
     EventDispatcher.getInstance().registerGeckoThreadListener(ADD_ON_EVENT_LISTENER, ON_FILTERS_LOAD_EVENT, ON_FILTERS_SAVE_EVENT);
-    loadUncompletedRequests();
+    loadPendingRequests();
   }
 
   public static void postToHandler(Runnable runnable)
   {
     GLOBAL_HANDLER.post(runnable);
   }
 
   public static void postToHandlerDelayed(Runnable runnable, long delayMillis)
   {
     GLOBAL_HANDLER.postDelayed(runnable, delayMillis);
   }
 
-  private static void loadUncompletedRequests()
+  private static void loadPendingRequests()
   {
     PRIVATE_HANDLER.post(new Runnable()
     {
       @Override
       public void run()
       {
-        final String jsonString = sharedPrefs.getString(UNCOMPLETED_REQUESTS_PREFS_KEY, null);
+        final String jsonString = sharedPrefs.getString(PENDING_REQUESTS_PREFS_KEY, null);
         PENDING_REQUESTS.addAll(0, jsonStringToRequestList(jsonString));
       }
     });
   }
 
   private static void sendOrEnqueueRequest(final AddOnRequest request)
   {
     PRIVATE_HANDLER.post(new Runnable()
     {
       @Override
@@ skipping 20 matching lines @@
       {
         for (final AddOnRequest request : PENDING_REQUESTS)
         {
           GeckoAppShell.sendRequestToGecko(request);
         }
         PENDING_REQUESTS.clear();
       }
     });
   }
 
-  private static void clearUncompletedRequests()
+  private static void clearPendingRequests()
   {
     PRIVATE_HANDLER.post(new Runnable()
     {
       @Override
       public void run()
       {
-        storeStringPref(UNCOMPLETED_REQUESTS_PREFS_KEY, null);
+        storeStringPref(PENDING_REQUESTS_PREFS_KEY, null);
       }
     });
   }
 
-  private static void storeUncompletedRequest(final AddOnRequest request)
+  private static void storePendingRequest(final AddOnRequest request)
   {
     PRIVATE_HANDLER.post(new Runnable()
     {
       @Override
       public void run()
       {
-        final String jsonString = sharedPrefs.getString(UNCOMPLETED_REQUESTS_PREFS_KEY, null);
+        final String jsonString = sharedPrefs.getString(PENDING_REQUESTS_PREFS_KEY, null);
         try
         {
           final JSONArray jsonArray = jsonString != null ? new JSONArray(jsonString) : new JSONArray();
           jsonArray.put(request.value);
-          storeStringPref(UNCOMPLETED_REQUESTS_PREFS_KEY, jsonArray.toString());
+          storeStringPref(PENDING_REQUESTS_PREFS_KEY, jsonArray.toString());
         }
         catch (JSONException e)
         {
-          Log.e(TAG, "Failed to store uncompleted request with error: " + e.getMessage(), e);
+          Log.e(TAG, "Failed to store pending request with error: " + e.getMessage(), e);
         }
       }
     });
   }
 
   private static List<AddOnRequest> jsonStringToRequestList(final String jsonString)
   {
     final List<AddOnRequest> requestList = new ArrayList<>();
     if (jsonString == null)
     {
@@ skipping 95 matching lines @@
     sendOrEnqueueRequest(request);
   }
 
   public static void setBoolean(final AdblockPlusApiCallback callback, final String name,
       final boolean enable)
   {
     Log.d(TAG, "setBoolean " + enable + " for " + name);
     final AddOnRequest request =
         new AddOnRequest(createRequestData("set" + makeFirstCharacterUppercase(name), enable), callback);
     sendOrEnqueueRequest(request);
-    storeUncompletedRequest(request);
+    storePendingRequest(request);
   }
 
   private static void callFunction(final AdblockPlusApiCallback callback, final String name,
       final Map<String, Object> parameters)
   {
-    // By default, requests are not stored on the uncompleted request prefs. This should apply for
+    // By default, requests are not stored on the pending request prefs. This should apply for
     // requests that doesn't result in save operations performed by the extension
     callFunction(callback, name, parameters, false);
   }
 
   private static void callFunction(final AdblockPlusApiCallback callback, final String name,
       final Map<String, Object> parameters, boolean resendIfAborted)
   {
     final JSONObject requestData = createRequestData(name);
     try
     {
       for (Map.Entry<String, Object> entry : parameters.entrySet())
       {
         requestData.put(entry.getKey(), entry.getValue());
       }
     }
     catch (JSONException e)
     {
       // we're only adding sane objects
       Log.e(TAG, "Creating request data failed with: " + e.getMessage(), e);
     }
     final AddOnRequest request = new AddOnRequest(requestData, callback);
     sendOrEnqueueRequest(request);
     if (resendIfAborted)
     {
-      storeUncompletedRequest(request);
+      storePendingRequest(request);
     }
   }
 
   public static void querySubscriptionListStatus(final AdblockPlusApiCallback callback,
       final String url)
   {
     Log.d(TAG, "querySubscriptionListStatus for " + url);
     final Map<String, Object> parameters = new HashMap<String, Object>();
     parameters.put("url", url);
     callFunction(callback, "isSubscriptionListed", parameters);
@@ skipping 125 matching lines @@
     {
       if (ON_FILTERS_LOAD_EVENT.equals(event))
       {
         // The filters have been loaded by the extension. Given that, we can send all pending requests
         filtersLoaded = true;
         sendPendingRequests();
       }
       else if (ON_FILTERS_SAVE_EVENT.equals(event))
       {
         // All changes have been saved by the extension. That way, we can clear our list of
-        // uncompleted requests
+        // pending requests
         // See https://issues.adblockplus.org/ticket/2853
-        clearUncompletedRequests();
+        clearPendingRequests();
       }
     }
   }
 }