Sync: local data verification for Android tests

This CL dramatically improves the ability to verify local Sync data in
Java Android tests. This is done by plumbing through the
ProfileSyncService's GetAllNodes method through JNI.

This new util will also allow for the server-side modification and
deletion of Sync entities in a future CL.

An older version of local data verification
(relying only on entity counts) has been removed.

BUG=365774

Review URL: https://codereview.chromium.org/1127233008

Cr-Commit-Position: refs/heads/master@{#330842}

chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java

@@ -18,6 +18,8 @@ import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chrome.browser.identity.UniqueIdentificationGenerator;
 import org.chromium.sync.internal_api.pub.PassphraseType;
 import org.chromium.sync.internal_api.pub.base.ModelType;
+import org.json.JSONArray;
+import org.json.JSONException;
 
 import java.util.HashSet;
 import java.util.Iterator;
@@ -48,6 +50,30 @@ public class ProfileSyncService {
         public void syncStateChanged();
     }
 
+    /**
+     * Callback for getAllNodes.
+     */
+    public static class GetAllNodesCallback {
+        private String mNodesString;
+        private boolean mHasResult = false;
+
+        // Invoked when getAllNodes completes.
+        public void onResult(String nodesString) {
+            mNodesString = nodesString;
+            mHasResult = true;
+        }
+
+        // Whether this callback contains a result.
+        public boolean hasResult() {
+            return mHasResult;
+        }
+
+        // Returns the result of GetAllNodes as a JSONArray.
+        public JSONArray getNodesAsJsonArray() throws JSONException {
+            return new JSONArray(mNodesString);
+        }
+    }
+
     private static final String TAG = "ProfileSyncService";
 
     @VisibleForTesting
@@ -586,6 +612,22 @@ public class ProfileSyncService {
                                     prompted);
     }
 
+    /**
+     * Invokes the onResult method of the callback from native code.
+     */
+    @CalledByNative
+    private static void onGetAllNodesResult(GetAllNodesCallback callback, String nodes) {
+        callback.onResult(nodes);
+    }
+
+    /**
+     * Retrieves a JSON version of local Sync data via the native GetAllNodes method.
+     * This method is asynchronous; the result will be sent to the callback.
+     */
+    public void getAllNodes(GetAllNodesCallback callback) {
+        nativeGetAllNodes(mNativeProfileSyncServiceAndroid, callback);
+    }
+
     // Native methods
     private native long nativeInit();
     private native void nativeEnableSync(long nativeProfileSyncServiceAndroid);
@@ -641,4 +683,6 @@ public class ProfileSyncService {
     private native long nativeGetLastSyncedTimeForTest(long nativeProfileSyncServiceAndroid);
     private native void nativeOverrideNetworkResourcesForTest(
             long nativeProfileSyncServiceAndroid, long networkResources);
+    private native void nativeGetAllNodes(
+            long nativeProfileSyncServiceAndroid, GetAllNodesCallback callback);
 }

chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTest.java

@@ -8,6 +8,7 @@ import android.accounts.Account;
 import android.app.Activity;
 import android.test.suitebuilder.annotation.LargeTest;
 import android.util.Log;
+import android.util.Pair;
 
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
@@ -28,8 +29,9 @@ import org.chromium.sync.protocol.TypedUrlSpecifics;
 import org.chromium.sync.signin.AccountManagerHelper;
 import org.chromium.sync.signin.ChromeSigninController;
 import org.chromium.ui.base.PageTransition;
+import org.json.JSONObject;
 
-import java.util.Map;
+import java.util.List;
 import java.util.concurrent.TimeoutException;
 
 /**
@@ -197,48 +199,12 @@ public class SyncTest extends SyncTestBase {
                 synced);
     }
 
-    /**
-     * Retrieves a local entity count and asserts that {@code expected} entities exist on the client
-     * with the ModelType represented by {@code modelTypeString}.
-     *
-     * TODO(pvalenzuela): Replace modelTypeString with the native ModelType enum or something else
-     * that will avoid callers needing to specify the native string version.
-     */
-    private void assertLocalEntityCount(String modelTypeString, int expected)
-            throws InterruptedException {
-        final SyncTestUtil.AboutSyncInfoGetter aboutInfoGetter =
-                new SyncTestUtil.AboutSyncInfoGetter(getActivity());
-        try {
-            runTestOnUiThread(aboutInfoGetter);
-        } catch (Throwable t) {
-            Log.w(TAG,
-                    "Exception while trying to fetch about sync info from ProfileSyncService.", t);
-            fail("Unable to fetch sync info from ProfileSyncService.");
-        }
-        boolean receivedModelTypeCounts = CriteriaHelper.pollForCriteria(new Criteria() {
-                @Override
-                public boolean isSatisfied() {
-                    return !aboutInfoGetter.getModelTypeCount().isEmpty();
-                }
-        }, SyncTestUtil.UI_TIMEOUT_MS, SyncTestUtil.CHECK_INTERVAL_MS);
-        assertTrue("No model type counts present. Sync might be disabled.",
-                receivedModelTypeCounts);
-
-        Map<String, Integer> modelTypeCount = aboutInfoGetter.getModelTypeCount();
-        assertTrue("No count for model type: " + modelTypeString,
-                modelTypeCount.containsKey(modelTypeString));
-
-        // Reduce by one to account for type's root entity. This entity is always included but
-        // these tests don't care about its existence.
-        int actual = modelTypeCount.get(modelTypeString) - 1;
-        assertEquals("Expected amount of local client entities did not match.", expected, actual);
-    }
-
     @LargeTest
     @Feature({"Sync"})
-    public void testDownloadTypedUrl() throws InterruptedException {
+    public void testDownloadTypedUrl() throws Exception {
         setupTestAccountAndSignInToSync(CLIENT_ID);
-        assertLocalEntityCount("Typed URLs", 0);
+        assertEquals("No typed URLs should exist on the client by default.",
+                0, SyncTestUtil.getLocalData(mContext, "Typed URLs").size());
 
         String url = "data:text,testDownloadTypedUrl";
         EntitySpecifics specifics = new EntitySpecifics();
@@ -250,27 +216,37 @@ public class SyncTest extends SyncTestBase {
         mFakeServerHelper.injectUniqueClientEntity(url /* name */, specifics);
 
         SyncTestUtil.triggerSyncAndWaitForCompletion(mContext);
-        assertLocalEntityCount("Typed URLs", 1);
 
-        // TODO(pvalenzuela): Also verify that the downloaded typed URL matches the one that was
-        // injected. This data should be retrieved from the Sync node browser data.
+        List<Pair<String, JSONObject>> typedUrls = SyncTestUtil.getLocalData(
+                mContext, "Typed URLs");
+        assertEquals("Only the injected typed URL should exist on the client.",
+                1, typedUrls.size());
+        JSONObject typedUrl = typedUrls.get(0).second;
+        assertEquals("The wrong URL was found for the typed URL.", url, typedUrl.getString("url"));
     }
 
     @LargeTest
     @Feature({"Sync"})
-    public void testDownloadBookmark() throws InterruptedException {
+    public void testDownloadBookmark() throws Exception {
         setupTestAccountAndSignInToSync(CLIENT_ID);
-        // 3 bookmark folders exist by default: Bookmarks Bar, Other Bookmarks, Mobile Bookmarks.
-        assertLocalEntityCount("Bookmarks", 3);
+        assertEquals("No bookmarks should exist on the client by default.",
+                0, SyncTestUtil.getLocalData(mContext, "Bookmarks").size());
 
+        String title = "Title";
+        String url = "http://chromium.org/";
         mFakeServerHelper.injectBookmarkEntity(
-                "Title", "http://chromium.org", mFakeServerHelper.getBookmarkBarFolderId());
+                title, url, mFakeServerHelper.getBookmarkBarFolderId());
 
         SyncTestUtil.triggerSyncAndWaitForCompletion(mContext);
-        assertLocalEntityCount("Bookmarks", 4);
 
-        // TODO(pvalenzuela): Also verify that the downloaded bookmark matches the one that was
-        // injected. This data should be retrieved from the Sync node browser data.
+        List<Pair<String, JSONObject>> bookmarks = SyncTestUtil.getLocalData(
+                mContext, "Bookmarks");
+        assertEquals("Only the injected bookmark should exist on the client.",
+                1, bookmarks.size());
+        JSONObject bookmark = bookmarks.get(0).second;
+        assertEquals("The wrong title was found for the bookmark.", title,
+                bookmark.getString("title"));
+        assertEquals("The wrong URL was found for the bookmark.", url, bookmark.getString("url"));
     }
 
     private static ContentViewCore getContentViewCore(ChromeShellActivity activity) {

chrome/browser/sync/profile_sync_service_android.cc

@@ -68,6 +68,26 @@ enum ModelTypeSelection {
   AUTOFILL_WALLET = 1 << 15,
 };
 
+// Native callback for the JNI GetAllNodes method. When
+// ProfileSyncService::GetAllNodes completes, this method is called and the
+// results are sent to the Java callback.
+void NativeGetAllNodesCallback(
+    const base::android::ScopedJavaGlobalRef<jobject>& callback,
+    scoped_ptr<base::ListValue> result) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  std::string json_string;
+  if (!result.get() || !base::JSONWriter::Write(*result, &json_string)) {
+    DVLOG(1) << "Writing as JSON failed. Passing empty string to Java code.";
+    json_string = std::string();
+  }
+
+  ScopedJavaLocalRef<jstring> java_json_string =
+      ConvertUTF8ToJavaString(env, json_string);
+  Java_ProfileSyncService_onGetAllNodesResult(env,
+                                              callback.obj(),
+                                              java_json_string.obj());
+}
+
 }  // namespace
 
 ProfileSyncServiceAndroid::ProfileSyncServiceAndroid(JNIEnv* env, jobject obj)
@@ -188,6 +208,17 @@ ScopedJavaLocalRef<jstring> ProfileSyncServiceAndroid::QuerySyncStatusSummary(
   return ConvertUTF8ToJavaString(env, status);
 }
 
+void ProfileSyncServiceAndroid::GetAllNodes(JNIEnv* env,
+                                            jobject obj,
+                                            jobject callback) {
+  base::android::ScopedJavaGlobalRef<jobject> java_callback;
+  java_callback.Reset(env, callback);
+
+  base::Callback<void(scoped_ptr<base::ListValue>)> native_callback =
+      base::Bind(&NativeGetAllNodesCallback, java_callback);
+  sync_service_->GetAllNodes(native_callback);
+}
+
 jboolean ProfileSyncServiceAndroid::SetSyncSessionsId(
     JNIEnv* env, jobject obj, jstring tag) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);

