Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: mobile/android/thirdparty/org/adblockplus/browser/AddOnBridge.java

Issue 29350065: Issue 2853 - Settings changes are sometimes not saved if the user quits the app (Closed)
Patch Set: Fixing a bug where uncompleted requests could be sent out of order with the use of filter load pool… Created Dec. 26, 2016, 9:32 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « mobile/android/base/GeckoApplication.java ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: mobile/android/thirdparty/org/adblockplus/browser/AddOnBridge.java
===================================================================
--- a/mobile/android/thirdparty/org/adblockplus/browser/AddOnBridge.java
+++ b/mobile/android/thirdparty/org/adblockplus/browser/AddOnBridge.java
@@ -12,70 +12,204 @@
* 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.browser;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import android.annotation.SuppressLint;
+import android.content.Context;
+import android.content.SharedPreferences;
import android.os.Handler;
import android.os.HandlerThread;
import android.util.Log;
+import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
+import org.mozilla.gecko.EventDispatcher;
import org.mozilla.gecko.GeckoAppShell;
+import org.mozilla.gecko.util.GeckoEventListener;
import org.mozilla.gecko.util.GeckoRequest;
import org.mozilla.gecko.util.NativeJSObject;
@SuppressLint("DefaultLocale")
public class AddOnBridge
{
private static final String TAG = "AdblockBrowser.AddOnBridge";
private static final String REQUEST_NAME = "AdblockPlus:Api";
- // Timeout for checking filter loading (in seconds)
- private static final int QUERY_GET_FILTERS_LOADED_TIMEOUT = 30;
- // How long to wait between retries (in milliseconds)
- private static final int QUERY_GET_FILTERS_LOADED_DELAY = 500;
// 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,
+ // 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<>();
+ private static final String UNCOMPLETED_REQUESTS_PREFS_KEY = "UNCOMPLETED_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();
+ }
+
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_HANDLER.post(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ final String jsonString = sharedPrefs.getString(UNCOMPLETED_REQUESTS_PREFS_KEY, null);
+ PENDING_REQUESTS.addAll(0, jsonStringToRequestList(jsonString));
+ }
+ });
+ }
+
+ private static void sendOrEnqueueRequest(final AddOnRequest request)
+ {
+ PRIVATE_HANDLER.post(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ if (!filtersLoaded)
+ {
+ PENDING_REQUESTS.add(request);
+ }
+ else
+ {
+ GeckoAppShell.sendRequestToGecko(request);
+ }
+ }
+ });
+ }
+
+ private static void sendPendingRequests()
+ {
+ PRIVATE_HANDLER.post(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ for (final AddOnRequest request : PENDING_REQUESTS)
+ {
+ GeckoAppShell.sendRequestToGecko(request);
+ }
+ PENDING_REQUESTS.clear();
+ }
+ });
+ }
+
+ private static void clearUncompletedRequests()
+ {
+ PRIVATE_HANDLER.post(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ storeStringPref(UNCOMPLETED_REQUESTS_PREFS_KEY, null);
+ }
+ });
+ }
+
+ private static void storeUncompletedRequest(final AddOnRequest request)
+ {
+ PRIVATE_HANDLER.post(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ final String jsonString = sharedPrefs.getString(UNCOMPLETED_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());
+ }
+ catch (JSONException e)
+ {
+ Log.e(TAG, "Failed to store uncompleted request with error: " + e.getMessage(), e);
+ }
+ }
+ });
+ }
+
+ private static List<AddOnRequest> jsonStringToRequestList(final String jsonString)
+ {
+ final List<AddOnRequest> requestList = new ArrayList<>();
+ if (jsonString == null)
+ {
+ return requestList;
+ }
+ try
+ {
+ final JSONArray jsonArray = new JSONArray(jsonString);
+ for (int i = 0; i < jsonArray.length(); i++)
+ {
+ final AddOnRequest request = new AddOnRequest(jsonArray.getJSONObject(i), null);
+ requestList.add(request);
+ }
+ }
+ catch (JSONException e)
+ {
+ Log.e(TAG, "Failed to parse json to request list with error: " + e.getMessage(), e);
+ }
+ return requestList;
+ }
+
+ private static void storeStringPref(String key, String value)
+ {
+ final SharedPreferences.Editor editor = sharedPrefs.edit();
+ editor.putString(key, value);
+ editor.commit();
+ }
+
public static boolean getBooleanFromJsObject(final NativeJSObject obj, final String name,
final boolean defaultValue)
{
try
{
return obj.getBoolean(name);
}
catch (final Exception e)
@@ -134,47 +268,61 @@ public class AddOnBridge
return str;
}
return Character.toString(Character.toUpperCase(str.charAt(0))) + str.substring(1);
}
public static void queryValue(final AdblockPlusApiCallback callback, final String name)
{
Log.d(TAG, "queryValue for " + name);
- GeckoAppShell.sendRequestToGecko(
- new ChainedRequest(
- createRequestData("get" + makeFirstCharacterUppercase(name)),
- callback));
+ final AddOnRequest request =
+ new AddOnRequest(createRequestData("get" + makeFirstCharacterUppercase(name)), callback);
+ sendOrEnqueueRequest(request);
}
public static void setBoolean(final AdblockPlusApiCallback callback, final String name,
final boolean enable)
{
Log.d(TAG, "setBoolean " + enable + " for " + name);
- GeckoAppShell.sendRequestToGecko(
- new ChainedRequest(
- createRequestData("set" + makeFirstCharacterUppercase(name), enable),
- callback));
+ final AddOnRequest request =
+ new AddOnRequest(createRequestData("set" + makeFirstCharacterUppercase(name), enable), callback);
+ sendOrEnqueueRequest(request);
+ storeUncompletedRequest(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
+ // 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);
}
- GeckoAppShell.sendRequestToGecko(new ChainedRequest(requestData, callback));
+ final AddOnRequest request = new AddOnRequest(requestData, callback);
+ sendOrEnqueueRequest(request);
+ if (resendIfAborted)
+ {
+ storeUncompletedRequest(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);
@@ -186,33 +334,33 @@ public class AddOnBridge
{
Log.d(TAG, "addSubscription for " + url + " (" + title + ")");
final Map<String, Object> parameters = new HashMap<String, Object>();
parameters.put("url", url);
if (title != null)
{
parameters.put("title", title);
}
- callFunction(callback, "addSubscription", parameters);
+ callFunction(callback, "addSubscription", parameters, true);
}
public static void queryActiveSubscriptions(final AdblockPlusApiCallback callback)
{
Log.d(TAG, "queryActiveSubscriptions");
final Map<String, Object> parameters = new HashMap<String, Object>();
callFunction(callback, "getActiveSubscriptions", parameters);
}
public static void removeSubscription(final AdblockPlusApiCallback callback,
final String url)
{
Log.d(TAG, "removeSubscription for " + url);
final Map<String, Object> parameters = new HashMap<String, Object>();
parameters.put("url", url);
- callFunction(callback, "removeSubscription", parameters);
+ callFunction(callback, "removeSubscription", parameters, true);
}
public static void queryIsLocal(final AdblockPlusApiCallback callback,
final String url)
{
Log.d(TAG, "queryIsLocal for " + url);
final Map<String, Object> parameters = new HashMap<String, Object>();
parameters.put("url", url);
@@ -230,50 +378,29 @@ public class AddOnBridge
public static void whitelistSite(final AdblockPlusApiCallback callback, final String url,
final boolean whitelisted)
{
Log.d(TAG, "whitelistSite for " + url);
final Map<String, Object> parameters = new HashMap<String, Object>();
parameters.put("url", url);
parameters.put("whitelisted", whitelisted);
- callFunction(callback, "whitelistSite", parameters);
+ callFunction(callback, "whitelistSite", parameters, true);
}
- private static class ChainedRequest extends GeckoRequest
+ private static class AddOnRequest extends GeckoRequest
{
private final JSONObject value;
private final AdblockPlusApiCallback apiCallback;
- private final boolean checkForFiltersLoaded;
- private final long creationTime;
- public ChainedRequest(final JSONObject value, final AdblockPlusApiCallback callback,
- final boolean checkForFiltersLoaded, final long creationTime)
+ AddOnRequest(final JSONObject value, final AdblockPlusApiCallback callback)
{
- super(AddOnBridge.REQUEST_NAME,
- checkForFiltersLoaded ? createRequestData("getFiltersLoaded") : value);
+ super(AddOnBridge.REQUEST_NAME, value);
this.value = value;
this.apiCallback = callback;
- this.checkForFiltersLoaded = checkForFiltersLoaded;
- this.creationTime = creationTime;
- }
-
- public ChainedRequest(final JSONObject value, final AdblockPlusApiCallback callback)
- {
- this(value, callback, true, System.currentTimeMillis());
- }
-
- public ChainedRequest cloneForRetry()
- {
- return new ChainedRequest(this.value, this.apiCallback, true, this.creationTime);
- }
-
- public ChainedRequest cloneForRequest()
- {
- return new ChainedRequest(this.value, this.apiCallback, false, this.creationTime);
}
private void invokeSuccessCallback(final NativeJSObject jsObject)
{
try
{
if (this.apiCallback != null)
{
@@ -294,73 +421,51 @@ public class AddOnBridge
}
}
private void invokeFailureCallback(final NativeJSObject jsObject)
{
invokeFailureCallback(getStringFromJsObject(jsObject, "error", "unknown error"));
}
- private void attemptRetry()
- {
- if (System.currentTimeMillis() - this.creationTime > (QUERY_GET_FILTERS_LOADED_TIMEOUT * 1000))
- {
- this.invokeFailureCallback("getFiltersLoaded timeout");
- }
- else
- {
- Log.d(TAG, "Retrying: " + this.value);
- final ChainedRequest next = this.cloneForRetry();
- PRIVATE_HANDLER.postDelayed(new Runnable()
- {
- @Override
- public void run()
- {
- GeckoAppShell.sendRequestToGecko(next);
- }
- }, QUERY_GET_FILTERS_LOADED_DELAY);
- }
- }
-
@Override
public void onError(final NativeJSObject error)
{
- if (this.checkForFiltersLoaded)
- {
- this.attemptRetry();
- }
- else
- {
- this.invokeFailureCallback(
- "GeckoRequest error: " + error.optString("message", "<no message>") + "\n" +
- error.optString("stack", "<no stack>"));
- }
+ this.invokeFailureCallback(
+ "GeckoRequest error: " + error.optString("message", "<no message>") + "\n" +
+ error.optString("stack", "<no stack>"));
}
@Override
public void onResponse(final NativeJSObject jsObject)
{
- if (this.checkForFiltersLoaded)
+ if (getBooleanFromJsObject(jsObject, "success", false))
{
- if (getBooleanFromJsObject(jsObject, "success", false)
- && getBooleanFromJsObject(jsObject, "value", false))
- {
- GeckoAppShell.sendRequestToGecko(this.cloneForRequest());
- }
- else
- {
- this.attemptRetry();
- }
+ this.invokeSuccessCallback(jsObject);
}
else
{
- if (getBooleanFromJsObject(jsObject, "success", false))
- {
- this.invokeSuccessCallback(jsObject);
- }
- else
- {
- this.invokeFailureCallback(jsObject);
- }
+ this.invokeFailureCallback(jsObject);
+ }
+ }
+ }
+
+ private static class AddOnEventListener implements GeckoEventListener
+ {
+ @Override
+ public void handleMessage(String event, JSONObject message)
+ {
+ 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
+ // See https://issues.adblockplus.org/ticket/2853
+ clearUncompletedRequests();
}
}
}
}
« no previous file with comments | « mobile/android/base/GeckoApplication.java ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld