Bug 969672 - Handle declined engines on Android. r=nalexander

mobile/android/base/sync/GlobalSession.java

@@ -18,6 +18,7 @@ import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.json.simple.JSONArray;
 import org.json.simple.parser.ParseException;
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.sync.crypto.CryptoException;
@@ -374,6 +375,7 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
   }
 
   public void updateMetaGlobalInPlace() {
+    config.metaGlobal.declined = this.declinedEngineNames();
     ExtendedJSONObject engines = config.metaGlobal.getEngines();
     for (Entry<String, EngineSettings> pair : enginesToUpdate.entrySet()) {
       if (pair.getValue() == null) {
@@ -651,6 +653,38 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
             Utils.toCommaSeparatedString(config.enabledEngineNames) + "' from meta/global.");
       }
     }
+
+    // Persist declined.
+    // Our declined engines at any point are:
+    // Whatever they were remotely, plus whatever they were locally, less any
+    // engines that were just enabled locally or remotely.
+    // If remote just 'won', our recently enabled list just got cleared.
+    final HashSet<String> allDeclined = new HashSet<String>();
+
+    final Set<String> newRemoteDeclined = global.getDeclinedEngineNames();
+    final Set<String> oldLocalDeclined = config.declinedEngineNames;
+
+    allDeclined.addAll(newRemoteDeclined);
+    allDeclined.addAll(oldLocalDeclined);
+
+    if (config.userSelectedEngines != null) {
+      for (Entry<String, Boolean> selection : config.userSelectedEngines.entrySet()) {
+        if (selection.getValue()) {
+          allDeclined.remove(selection.getKey());
+        }
+      }
+    }
+
+    config.declinedEngineNames = allDeclined;
+    if (config.declinedEngineNames.isEmpty()) {
+      Logger.debug(LOG_TAG, "meta/global reported no declined engine names, and we have none declined locally.");
+    } else {
+      if (Logger.shouldLogVerbose(LOG_TAG)) {
+        Logger.trace(LOG_TAG, "Persisting declined engine names '" +
+            Utils.toCommaSeparatedString(config.declinedEngineNames) + "' from meta/global.");
+      }
+    }
+
     config.persistToPrefs();
     advance();
   }
@@ -901,6 +935,27 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
     resetStages(this.getSyncStagesByName(names));
   }
 