chrome/browser/sync/profile_sync_service_android.h

@@ -53,6 +53,10 @@ class ProfileSyncServiceAndroid : public sync_driver::SyncServiceObserver {
   base::android::ScopedJavaLocalRef<jstring> QuerySyncStatusSummary(
       JNIEnv* env, jobject obj);
 
+  // Retrieves all Sync data as JSON. This method is asynchronous; all data is
+  // passed to |callback| upon completion.
+  void GetAllNodes(JNIEnv* env, jobject obj, jobject callback);
+
   // Called from Java early during startup to ensure we use the correct
   // unique machine tag in session sync. Returns true if the machine tag was
   // succesfully set.

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/sync/SyncTestUtil.java

@@ -30,7 +30,10 @@ import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.concurrent.Callable;
@@ -77,28 +80,6 @@ public final class SyncTestUtil {
                 second.toLowerCase(Locale.US).trim());
     }
 
-    /**
-     * Creates a {@link Map} containing the counts of each entity by model type.
-     */
-    private static Map<String, Integer> createModelTypeCount(String rawJson) throws JSONException {
-        Map<String, Integer> modelTypeCount = new HashMap<String, Integer>();
-        JSONObject aboutInfo = new JSONObject(rawJson);
-
-        JSONArray typeStatusArray = aboutInfo.getJSONArray("type_status");
-        for (int i = 0; i < typeStatusArray.length(); i++) {
-            JSONObject typeInfo = typeStatusArray.getJSONObject(i);
-            String name = typeInfo.getString("name");
-            try {
-                int total = typeInfo.getInt("num_entries");
-                modelTypeCount.put(name, total);
-            } catch (JSONException e) {
-                // This is the header entry which does not have a valid count. Don't include it in
-                // the map.
-            }
-        }
-        return modelTypeCount;
-    }
-
     /**
      * Parses raw JSON into a map with keys Pair<String, String>. The first string in each Pair
      * corresponds to the title under which a given stat_name/stat_value is situated, and the second
@@ -368,6 +349,102 @@ public final class SyncTestUtil {
         verifySignedInWithAccount(context, account);
     }
 
+    /**
+     * Retrieves the local Sync data as a JSONArray via ProfileSyncService.
+     *
+     * This method blocks until the data is available or until it times out.
+     */
+    private static JSONArray getAllNodesAsJsonArray(final Context context) throws JSONException {
+        final Semaphore semaphore = new Semaphore(0);
+        final ProfileSyncService.GetAllNodesCallback callback =
+                new ProfileSyncService.GetAllNodesCallback() {
+                    @Override
+                    public void onResult(String nodesString) {
+                        super.onResult(nodesString);
+                        semaphore.release();
+                    }
+        };
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                ProfileSyncService.get(context).getAllNodes(callback);
+            }
+        });
+
+        try {
+            Assert.assertTrue("Semaphore should have been released.",
+                    semaphore.tryAcquire(UI_TIMEOUT_MS, TimeUnit.MILLISECONDS));
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+
+        return callback.getNodesAsJsonArray();
+    }
+
+
+    /**
+     * Extracts datatype-specific information from the given JSONObject. The returned JSONObject
+     * contains the same data as a specifics protocol buffer (e.g., TypedUrlSpecifics).
+     */
+    private static JSONObject extractSpecifics(JSONObject node) throws JSONException {
+        JSONObject specifics = node.getJSONObject("SPECIFICS");
+        // The key name here is type-specific (e.g., "typed_url" for Typed URLs), so we
+        // can't hard code a value.
+        Iterator<String> keysIterator = specifics.keys();
+        String key = null;
+        if (!keysIterator.hasNext()) {
+            throw new JSONException("Specifics object has 0 keys.");
+        }
+        key = keysIterator.next();
+
+        if (keysIterator.hasNext()) {
+            throw new JSONException("Specifics object has more than 1 key.");
+        }
+        return specifics.getJSONObject(key);
+    }
+
+    /**
+     * Returns the local Sync data present for a single datatype.
+     *
+     * For each data entity, a Pair is returned. The first piece of data is the entity's server ID.
+     * This is useful for activities like deleting an entity on the server. The second piece of data
+     * is a JSONObject representing the datatype-specific information for the entity. This data is
+     * the same as the data stored in a specifics protocol buffer (e.g., TypedUrlSpecifics).
+     *
+     * @param context the Context used to retreive the correct ProfileSyncService
+     * @param typeString a String representing a specific datatype.
+     *
+     * TODO(pvalenzuela): Replace typeString with the native ModelType enum or something else
+     * that will avoid callers needing to specify the native string version.
+     *
+     * @return a List of Pair<String, JSONObject> representing the local Sync data
+     */
+    public static List<Pair<String, JSONObject>> getLocalData(
+            Context context, String typeString) throws JSONException {
+        JSONArray localData = getAllNodesAsJsonArray(context);
+        JSONArray datatypeNodes = new JSONArray();
+        for (int i = 0; i < localData.length(); i++) {
+            JSONObject datatypeObject = localData.getJSONObject(i);
+            if (datatypeObject.getString("type").equals(typeString)) {
+                datatypeNodes = datatypeObject.getJSONArray("nodes");
+                break;
+            }
+        }
+
+        List<Pair<String, JSONObject>> localDataForDatatype =
+                new ArrayList<Pair<String, JSONObject>>(datatypeNodes.length());
+        for (int i = 0; i < datatypeNodes.length(); i++) {
+            JSONObject entity = datatypeNodes.getJSONObject(i);
+            if (!entity.getString("UNIQUE_SERVER_TAG").isEmpty()) {
+                // Ignore permanent items (e.g., root datatype folders).
+                continue;
+            }
+            localDataForDatatype.add(Pair.create(entity.getString("ID"), extractSpecifics(entity)));
+        }
+        return localDataForDatatype;
+    }
+
     /**
      * Retrieves the sync internals information which is the basis for chrome://sync-internals and
      * makes the result available in {@link AboutSyncInfoGetter#getAboutInfo()}.
@@ -378,12 +455,10 @@ public final class SyncTestUtil {
         private static final String TAG = "AboutSyncInfoGetter";
         final Context mContext;
         Map<Pair<String, String>, String> mAboutInfo;
-        Map<String, Integer> mModelTypeCount;
 
         public AboutSyncInfoGetter(Context context) {
             mContext = context.getApplicationContext();
             mAboutInfo = new HashMap<Pair<String, String>, String>();
-            mModelTypeCount = new HashMap<String, Integer>();
         }
 
         @Override
@@ -391,7 +466,6 @@ public final class SyncTestUtil {
             String info = ProfileSyncService.get(mContext).getSyncInternalsInfoForTest();
             try {
                 mAboutInfo = getAboutInfoStats(info);
-                mModelTypeCount = createModelTypeCount(info);
             } catch (JSONException e) {
                 Log.w(TAG, "Unable to parse JSON message: " + info, e);
             }
@@ -400,10 +474,6 @@ public final class SyncTestUtil {
         public Map<Pair<String, String>, String> getAboutInfo() {
             return mAboutInfo;
         }
-
-        public Map<String, Integer> getModelTypeCount() {
-            return mModelTypeCount;
-        }
     }
 
     /**