Bug 846772 - Add prefs observing to PrefsHelper/browser.js on Android. r=kats

build/mobile/robocop/Actions.java.in

@@ -23,6 +23,12 @@ public interface Actions {
         /** Blocks until the event has been received and returns data associated with the event. */
         public String blockForEventData();
 
+        /**
+         * Blocks until the event has been received, or until the timeout has been exceeded.
+         * Returns the data associated with the event, if applicable.
+         */
+        public String blockForEventDataWithTimeout(long millis);
+
         /** Polls to see if the event has been received. Once this returns true, subsequent calls will also return true. */
         public boolean eventReceived();
     }

build/mobile/robocop/FennecNativeActions.java.in

@@ -115,20 +115,27 @@ public class FennecNativeActions implements Actions {
         }
 
         public synchronized void blockForEvent() {
+            blockForEvent(MAX_WAIT_MS, true);
+        }
+
+        private synchronized void blockForEvent(long millis, boolean failOnTimeout) {
             long startTime = SystemClock.uptimeMillis();
             long endTime = 0;
             while (! mEventReceived) {
                 try {
-                    this.wait(MAX_WAIT_MS);
+                    this.wait(millis);
                 } catch (InterruptedException ie) {
                     FennecNativeDriver.log(LogLevel.ERROR, ie);
                     break;
                 }
                 endTime = SystemClock.uptimeMillis();
-                if (!mEventReceived && (endTime - startTime >= MAX_WAIT_MS)) {
-                    FennecNativeDriver.logAllStackTraces(FennecNativeDriver.LogLevel.ERROR);
-                    mAsserter.ok(false, "GeckoEventExpecter", 
-                        "blockForEvent timeout: "+mGeckoEvent);
+                if (!mEventReceived && (endTime - startTime >= millis)) {
+                    if (failOnTimeout) {
+                        FennecNativeDriver.logAllStackTraces(FennecNativeDriver.LogLevel.ERROR);
+                        mAsserter.ok(false, "GeckoEventExpecter", 
+                            "blockForEvent timeout: "+mGeckoEvent);
+                    }
+                    mEventData = null;
                     return;
                 }
             }
@@ -199,6 +206,11 @@ public class FennecNativeActions implements Actions {
             return mEventData;
         }
 
+        public synchronized String blockForEventDataWithTimeout(long millis) {
+            blockForEvent(millis, false);
+            return mEventData;
+        }
+
         public synchronized boolean eventReceived() {
             return mEventEverReceived;
         }
@@ -287,20 +299,22 @@ public class FennecNativeActions implements Actions {
             }
         }
 
-        public synchronized void blockForEvent() {
+        private synchronized void blockForEvent(long millis, boolean failOnTimeout) {
             long startTime = SystemClock.uptimeMillis();
             long endTime = 0;
             while (!mPaintDone) {
                 try {
-                    this.wait(MAX_WAIT_MS);
+                    this.wait(millis);
                 } catch (InterruptedException ie) {
                     FennecNativeDriver.log(LogLevel.ERROR, ie);
                     break;
                 }
                 endTime = SystemClock.uptimeMillis();
-                if (!mPaintDone && (endTime - startTime >= MAX_WAIT_MS)) {
-                    FennecNativeDriver.logAllStackTraces(FennecNativeDriver.LogLevel.ERROR);
-                    mAsserter.ok(false, "PaintExpecter", "blockForEvent timeout");
+                if (!mPaintDone && (endTime - startTime >= millis)) {
+                    if (failOnTimeout) {
+                        FennecNativeDriver.logAllStackTraces(FennecNativeDriver.LogLevel.ERROR);
+                        mAsserter.ok(false, "PaintExpecter", "blockForEvent timeout");
+                    }
                     return;
                 }
             }
@@ -311,11 +325,20 @@ public class FennecNativeActions implements Actions {
             }
         }
 
+        public synchronized void blockForEvent() {
+            blockForEvent(MAX_WAIT_MS, true);
+        }
+
         public synchronized String blockForEventData() {
             blockForEvent();
             return null;
         }
 
+        public synchronized String blockForEventDataWithTimeout(long millis) {
+            blockForEvent(millis, false);
+            return null;
+        }
+
         public synchronized boolean eventReceived() {
             return mPaintDone;
         }

mobile/android/base/PrefsHelper.java

@@ -26,21 +26,21 @@ public final class PrefsHelper {
     private static final Map<Integer, PrefHandler> sCallbacks = new HashMap<Integer, PrefHandler>();
     private static int sUniqueRequestId = 1;
 
-    public static void getPref(String prefName, PrefHandler callback) {
+    public static int getPref(String prefName, PrefHandler callback) {
         JSONArray prefs = new JSONArray();
         prefs.put(prefName);
-        getPrefs(prefs, callback);
+        return getPrefs(prefs, callback);
     }
 
-    public static void getPrefs(String[] prefNames, PrefHandler callback) {
+    public static int getPrefs(String[] prefNames, PrefHandler callback) {
         JSONArray prefs = new JSONArray();
         for (String p : prefNames) {
             prefs.put(p);
         }
-        getPrefs(prefs, callback);
+        return getPrefs(prefs, callback);
     }
 
-    public static void getPrefs(JSONArray prefNames, PrefHandler callback) {
+    public static int getPrefs(JSONArray prefNames, PrefHandler callback) {
         int requestId;
         synchronized (PrefsHelper.class) {
             ensureRegistered();
@@ -52,19 +52,25 @@ public final class PrefsHelper {
         GeckoEvent event;
         try {
             JSONObject message = new JSONObject();
-            message.put("requestId", requestId);
+            message.put("requestId", Integer.toString(requestId));
             message.put("preferences", prefNames);
-            event = GeckoEvent.createBroadcastEvent("Preferences:Get", message.toString());
+            event = GeckoEvent.createBroadcastEvent(callback.isObserver() ?
+                "Preferences:Observe" : "Preferences:Get", message.toString());
             GeckoAppShell.sendEventToGecko(event);
         } catch (Exception e) {
-            Log.e(LOGTAG, "Error while composing Preferences:Get message", e);
+            Log.e(LOGTAG, "Error while composing Preferences:" +
+                  (callback.isObserver() ? "Observe" : "Get") + " message", e);
 
             // if we failed to send the message, drop our reference to the callback because
             // otherwise it will leak since we will never get the response
             synchronized (PrefsHelper.class) {
                 sCallbacks.remove(requestId);
             }
+
+            return -1;
         }
+
+        return requestId;
     }
 
     private static void ensureRegistered() {
@@ -78,7 +84,11 @@ public final class PrefsHelper {
                     PrefHandler callback;
                     synchronized (PrefsHelper.class) {
                         try {
-                            callback = sCallbacks.remove(message.getInt("requestId"));
+                            int requestId = message.getInt("requestId");
+                            callback = sCallbacks.get(requestId);
+                            if (callback != null && !callback.isObserver()) {
+                                sCallbacks.remove(requestId);
+                            }
                         } catch (Exception e) {
                             callback = null;
                         }
@@ -142,10 +152,29 @@ public final class PrefsHelper {
         }
     }
 
+    public static void removeObserver(int requestId) {
+        if (requestId < 0) {
+            throw new IllegalArgumentException("Invalid request ID");
+        }
+
+        synchronized (PrefsHelper.class) {
+            PrefHandler callback = sCallbacks.remove(requestId);
+            if (callback == null) {
+                Log.e(LOGTAG, "Unknown request ID " + requestId);
+                return;
+            }
+        }
+
+        GeckoEvent event = GeckoEvent.createBroadcastEvent("Preferences:RemoveObserver",
+                                                           Integer.toString(requestId));
+        GeckoAppShell.sendEventToGecko(event);
+    }
+
     public interface PrefHandler {
         void prefValue(String pref, boolean value);
         void prefValue(String pref, int value);
         void prefValue(String pref, String value);
+        boolean isObserver();
         void finish();
     }
 
@@ -168,5 +197,10 @@ public final class PrefsHelper {
         @Override
         public void finish() {
         }
+
+        @Override
+        public boolean isObserver() {
+            return false;
+        }
     }
 }

mobile/android/base/tests/robocop.ini

@@ -8,6 +8,7 @@
 [testMigration]
 [testLoad]
 [testNewTab]
+[testPrefsObserver]
 [testPanCorrectness]
 # [test_bug720538] # see bug 746876
 [testFlingCorrectness]

mobile/android/base/tests/testPrefsObserver.java.in

@@ -0,0 +1,121 @@
+#filter substitution
+package @ANDROID_PACKAGE_NAME@.tests;
+
+import @ANDROID_PACKAGE_NAME@.*;
+import android.app.Instrumentation;
+
+import org.json.JSONArray;
+import org.json.JSONException;
+import org.json.JSONObject;
+
+/**
+ * Basic test to check bounce-back from overscroll.
+ * - Load the page and verify it draws
+ * - Drag page downwards by 100 pixels into overscroll, verify it snaps back.
+ * - Drag page rightwards by 100 pixels into overscroll, verify it snaps back.
+ */
+public class testPrefsObserver extends BaseTest {
+    private static final String PREF_TEST_PREF = "robocop.tests.dummy";
+    private static final String PREF_REQUEST_ID = "testPrefsObserver";
+    private static final long PREF_TIMEOUT = 10000;
+
+    private Actions.RepeatedEventExpecter mExpecter;
+
+    @Override
+    protected int getTestType() {
+        return TEST_MOCHITEST;
+    }
+
+    public void setPref(boolean value) throws JSONException {
+        mAsserter.dumpLog("Setting pref");
+
+        JSONObject jsonPref = new JSONObject();
+        jsonPref.put("name", PREF_TEST_PREF);
+        jsonPref.put("type", "bool");
+        jsonPref.put("value", value);
+        mActions.sendGeckoEvent("Preferences:Set", jsonPref.toString());
+    }
+
+    public void waitAndCheckPref(boolean value) throws JSONException {
+        mAsserter.dumpLog("Waiting to check pref");
+
+        JSONObject data = null;
+        String requestId = "";
+
+        while (!requestId.equals(PREF_REQUEST_ID)) {
+            data = new JSONObject(mExpecter.blockForEventData());
+            if (!mExpecter.eventReceived()) {
+                mAsserter.ok(false, "Checking pref is correct value", "Didn't receive pref");
+                return;
+            }
+            requestId = data.getString("requestId");
+        }
+
+        JSONObject pref = data.getJSONArray("preferences").getJSONObject(0);
+        mAsserter.is(pref.getString("name"), PREF_TEST_PREF, "Pref name is correct");
+        mAsserter.is(pref.getString("type"), "bool", "Pref type is correct");
+        mAsserter.is(pref.getBoolean("value"), value, "Pref value is correct");
+    }
+
+    public void verifyDisconnect() throws JSONException {
+        mAsserter.dumpLog("Checking pref observer is removed");
+
+        JSONObject pref = null;
+        String requestId = "";
+
+        while (!requestId.equals(PREF_REQUEST_ID)) {
+            String data = mExpecter.blockForEventDataWithTimeout(PREF_TIMEOUT);
+            if (data == null) {
+                mAsserter.ok(true, "Verifying pref is unobserved", "Didn't get unobserved pref");
+                return;
+            }
+            pref = new JSONObject(data);
+            requestId = pref.getString("requestId");
+        }
+
+        mAsserter.ok(false, "Received unobserved pref change", "");
+    }
+
+    public void observePref() throws JSONException {
+        mAsserter.dumpLog("Setting up pref observer");
+
+        // Setup the pref observer
+        JSONArray getPrefData = new JSONArray();
+        getPrefData.put(PREF_TEST_PREF);
+        JSONObject message = new JSONObject();
+        message.put("requestId", PREF_REQUEST_ID);
+        message.put("preferences", getPrefData);
+        mExpecter = mActions.expectGeckoEvent("Preferences:Data");
+        mActions.sendGeckoEvent("Preferences:Observe", message.toString());
+    }
+
+    public void removePrefObserver() {
+        mAsserter.dumpLog("Removing pref observer");
+
+        mActions.sendGeckoEvent("Preferences:RemoveObservers", PREF_REQUEST_ID);
+    }
+
+    public void testPrefsObserver() {
+        blockForGeckoReady();
+
+        try {
+            setPref(false);
+            observePref();
+            waitAndCheckPref(false);
+
+            setPref(true);
+            waitAndCheckPref(true);
+
+            removePrefObserver();
+            setPref(false);
+            verifyDisconnect();
+        } catch (Exception ex) {
+            mAsserter.ok(false, "exception in testPrefsObserver", ex.toString());
+        } finally {
+            // Make sure we remove the observer - if it's already removed, this
+            // will do nothing.
+            removePrefObserver();
+        }
+    }
+}
+

mobile/android/chrome/content/browser.js

@@ -169,6 +169,7 @@ var Strings = {};
 var BrowserApp = {
   _tabs: [],
   _selectedTab: null,
+  _prefObservers: [],
 
   get isTablet() {
     let sysInfo = Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2);
@@ -200,6 +201,8 @@ var BrowserApp = {
     Services.obs.addObserver(this, "Browser:Quit", false);
     Services.obs.addObserver(this, "Preferences:Get", false);
     Services.obs.addObserver(this, "Preferences:Set", false);
+    Services.obs.addObserver(this, "Preferences:Observe", false);
+    Services.obs.addObserver(this, "Preferences:RemoveObservers", false);
     Services.obs.addObserver(this, "ScrollTo:FocusedInput", false);
     Services.obs.addObserver(this, "Sanitize:ClearData", false);
     Services.obs.addObserver(this, "FullScreen:Exit", false);
@@ -883,16 +886,23 @@ var BrowserApp = {
     webBrowserPrint.print(printSettings, download);
   },
 
-  getPreferences: function getPreferences(aPrefNames) {
+  getPreferences: function getPreferences(aPrefsRequest, aListen) {
     try {
-      let json = JSON.parse(aPrefNames);
       let prefs = [];
 
-      for each (let prefName in json.preferences) {
+      for each (let prefName in aPrefsRequest.preferences) {
         let pref = {
           name: prefName
         };
 
+        if (aListen) {
+          if (this._prefObservers[prefName])
+            this._prefObservers[prefName].push(aPrefsRequest.requestId);
+          else
+            this._prefObservers[prefName] = [ aPrefsRequest.requestId ];
+          Services.prefs.addObserver(prefName, this, false);
+        }
+
         // The plugin pref is actually two separate prefs, so
         // we need to handle it differently
         if (prefName == "plugin.enable") {
@@ -956,10 +966,33 @@ var BrowserApp = {
 
       sendMessageToJava({
         type: "Preferences:Data",
-        requestId: json.requestId,    // opaque request identifier, can be any string/int/whatever
+        requestId: aPrefsRequest.requestId,    // opaque request identifier, can be any string/int/whatever
         preferences: prefs
       });
-    } catch (e) {}
+
+    } catch (e) {
+      dump("Unhandled exception getting prefs: " + e);
+    }
+  },
+
+  removePreferenceObservers: function removePreferenceObservers(aRequestId) {
+    let newPrefObservers = [];
+    for (let prefName in this._prefObservers) {
+      let requestIds = this._prefObservers[prefName];
+      // Remove the requestID from the preference handlers
+      let i = requestIds.indexOf(aRequestId);
+      if (i >= 0) {
+        requestIds.splice(i, 1);
+      }
+
+      // If there are no more request IDs, remove the observer
+      if (requestIds.length == 0) {
+        Services.prefs.removeObserver(prefName, this);
+      } else {
+        newPrefObservers[prefName] = requestIds;
+      }
+    }
+    this._prefObservers = newPrefObservers;
   },
 
   setPreferences: function setPreferences(aPref) {
@@ -1185,13 +1218,21 @@ var BrowserApp = {
         break;
 
       case "Preferences:Get":
-        this.getPreferences(aData);
+        this.getPreferences(JSON.parse(aData));
         break;
 
       case "Preferences:Set":
         this.setPreferences(aData);
         break;
 
+      case "Preferences:Observe":
+        this.getPreferences(JSON.parse(aData), true);
+        break;
+
+      case "Preferences:RemoveObservers":
+        this.removePreferenceObservers(aData);
+        break;
+
       case "ScrollTo:FocusedInput":
         // these messages come from a change in the viewable area and not user interaction
         // we allow scrolling to the selected input, but not zooming the page
@@ -1257,6 +1298,14 @@ var BrowserApp = {
         gViewportMargins = JSON.parse(aData);
         break;
 
+      case "nsPref:changed":
+        for each (let requestId in this._prefObservers[aData]) {
+          let request = { requestId : requestId,
+                          preferences : [ aData ] };
+          this.getPreferences(request, false);
+        }
+        break;
+
       default:
         dump('BrowserApp.observe: unexpected topic "' + aTopic + '"\n');
         break;