+  /**
+   * Engines to explicitly mark as declined in a fresh meta/global record.
+   * <p>
+   * Returns an empty array if the user hasn't elected to customize data types,
+   * or an array of engines that the user un-checked during customization.
+   * <p>
+   * Engines that Android Sync doesn't recognize are <b>not</b> included in
+   * the returned array.
+   *
+   * @return a new JSONArray of engine names.
+   */
+  @SuppressWarnings("unchecked")
+  protected JSONArray declinedEngineNames() {
+    final JSONArray declined = new JSONArray();
+    for (String engine : config.declinedEngineNames) {
+      declined.add(engine);
+    };
+
+    return declined;
+  }
+
   /**
    * Engines to include in a fresh meta/global record.
    * <p>
@@ -983,6 +1038,10 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
     metaGlobal.setStorageVersion(STORAGE_VERSION);
     metaGlobal.setEngines(engines);
 
+    // We assume that the config's declined engines have been updated
+    // according to the user's selections.
+    metaGlobal.setDeclinedEngineNames(this.declinedEngineNames());
+
     return metaGlobal;
   }
 

mobile/android/base/sync/MetaGlobal.java

@@ -6,11 +6,13 @@ package org.mozilla.gecko.sync;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import org.json.simple.JSONArray;
 import org.json.simple.parser.ParseException;
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.sync.MetaGlobalException.MetaGlobalMalformedSyncIDException;
@@ -27,6 +29,7 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
 
   // Fields.
   protected ExtendedJSONObject  engines;
+  protected JSONArray           declined;
   protected Long                storageVersion;
   protected String              syncID;
 
@@ -77,6 +80,7 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
     json.put("storageVersion", storageVersion);
     json.put("engines", engines);
     json.put("syncID", syncID);
+    json.put("declined", declined);
     return json;
   }
 
@@ -93,14 +97,18 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
     return record;
   }
 
-  public void setFromRecord(CryptoRecord record) throws IllegalStateException, IOException, ParseException, NonObjectJSONException {
+  public void setFromRecord(CryptoRecord record) throws IllegalStateException, IOException, ParseException, NonObjectJSONException, NonArrayJSONException {
     if (record == null) {
       throw new IllegalArgumentException("Cannot set meta/global from null record");
     }
     Logger.debug(LOG_TAG, "meta/global is " + record.payload.toJSONString());
     this.storageVersion = (Long) record.payload.get("storageVersion");
     this.syncID = (String) record.payload.get("syncID");
+
     setEngines(record.payload.getObject("engines"));
+
+    // Accepts null -- declined can be missing.
+    setDeclinedEngineNames(record.payload.getArray("declined"));
   }
 
   public Long getStorageVersion() {
@@ -115,6 +123,59 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
     return engines;
   }
 
+  @SuppressWarnings("unchecked")
+  public void declineEngine(String engine) {
+    if (this.declined == null) {
+      JSONArray replacement = new JSONArray();
+      replacement.add(engine);
+      setDeclinedEngineNames(replacement);
+      return;
+    }
+
+    this.declined.add(engine);
+  }
+
+  @SuppressWarnings("unchecked")
+  public void declineEngineNames(Collection<String> additional) {
+    if (this.declined == null) {
+      JSONArray replacement = new JSONArray();
+      replacement.addAll(additional);
+      setDeclinedEngineNames(replacement);
+      return;
+    }
+
+    for (String engine : additional) {
+      if (!this.declined.contains(engine)) {
+        this.declined.add(engine);
+      }
+    }
+  }
+
+  public void setDeclinedEngineNames(JSONArray declined) {
+    if (declined == null) {
+      this.declined = new JSONArray();
+      return;
+    }
+    this.declined = declined;
+  }
+
+  /**
+   * Return the set of engines that we support (given as an argument)
+   * but the user hasn't explicitly declined on another device.
+   *
+   * Can return the input if the user hasn't declined any engines.
+   */
+  public Set<String> getNonDeclinedEngineNames(Set<String> supported) {
+    if (this.declined == null ||
+        this.declined.isEmpty()) {
+      return supported;
+    }
+
+    final Set<String> result = new HashSet<String>(supported);
+    result.removeAll(this.declined);
+    return result;
+  }
+
   public void setEngines(ExtendedJSONObject engines) {
     if (engines == null) {
       engines = new ExtendedJSONObject();
@@ -196,6 +257,14 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
     return new HashSet<String>(engines.keySet());
   }
 
+  @SuppressWarnings("unchecked")
+  public Set<String> getDeclinedEngineNames() {
+    if (declined == null) {
+      return null;
+    }
+    return new HashSet<String>(declined);
+  }
+
   /**
    * Returns if the server settings and local settings match.
    * Throws a specific MetaGlobalException if that's not the case.

mobile/android/base/sync/SyncConfiguration.java

@@ -198,6 +198,7 @@ public class SyncConfiguration {
    * fresh meta/global record for upload.
    */
   public Set<String> enabledEngineNames;
+  public Set<String> declinedEngineNames = new HashSet<String>();
 
   /**
    * Names of stages to sync <it>this sync</it>, or <code>null</code> to sync
@@ -248,6 +249,7 @@ public class SyncConfiguration {
   public static final String PREF_SYNC_ID = "syncID";
 
   public static final String PREF_ENABLED_ENGINE_NAMES = "enabledEngineNames";
+  public static final String PREF_DECLINED_ENGINE_NAMES = "declinedEngineNames";
   public static final String PREF_USER_SELECTED_ENGINES_TO_SYNC = "userSelectedEngines";
   public static final String PREF_USER_SELECTED_ENGINES_TO_SYNC_TIMESTAMP = "userSelectedEnginesTimestamp";
 
@@ -311,27 +313,47 @@ public class SyncConfiguration {
   }
 
   /**
-   * Gets the engine names that are enabled in meta/global.
+   * Gets the engine names that are enabled, declined, or other (depending on pref) in meta/global.
    *
    * @param prefs
    *          SharedPreferences that the engines are associated with.
+   * @param pref
+   *          The preference name to use. E.g, PREF_ENABLED_ENGINE_NAMES.
    * @return Set<String> of the enabled engine names if they have been stored,
    *         or null otherwise.
    */
-  public static Set<String> getEnabledEngineNames(SharedPreferences prefs) {
+  protected static Set<String> getEngineNamesFromPref(SharedPreferences prefs, String pref) {
-    String json = prefs.getString(PREF_ENABLED_ENGINE_NAMES, null);
+    final String json = prefs.getString(pref, null);
     if (json == null) {
       return null;
     }
     try {
-      ExtendedJSONObject o = ExtendedJSONObject.parseJSONObject(json);
+      final ExtendedJSONObject o = ExtendedJSONObject.parseJSONObject(json);
       return new HashSet<String>(o.keySet());
     } catch (Exception e) {
-      // enabledEngineNames can be null.
       return null;
     }
   }
 
+  /**
+   * Returns the set of engine names that the user has enabled. If none
+   * have been stored in prefs, <code>null</code> is returned.
+   */
+  public static Set<String> getEnabledEngineNames(SharedPreferences prefs) {
+      return getEngineNamesFromPref(prefs, PREF_ENABLED_ENGINE_NAMES);
+  }
+
+  /**
+   * Returns the set of engine names that the user has declined.
+   */
+  public static Set<String> getDeclinedEngineNames(SharedPreferences prefs) {
+    final Set<String> names = getEngineNamesFromPref(prefs, PREF_DECLINED_ENGINE_NAMES);
+    if (names == null) {
+        return new HashSet<String>();
+    }
+    return names;
+  }
+
   /**
    * Gets the engines whose sync states have been changed by the user through the
    * SelectEnginesActivity.
@@ -371,6 +393,9 @@ public class SyncConfiguration {
   /**
    * Store a Map of engines and their sync states to prefs.
    *
+   * Any engine that's disabled in the input is also recorded
+   * as a declined engine, overwriting the stored values.
+   *
    * @param prefs
    *          SharedPreferences that the engines are associated with.
    * @param selectedEngines
@@ -378,20 +403,33 @@ public class SyncConfiguration {
    */
   public static void storeSelectedEnginesToPrefs(SharedPreferences prefs, Map<String, Boolean> selectedEngines) {
     ExtendedJSONObject jObj = new ExtendedJSONObject();
+    HashSet<String> declined = new HashSet<String>();
     for (Entry<String, Boolean> e : selectedEngines.entrySet()) {
-      jObj.put(e.getKey(), e.getValue());
+      final Boolean enabled = e.getValue();
+      final String engine = e.getKey();
+      jObj.put(engine, enabled);
+      if (!enabled) {
+        declined.add(engine);
+      }
     }
+
+    // Our history checkbox drives form history, too.
+    // We don't need to do this for enablement: that's done at retrieval time.
+    if (selectedEngines.containsKey("history") && !selectedEngines.get("history").booleanValue()) {
+        declined.add("forms");
+    }
+
     String json = jObj.toJSONString();
     long currentTime = System.currentTimeMillis();
     Editor edit = prefs.edit();
     edit.putString(PREF_USER_SELECTED_ENGINES_TO_SYNC, json);
+    edit.putString(PREF_DECLINED_ENGINE_NAMES, setToJSONObjectString(declined));
     edit.putLong(PREF_USER_SELECTED_ENGINES_TO_SYNC_TIMESTAMP, currentTime);
     Logger.error(LOG_TAG, "Storing user-selected engines at [" + currentTime + "].");
     edit.commit();
   }
 
   public void loadFromPrefs(SharedPreferences prefs) {
-
     if (prefs.contains(PREF_CLUSTER_URL)) {
       String u = prefs.getString(PREF_CLUSTER_URL, null);
       try {
@@ -406,6 +444,7 @@ public class SyncConfiguration {
       Logger.trace(LOG_TAG, "Set syncID from bundle: " + syncID);
     }
     enabledEngineNames = getEnabledEngineNames(prefs);
+    declinedEngineNames = getDeclinedEngineNames(prefs);
     userSelectedEngines = getUserSelectedEngines(prefs);
     userSelectedEnginesTimestamp = prefs.getLong(PREF_USER_SELECTED_ENGINES_TO_SYNC_TIMESTAMP, 0);
     // We don't set crypto/keys here because we need the syncKeyBundle to decrypt the JSON
@@ -417,6 +456,14 @@ public class SyncConfiguration {
     this.persistToPrefs(this.getPrefs());
   }
 
+  private static String setToJSONObjectString(Set<String> set) {
+    ExtendedJSONObject o = new ExtendedJSONObject();
+    for (String name : set) {
+      o.put(name, 0);
+    }
+    return o.toJSONString();
+  }
+
   public void persistToPrefs(SharedPreferences prefs) {
     Editor edit = prefs.edit();
     if (clusterURL == null) {
@@ -430,11 +477,12 @@ public class SyncConfiguration {
     if (enabledEngineNames == null) {
       edit.remove(PREF_ENABLED_ENGINE_NAMES);
     } else {
-      ExtendedJSONObject o = new ExtendedJSONObject();
+      edit.putString(PREF_ENABLED_ENGINE_NAMES, setToJSONObjectString(enabledEngineNames));
-      for (String engineName : enabledEngineNames) {
+    }
-        o.put(engineName, 0);
+    if (declinedEngineNames.isEmpty()) {
-      }
+      edit.remove(PREF_DECLINED_ENGINE_NAMES);
-      edit.putString(PREF_ENABLED_ENGINE_NAMES, o.toJSONString());
+    } else {
+      edit.putString(PREF_DECLINED_ENGINE_NAMES, setToJSONObjectString(declinedEngineNames));
     }
     if (userSelectedEngines == null) {
       edit.remove(PREF_USER_SELECTED_ENGINES_TO_SYNC);

mobile/android/base/sync/stage/ServerSyncStage.java

@@ -515,7 +515,9 @@ public abstract class ServerSyncStage extends AbstractSessionManagingSyncStage i
       if (!isEnabled) {
         // Engine has been disabled; update meta/global with engine removal for upload.
         session.removeEngineFromMetaGlobal(name);
+        session.config.declinedEngineNames.add(name);
       } else {
+        session.config.declinedEngineNames.remove(name);
         // Add engine with new syncID to meta/global for upload.
         String newSyncID = Utils.generateGuid();
         session.recordForMetaGlobalUpdate(name, new EngineSettings(newSyncID, this.getStorageVersion()));

mobile/android/tests/background/junit3/src/sync/TestSyncConfiguration.java

@@ -25,6 +25,33 @@ public class TestSyncConfiguration extends AndroidSyncTestCase implements PrefsS
     return this.getApplicationContext().getSharedPreferences(name, mode);
   }
 
+  /**
+   * Ensure that declined engines persist through prefs.
+   */
+  public void testDeclinedEngineNames() {
+    SyncConfiguration config = null;
+    SharedPreferences prefs = getPrefs(TEST_PREFS_NAME, 0);
+
+    config = newSyncConfiguration();
+    config.declinedEngineNames = new HashSet<String>();
+    config.declinedEngineNames.add("test1");
+    config.declinedEngineNames.add("test2");
+    config.persistToPrefs();
+    assertTrue(prefs.contains(SyncConfiguration.PREF_DECLINED_ENGINE_NAMES));
+    config = newSyncConfiguration();
+    Set<String> expected = new HashSet<String>();
+    for (String name : new String[] { "test1", "test2" }) {
+      expected.add(name);
+    }
+    assertEquals(expected, config.declinedEngineNames);
+
+    config.declinedEngineNames = null;
+    config.persistToPrefs();
+    assertFalse(prefs.contains(SyncConfiguration.PREF_DECLINED_ENGINE_NAMES));
+    config = newSyncConfiguration();
+    assertNull(config.declinedEngineNames);
+  }
+
   public void testEnabledEngineNames() {
     SyncConfiguration config = null;
     SharedPreferences prefs = getPrefs(TEST_PREFS_NAME, 0);