Remove direct Criteria usage from sync tests.

BUG=1071247

Change-Id: I145041673b9e9779d2a342178cf26146da12cc41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2156108Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760515}

chrome/android/javatests/src/org/chromium/chrome/browser/sync/AutofillTest.java

@@ -18,7 +18,6 @@ import org.junit.runner.RunWith;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
-import org.chromium.chrome.browser.sync.SyncTestRule.DataCriteria;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
 import org.chromium.components.sync.ModelType;
@@ -73,13 +72,6 @@ public class AutofillTest {
         }
     }
 
-    private abstract class ClientAutofillCriteria extends DataCriteria<Autofill> {
-        @Override
-        public List<Autofill> getData() throws Exception {
-            return getClientAutofillProfiles();
-        }
-    }
-
     @Before
     public void setUp() throws Exception {
         mSyncTestRule.setUpTestAccountAndSignIn();
@@ -123,11 +115,12 @@ public class AutofillTest {
         mSyncTestRule.getFakeServerHelper().modifyEntitySpecifics(
                 autofill.id, getServerAutofillProfile(STREET, MODIFIED_CITY, STATE, ZIP));
         SyncTestUtil.triggerSync();
-        mSyncTestRule.pollInstrumentationThread(new ClientAutofillCriteria() {
-            @Override
-            public boolean isSatisfied(List<Autofill> autofills) {
-                Autofill modifiedAutofill = autofills.get(0);
-                return modifiedAutofill.city.equals(MODIFIED_CITY);
+        mSyncTestRule.pollInstrumentationThread(() -> {
+            try {
+                Autofill modifiedAutofill = getClientAutofillProfiles().get(0);
+                Assert.assertEquals(MODIFIED_CITY, modifiedAutofill.city);
+            } catch (JSONException ex) {
+                throw new RuntimeException(ex);
             }
         });
     }

chrome/android/javatests/src/org/chromium/chrome/browser/sync/BookmarksTest.java

@@ -20,7 +20,6 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
 import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.chrome.browser.sync.SyncTestRule.DataCriteria;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
 import org.chromium.components.bookmarks.BookmarkId;
@@ -33,6 +32,7 @@ import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
 
 /**
  * Test suite for the bookmarks sync data type.
@@ -73,20 +73,6 @@ public class BookmarksTest {
         }
     }
 
-    private abstract class ClientBookmarksCriteria extends DataCriteria<Bookmark> {
-        @Override
-        public List<Bookmark> getData() throws Exception {
-            return getClientBookmarks();
-        }
-    }
-
-    private abstract class ServerBookmarksCriteria extends DataCriteria<Bookmark> {
-        @Override
-        public List<Bookmark> getData() throws Exception {
-            return getServerBookmarks();
-        }
-    }
-
     @Before
     public void setUp() throws Exception {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
@@ -134,12 +120,9 @@ public class BookmarksTest {
         Bookmark bookmark = getClientBookmarks().get(0);
         modifyServerBookmark(bookmark.id, MODIFIED_TITLE, URL);
         SyncTestUtil.triggerSync();
-        mSyncTestRule.pollInstrumentationThread(new ClientBookmarksCriteria() {
-            @Override
-            public boolean isSatisfied(List<Bookmark> bookmarks) {
-                Bookmark modifiedBookmark = bookmarks.get(0);
-                return modifiedBookmark.title.equals(MODIFIED_TITLE);
-            }
+        mSyncTestRule.pollInstrumentationThread(() -> {
+            Bookmark modifiedBookmark = getClientBookmarks().get(0);
+            Assert.assertEquals(MODIFIED_TITLE, modifiedBookmark.title);
         });
     }
 
@@ -195,13 +178,11 @@ public class BookmarksTest {
         mSyncTestRule.getFakeServerHelper().modifyBookmarkEntity(
                 bookmark.id, TITLE, URL, folder.id);
         SyncTestUtil.triggerSync();
-        mSyncTestRule.pollInstrumentationThread(new ClientBookmarksCriteria() {
-            @Override
-            public boolean isSatisfied(List<Bookmark> bookmarks) {
-                Bookmark modifiedBookmark = bookmarks.get(bookmarks.get(0).isFolder() ? 1 : 0);
-                // The "s" is prepended because the server adds one to the parentId.
-                return modifiedBookmark.parentId.equals("s" + folder.id);
-            }
+        mSyncTestRule.pollInstrumentationThread(() -> {
+            List<Bookmark> bookmarks = getClientBookmarks();
+            Bookmark modifiedBookmark = bookmarks.get(bookmarks.get(0).isFolder() ? 1 : 0);
+            // The "s" is prepended because the server adds one to the parentId.
+            Assert.assertEquals("s" + folder.id, modifiedBookmark.parentId);
         });
     }
 
@@ -238,12 +219,10 @@ public class BookmarksTest {
         modifyServerBookmarkFolder(folder.id, MODIFIED_TITLE);
         SyncTestUtil.triggerSync();
 
-        mSyncTestRule.pollInstrumentationThread(new ClientBookmarksCriteria() {
-            @Override
-            public boolean isSatisfied(List<Bookmark> bookmarks) {
-                Bookmark modifiedFolder = bookmarks.get(0);
-                return modifiedFolder.isFolder() && modifiedFolder.title.equals(MODIFIED_TITLE);
-            }
+        mSyncTestRule.pollInstrumentationThread(() -> {
+            Bookmark modifiedFolder = getClientBookmarks().get(0);
+            Assert.assertTrue(modifiedFolder.isFolder());
+            Assert.assertEquals(MODIFIED_TITLE, modifiedFolder.title);
         });
     }
 
@@ -331,12 +310,11 @@ public class BookmarksTest {
         // Move on client, sync, and verify the move on the server.
         moveClientBookmark(bookmarkId, folderId);
         SyncTestUtil.triggerSync();
-        mSyncTestRule.pollInstrumentationThread(new ServerBookmarksCriteria() {
-            @Override
-            public boolean isSatisfied(List<Bookmark> bookmarks) {
-                Bookmark modifiedBookmark = bookmarks.get(bookmarks.get(0).isFolder() ? 1 : 0);
-                return modifiedBookmark.parentId.equals(folder.id);
-            }
+        mSyncTestRule.pollInstrumentationThread(() -> {
+            List<Bookmark> serverBookmarks = getServerBookmarks();
+            Bookmark modifiedBookmark =
+                    serverBookmarks.get(serverBookmarks.get(0).isFolder() ? 1 : 0);
+            Assert.assertEquals(folder.id, modifiedBookmark.parentId);
         });
     }
 
@@ -462,31 +440,42 @@ public class BookmarksTest {
                 () -> { mBookmarkBridge.moveBookmark(id, newParentId, 0 /* new index */); });
     }
 
-    private List<Bookmark> getClientBookmarks() throws JSONException {
-        List<Pair<String, JSONObject>> rawBookmarks =
-                SyncTestUtil.getLocalData(mSyncTestRule.getTargetContext(), BOOKMARKS_TYPE_STRING);
-        List<Bookmark> bookmarks = new ArrayList<Bookmark>(rawBookmarks.size());
-        for (Pair<String, JSONObject> rawBookmark : rawBookmarks) {
-            String id = rawBookmark.first;
-            JSONObject json = rawBookmark.second;
-            bookmarks.add(new Bookmark(id, json.getString("legacy_canonicalized_title"),
-                    json.optString("url", null), json.getString("parent_id")));
+    private List<Bookmark> getClientBookmarks() {
+        try {
+            List<Pair<String, JSONObject>> rawBookmarks = SyncTestUtil.getLocalData(
+                    mSyncTestRule.getTargetContext(), BOOKMARKS_TYPE_STRING);
+            List<Bookmark> bookmarks = new ArrayList<Bookmark>(rawBookmarks.size());
+            for (Pair<String, JSONObject> rawBookmark : rawBookmarks) {
+                String id = rawBookmark.first;
+                JSONObject json = rawBookmark.second;
+                bookmarks.add(new Bookmark(id, json.getString("legacy_canonicalized_title"),
+                        json.optString("url", null), json.getString("parent_id")));
+            }
+            return bookmarks;
+        } catch (JSONException ex) {
+            Assert.fail(ex.toString());
+            return null;
         }
-        return bookmarks;
-    }
-
-    private List<Bookmark> getServerBookmarks() throws Exception {
-        List<SyncEntity> entities =
-                mSyncTestRule.getFakeServerHelper().getSyncEntitiesByModelType(ModelType.BOOKMARKS);
-        List<Bookmark> bookmarks = new ArrayList<Bookmark>(entities.size());
-        for (SyncEntity entity : entities) {
-            String id = entity.getIdString();
-            String parentId = entity.getParentIdString();
-            BookmarkSpecifics specifics = entity.getSpecifics().getBookmark();
-            bookmarks.add(new Bookmark(id, specifics.getLegacyCanonicalizedTitle(),
-                    entity.getFolder() ? null : specifics.getUrl(), parentId));
+    }
+
+    private List<Bookmark> getServerBookmarks() {
+        try {
+            List<SyncEntity> entities =
+                    mSyncTestRule.getFakeServerHelper().getSyncEntitiesByModelType(
+                            ModelType.BOOKMARKS);
+            List<Bookmark> bookmarks = new ArrayList<Bookmark>(entities.size());
+            for (SyncEntity entity : entities) {
+                String id = entity.getIdString();
+                String parentId = entity.getParentIdString();
+                BookmarkSpecifics specifics = entity.getSpecifics().getBookmark();
+                bookmarks.add(new Bookmark(id, specifics.getLegacyCanonicalizedTitle(),
+                        entity.getFolder() ? null : specifics.getUrl(), parentId));
+            }
+            return bookmarks;
+        } catch (ExecutionException ex) {
+            Assert.fail(ex.toString());
+            return null;
         }
-        return bookmarks;
     }
 
     private void assertClientBookmarkCount(int count) throws JSONException {
@@ -513,17 +502,10 @@ public class BookmarksTest {
     }
 
     private void waitForServerBookmarkCountWithName(final int count, final String name) {
-        mSyncTestRule.pollInstrumentationThread(new Criteria(
-                "Expected " + count + " remote bookmarks with name " + name + ".") {
-            @Override
-            public boolean isSatisfied() {
-                try {
-                    return mSyncTestRule.getFakeServerHelper().verifyEntityCountByTypeAndName(
-                            count, ModelType.BOOKMARKS, name);
-                } catch (Exception e) {
-                    throw new RuntimeException(e);
-                }
-            }
-        });
+        mSyncTestRule.pollInstrumentationThread(
+                ()
+                        -> mSyncTestRule.getFakeServerHelper().verifyEntityCountByTypeAndName(
+                                count, ModelType.BOOKMARKS, name),
+                "Expected " + count + " remote bookmarks with name " + name + ".");
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/sync/FirstRunTest.java

@@ -36,7 +36,6 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.ActivityUtils;
 import org.chromium.chrome.test.util.browser.signin.SigninTestUtil;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
-import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
@@ -209,12 +208,8 @@ public class FirstRunTest {
             processFirstRunOnUiThread();
         }
 
-        CriteriaHelper.pollInstrumentationThread(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                return FirstRunSignInProcessor.getFirstRunFlowSignInComplete();
-            }
-        });
+        CriteriaHelper.pollInstrumentationThread(
+                () -> FirstRunSignInProcessor.getFirstRunFlowSignInComplete());
         return settingsActivity;
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/sync/OpenTabsTest.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.sync;
 import android.support.test.filters.LargeTest;
 import android.util.Pair;
 
+import org.hamcrest.Matchers;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -268,31 +269,21 @@ public class OpenTabsTest {
 
     private void waitForServerTabs(final String... urls) {
         mSyncTestRule.pollInstrumentationThread(
-                new Criteria("Expected server open tabs: " + Arrays.toString(urls)) {
-                    @Override
-                    public boolean isSatisfied() {
-                        try {
-                            return mSyncTestRule.getFakeServerHelper().verifySessions(urls);
-                        } catch (Exception e) {
-                            throw new RuntimeException(e);
-                        }
-                    }
-                });
+                ()
+                        -> mSyncTestRule.getFakeServerHelper().verifySessions(urls),
+                "Expected server open tabs: " + Arrays.toString(urls));
     }
 
     private String getClientName() throws Exception {
-        mSyncTestRule.pollInstrumentationThread(new Criteria(
-                "Expected at least one tab entity to exist.") {
-            @Override
-            public boolean isSatisfied() {
-                try {
-                    return SyncTestUtil
-                                   .getLocalData(mSyncTestRule.getTargetContext(), OPEN_TABS_TYPE)
-                                   .size()
-                            > 0;
-                } catch (JSONException e) {
-                    return false;
-                }
+        mSyncTestRule.pollInstrumentationThread(() -> {
+            try {
+                int size =
+                        SyncTestUtil.getLocalData(mSyncTestRule.getTargetContext(), OPEN_TABS_TYPE)
+                                .size();
+                Assert.assertThat("Expected at least one tab entity to exist.", size,
+                        Matchers.greaterThan(0));
+            } catch (JSONException ex) {
+                Assert.fail(ex.toString());
             }
         });
         List<Pair<String, JSONObject>> tabEntities =

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

@@ -80,15 +80,10 @@ public class SyncTest {
     public void testStopAndClear() {
         mSyncTestRule.setUpTestAccountAndSignIn();
         CriteriaHelper.pollUiThread(
-                new Criteria("Timed out checking that hasPrimaryAccount() == true") {
-                    @Override
-                    public boolean isSatisfied() {
-                        return IdentityServicesProvider.get()
-                                .getIdentityManager()
-                                .hasPrimaryAccount();
-                    }
-                },
-                SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
+                ()
+                        -> IdentityServicesProvider.get().getIdentityManager().hasPrimaryAccount(),
+                "Timed out checking that hasPrimaryAccount() == true", SyncTestUtil.TIMEOUT_MS,
+                SyncTestUtil.INTERVAL_MS);
 
         mSyncTestRule.clearServerData();
 
@@ -96,15 +91,10 @@ public class SyncTest {
         Assert.assertNull(SigninTestUtil.getCurrentAccount());
         Assert.assertFalse(SyncTestUtil.isSyncRequested());
         CriteriaHelper.pollUiThread(
-                new Criteria("Timed out checking that hasPrimaryAccount() == false") {
-                    @Override
-                    public boolean isSatisfied() {
-                        return !IdentityServicesProvider.get()
-                                        .getIdentityManager()
-                                        .hasPrimaryAccount();
-                    }
-                },
-                SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
+                ()
+                        -> !IdentityServicesProvider.get().getIdentityManager().hasPrimaryAccount(),
+                "Timed out checking that hasPrimaryAccount() == false", SyncTestUtil.TIMEOUT_MS,
+                SyncTestUtil.INTERVAL_MS);
     }
 
     /*
@@ -138,12 +128,8 @@ public class SyncTest {
             SigninHelper.get().validateAccountSettings(true);
         });
 
-        CriteriaHelper.pollInstrumentationThread(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                return newAccount.equals(SigninTestUtil.getCurrentAccount());
-            }
-        });
+        CriteriaHelper.pollInstrumentationThread(
+                Criteria.equals(newAccount, SigninTestUtil::getCurrentAccount));
         SyncTestUtil.waitForSyncActive();
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTestRule.java

@@ -52,6 +52,7 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 
@@ -69,25 +70,6 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
                     ModelType.PREFERENCES, ModelType.PROXY_TABS, ModelType.TYPED_URLS,
             }));
 
-    public abstract static class DataCriteria<T> extends Criteria {
-        public DataCriteria() {
-            super("Sync data criteria not met.");
-        }
-
-        public abstract boolean isSatisfied(List<T> data);
-
-        public abstract List<T> getData() throws Exception;
-
-        @Override
-        public boolean isSatisfied() {
-            try {
-                return isSatisfied(getData());
-            } catch (Exception e) {
-                throw new RuntimeException(e);
-            }
-        }
-    }
-
     /**
      * Simple activity that mimics a trusted vault key retrieval flow that succeeds immediately.
      */
@@ -263,12 +245,9 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
     public void clearServerData() {
         mFakeServerHelper.clearServerData();
         SyncTestUtil.triggerSync();
-        CriteriaHelper.pollUiThread(new Criteria("Timed out waiting for sync to stop.") {
-            @Override
-            public boolean isSatisfied() {
-                return !ProfileSyncService.get().isSyncRequested();
-            }
-        }, SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
+        CriteriaHelper.pollUiThread(
+                Criteria.equals(false, () -> ProfileSyncService.get().isSyncRequested()),
+                SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
     }
 
     /*
@@ -309,11 +288,22 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
         });
     }
 
+    @Deprecated // TODO(tedchoc): Remove this method once Criteria.equals returns a Runnable.
     public void pollInstrumentationThread(Criteria criteria) {
         CriteriaHelper.pollInstrumentationThread(
                 criteria, SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
     }
 
+    public void pollInstrumentationThread(Runnable criteria) {
+        CriteriaHelper.pollInstrumentationThread(
+                criteria, SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
+    }
+
+    public void pollInstrumentationThread(Callable<Boolean> criteria, String reason) {
+        CriteriaHelper.pollInstrumentationThread(
+                criteria, reason, SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
+    }
+
     @Override
     public Statement apply(final Statement statement, final Description desc) {
         final Statement base = super.apply(new Statement() {

chrome/android/javatests/src/org/chromium/chrome/browser/sync/TypedUrlsTest.java

@@ -181,18 +181,12 @@ public class TypedUrlsTest {
 
     private void waitForServerTypedUrlCountWithName(final int count, final String name) {
         CriteriaHelper.pollInstrumentationThread(
-                new Criteria("Expected " + count + " server typed URLs with name " + name + ".") {
-                    @Override
-                    public boolean isSatisfied() {
-                        try {
-                            return mSyncTestRule.getFakeServerHelper()
-                                    .verifyEntityCountByTypeAndName(
-                                            count, ModelType.TYPED_URLS, name);
-                        } catch (Exception e) {
-                            throw new RuntimeException(e);
-                        }
-                    }
+                ()
+                        -> {
+                    return mSyncTestRule.getFakeServerHelper().verifyEntityCountByTypeAndName(
+                            count, ModelType.TYPED_URLS, name);
                 },
+                "Expected " + count + " server typed URLs with name " + name + ".",
                 SyncTestUtil.TIMEOUT_MS, SyncTestUtil.INTERVAL_MS);
     }