Bug 1046709 - Part 4: Sync changes r=nalexander,rnewman draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Wed, 13 Apr 2016 16:33:36 -0700
changeset 353766 f0a86673ab90ec2c2d8b7d31e168ff97da565428
parent 353765 d1d498db33308be8f7273d47dc0c890ed6c7a7f8
child 353767 df749bc66779f371907f63fe22b5bdf835fee41a
push id15929
push usergkruglov@mozilla.com
push dateTue, 19 Apr 2016 19:45:51 +0000
reviewersnalexander, rnewman
bugs1046709
milestone48.0a1
Bug 1046709 - Part 4: Sync changes r=nalexander,rnewman - insert/merge visits on "sync down" - ensure to attach visits on "sync up" - tests MozReview-Commit-ID: 4OmyQS5JSm7
mobile/android/base/android-services.mozbuild
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/VisitsHelper.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/android/VisitsHelperTest.java
--- a/mobile/android/base/android-services.mozbuild
+++ b/mobile/android/base/android-services.mozbuild
@@ -938,32 +938,32 @@ sync_java_files = [TOPSRCDIR + '/mobile/
     'sync/NonObjectJSONException.java',
     'sync/NullClusterURLException.java',
     'sync/PersistedMetaGlobal.java',
     'sync/PrefsBackoffHandler.java',
     'sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java',
     'sync/repositories/android/AndroidBrowserBookmarksRepository.java',
     'sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java',
     'sync/repositories/android/AndroidBrowserHistoryDataAccessor.java',
-    'sync/repositories/android/AndroidBrowserHistoryDataExtender.java',
     'sync/repositories/android/AndroidBrowserHistoryRepository.java',
     'sync/repositories/android/AndroidBrowserHistoryRepositorySession.java',
     'sync/repositories/android/AndroidBrowserRepository.java',
     'sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java',
     'sync/repositories/android/AndroidBrowserRepositorySession.java',
     'sync/repositories/android/BookmarksDeletionManager.java',
     'sync/repositories/android/BookmarksInsertionManager.java',
     'sync/repositories/android/BrowserContractHelpers.java',
     'sync/repositories/android/CachedSQLiteOpenHelper.java',
     'sync/repositories/android/ClientsDatabase.java',
     'sync/repositories/android/ClientsDatabaseAccessor.java',
     'sync/repositories/android/FennecTabsRepository.java',
     'sync/repositories/android/FormHistoryRepositorySession.java',
     'sync/repositories/android/PasswordsRepositorySession.java',
     'sync/repositories/android/RepoUtils.java',
+    'sync/repositories/android/VisitsHelper.java',
     'sync/repositories/BookmarkNeedsReparentingException.java',
     'sync/repositories/BookmarksRepository.java',
     'sync/repositories/ConstrainedServer11Repository.java',
     'sync/repositories/delegates/DeferrableRepositorySessionCreationDelegate.java',
     'sync/repositories/delegates/DeferredRepositorySessionBeginDelegate.java',
     'sync/repositories/delegates/DeferredRepositorySessionFetchRecordsDelegate.java',
     'sync/repositories/delegates/DeferredRepositorySessionFinishDelegate.java',
     'sync/repositories/delegates/DeferredRepositorySessionStoreDelegate.java',
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java
@@ -1,42 +1,33 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync.repositories.android;
 
 import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Map;
 
 import org.json.simple.JSONArray;
 import org.json.simple.JSONObject;
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.sync.repositories.NullCursorException;
 import org.mozilla.gecko.sync.repositories.domain.HistoryRecord;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
 import android.content.ContentValues;
 import android.content.Context;
 import android.net.Uri;
 
 public class AndroidBrowserHistoryDataAccessor extends
     AndroidBrowserRepositoryDataAccessor {
 
-  private final AndroidBrowserHistoryDataExtender dataExtender;
-
   public AndroidBrowserHistoryDataAccessor(Context context) {
     super(context);
-    dataExtender = new AndroidBrowserHistoryDataExtender(context);
-  }
-
-  public AndroidBrowserHistoryDataExtender getHistoryDataExtender() {
-    return dataExtender;
   }
 
   @Override
   protected Uri getUri() {
     return BrowserContractHelpers.HISTORY_CONTENT_URI;
   }
 
   @Override
@@ -46,73 +37,81 @@ public class AndroidBrowserHistoryDataAc
     cv.put(BrowserContract.History.GUID, rec.guid);
     cv.put(BrowserContract.History.TITLE, rec.title);
     cv.put(BrowserContract.History.URL, rec.histURI);
     if (rec.visits != null) {
       JSONArray visits = rec.visits;
       long mostRecent = 0;
       for (int i = 0; i < visits.size(); i++) {
         JSONObject visit = (JSONObject) visits.get(i);
-        long visitDate = (Long) visit
-            .get(AndroidBrowserHistoryRepositorySession.KEY_DATE);
+        long visitDate = (Long) visit.get(VisitsHelper.SYNC_DATE_KEY);
         if (visitDate > mostRecent) {
           mostRecent = visitDate;
         }
       }
-      // Fennec stores milliseconds. The rest of Sync works in microseconds.
+      // Fennec stores history timestamps in milliseconds, and visit timestamps in microseconds.
+      // The rest of Sync works in microseconds. This is the conversion point for records coming form Sync.
       cv.put(BrowserContract.History.DATE_LAST_VISITED, mostRecent / 1000);
       cv.put(BrowserContract.History.VISITS, Long.toString(visits.size()));
     }
     return cv;
   }
 
   @Override
   protected String[] getAllColumns() {
     return BrowserContractHelpers.HistoryColumns;
   }
 
   @Override
   public Uri insert(Record record) {
     HistoryRecord rec = (HistoryRecord) record;
+
+    Logger.debug(LOG_TAG, "Storing record " + record.guid);
+    Uri newRecordUri = super.insert(record);
+
     Logger.debug(LOG_TAG, "Storing visits for " + record.guid);
-    dataExtender.store(record.guid, rec.visits);
-    Logger.debug(LOG_TAG, "Storing record " + record.guid);
-    return super.insert(record);
+    context.getContentResolver().bulkInsert(
+            BrowserContract.Visits.CONTENT_URI,
+            VisitsHelper.getVisitsContentValues(rec.guid, rec.visits)
+    );
+
+    return newRecordUri;
   }
 
+  /**
+   * Given oldGUID, first updates corresponding history record with new values (super operation),
+   * and then inserts visits from the new record.
+   * Existing visits from the old record are updated on database level to point to new GUID if necessary.
+   *
+   * @param oldGUID GUID of old <code>HistoryRecord</code>
+   * @param newRecord new <code>HistoryRecord</code> to replace old one with, and insert visits from
+   */
   @Override
   public void update(String oldGUID, Record newRecord) {
+    // First, update existing history records with new values. This might involve changing history GUID,
+    // and thanks to ON UPDATE CASCADE clause on Visits.HISTORY_GUID foreign key, visits will be "ported over"
+    // to the new GUID.
+    super.update(oldGUID, newRecord);
+
+    // Now we need to insert any visits from the new record
     HistoryRecord rec = (HistoryRecord) newRecord;
     String newGUID = newRecord.guid;
     Logger.debug(LOG_TAG, "Storing visits for " + newGUID + ", replacing " + oldGUID);
-    dataExtender.delete(oldGUID);
-    dataExtender.store(newGUID, rec.visits);
-    super.update(oldGUID, newRecord);
+
+    context.getContentResolver().bulkInsert(
+            BrowserContract.Visits.CONTENT_URI,
+            VisitsHelper.getVisitsContentValues(newGUID, rec.visits)
+    );
   }
 
-  @Override
-  public int purgeGuid(String guid) {
-    Logger.debug(LOG_TAG, "Purging record with " + guid);
-    dataExtender.delete(guid);
-    return super.purgeGuid(guid);
-  }
-
-  public void closeExtender() {
-    dataExtender.close();
-  }
-
-  public static String[] GUID_AND_ID = new String[] { BrowserContract.History.GUID, BrowserContract.History._ID };
-
   /**
    * Insert records.
    * <p>
    * This inserts all the records (using <code>ContentProvider.bulkInsert</code>),
-   * then inserts all the visit information (using the data extender's
-   * <code>bulkInsert</code>, which internally uses a single database
-   * transaction).
+   * then inserts all the visit information (also using <code>ContentProvider.bulkInsert</code>).
    *
    * @param records
    *          the records to insert.
    * @return
    *          the number of records actually inserted.
    * @throws NullCursorException
    */
   public int bulkInsert(ArrayList<HistoryRecord> records) throws NullCursorException {
@@ -135,13 +134,22 @@ public class AndroidBrowserHistoryDataAc
     int inserted = context.getContentResolver().bulkInsert(getUri(), cvs);
     if (inserted == size) {
       Logger.debug(LOG_TAG, "Inserted " + inserted + " records, as expected.");
     } else {
       Logger.debug(LOG_TAG, "Inserted " +
                    inserted + " records but expected " +
                    size     + " records; continuing to update visits.");
     }
-    // Then update the history visits.
-    dataExtender.bulkInsert(records);
+
+    for (Record record : records) {
+      HistoryRecord rec = (HistoryRecord) record;
+      if (rec.visits != null && rec.visits.size() != 0) {
+        context.getContentResolver().bulkInsert(
+                BrowserContract.Visits.CONTENT_URI,
+                VisitsHelper.getVisitsContentValues(rec.guid, rec.visits)
+        );
+      }
+    }
+
     return inserted;
   }
 }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java
@@ -1,45 +1,40 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync.repositories.android;
 
 import java.util.ArrayList;
 
-import org.json.simple.JSONArray;
-import org.json.simple.JSONObject;
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.db.BrowserContract;
-import org.mozilla.gecko.sync.repositories.InactiveSessionException;
 import org.mozilla.gecko.sync.repositories.InvalidSessionTransitionException;
 import org.mozilla.gecko.sync.repositories.NoGuidForIdException;
 import org.mozilla.gecko.sync.repositories.NullCursorException;
 import org.mozilla.gecko.sync.repositories.ParentNotFoundException;
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionBeginDelegate;
-import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFinishDelegate;
 import org.mozilla.gecko.sync.repositories.domain.HistoryRecord;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
+import android.content.ContentProviderClient;
 import android.content.Context;
 import android.database.Cursor;
+import android.os.RemoteException;
 
 public class AndroidBrowserHistoryRepositorySession extends AndroidBrowserRepositorySession {
   public static final String LOG_TAG = "ABHistoryRepoSess";
 
-  public static final String KEY_DATE = "date";
-  public static final String KEY_TYPE = "type";
-  public static final long DEFAULT_VISIT_TYPE = 1;
-
   /**
    * The number of records to queue for insertion before writing to databases.
    */
   public static final int INSERT_RECORD_THRESHOLD = 50;
+  public static final int RECENT_VISITS_LIMIT = 20;
 
   public AndroidBrowserHistoryRepositorySession(Repository repository, Context context) {
     super(repository);
     dbHelper = new AndroidBrowserHistoryDataAccessor(context);
   }
 
   @Override
   public void begin(RepositorySessionBeginDelegate delegate) throws InvalidSessionTransitionException {
@@ -81,84 +76,44 @@ public class AndroidBrowserHistoryReposi
     return !RepoUtils.isValidHistoryURI(r.histURI);
   }
 
   @Override
   protected Record transformRecord(Record record) throws NullCursorException {
     return addVisitsToRecord(record);
   }
 
-  @SuppressWarnings("unchecked")
-  private void addVisit(JSONArray visits, long date, long visitType) {
-    JSONObject visit = new JSONObject();
-    visit.put(KEY_DATE, date);               // Microseconds since epoch.
-    visit.put(KEY_TYPE, visitType);
-    visits.add(visit);
-  }
-
-  private void addVisit(JSONArray visits, long date) {
-    addVisit(visits, date, DEFAULT_VISIT_TYPE);
-  }
-
-  private AndroidBrowserHistoryDataExtender getDataExtender() {
-    return ((AndroidBrowserHistoryDataAccessor) dbHelper).getHistoryDataExtender();
-  }
-
   private Record addVisitsToRecord(Record record) throws NullCursorException {
     Logger.debug(LOG_TAG, "Adding visits for GUID " + record.guid);
-    HistoryRecord hist = (HistoryRecord) record;
-    JSONArray visitsArray = getDataExtender().visitsForGUID(hist.guid);
-    long missingRecords = hist.fennecVisitCount - visitsArray.size();
 
-    // Note that Fennec visit times are milliseconds, and we are working
-    // in microseconds. This is the point at which we translate.
-
-    // Add (missingRecords - 1) fake visits...
-    if (missingRecords > 0) {
-      long fakes = missingRecords - 1;
-      for (int j = 0; j < fakes; j++) {
-        // Set fake visit timestamp to be just previous to
-        // the real one we are about to add.
-        // TODO: make these equidistant?
-        long fakeDate = (hist.fennecDateVisited - (1 + j)) * 1000;
-        addVisit(visitsArray, fakeDate);
-      }
-
-      // ... and the 1 actual record we have.
-      // We still have to fake the visit type: Fennec doesn't track that.
-      addVisit(visitsArray, hist.fennecDateVisited * 1000);
+    // Sync is an object store, so what we attach here will replace what's already present on the Sync servers.
+    // We upload just a recent subset of visits for each history record for space and bandwidth reasons.
+    // We chose 20 to be conservative.  See Bug 1164660 for details.
+    ContentProviderClient visitsClient = dbHelper.context.getContentResolver().acquireContentProviderClient(BrowserContractHelpers.VISITS_CONTENT_URI);
+    if (visitsClient == null) {
+      throw new IllegalStateException("Could not obtain a ContentProviderClient for Visits URI");
     }
 
-    hist.visits = visitsArray;
-    return hist;
+    try {
+      ((HistoryRecord) record).visits = VisitsHelper.getRecentHistoryVisitsForGUID(
+              visitsClient, record.guid, RECENT_VISITS_LIMIT);
+    } catch (RemoteException e) {
+      throw new IllegalStateException("Error while obtaining visits for a record", e);
+    } finally {
+      visitsClient.release();
+    }
+
+    return record;
   }
 
   @Override
   protected Record prepareRecord(Record record) {
     return record;
   }
 
-  @Override
-  public void abort() {
-    if (dbHelper != null) {
-      ((AndroidBrowserHistoryDataAccessor) dbHelper).closeExtender();
-      dbHelper = null;
-    }
-    super.abort();
-  }
-
-  @Override
-  public void finish(final RepositorySessionFinishDelegate delegate) throws InactiveSessionException {
-    if (dbHelper != null) {
-      ((AndroidBrowserHistoryDataAccessor) dbHelper).closeExtender();
-      dbHelper = null;
-    }
-    super.finish(delegate);
-  }
-
   protected final Object recordsBufferMonitor = new Object();
   protected ArrayList<HistoryRecord> recordsBuffer = new ArrayList<HistoryRecord>();
 
   /**
    * Queue record for insertion, possibly flushing the queue.
    * <p>
    * Must be called on <code>storeWorkQueue</code> thread! But this is only
    * called from <code>store</code>, which is called on the queue thread.
new file mode 100644
--- /dev/null
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/VisitsHelper.java
@@ -0,0 +1,130 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.sync.repositories.android;
+
+import android.content.ContentProviderClient;
+import android.content.ContentValues;
+import android.database.Cursor;
+import android.net.Uri;
+import android.os.RemoteException;
+import android.support.annotation.NonNull;
+
+import org.json.simple.JSONArray;
+import org.json.simple.JSONObject;
+import org.mozilla.gecko.db.BrowserContract.Visits;
+
+/**
+ * This class is used by History Sync code (see <code>AndroidBrowserHistoryDataAccessor</code> and <code>AndroidBrowserHistoryRepositorySession</code>,
+ * and provides utility functions for working with history visits. Primarily we're either inserting visits
+ * into local database based on data received from Sync, or we're preparing local visits for upload into Sync.
+ */
+public class VisitsHelper {
+    public static final boolean DEFAULT_IS_LOCAL_VALUE = false;
+    public static final String SYNC_TYPE_KEY = "type";
+    public static final String SYNC_DATE_KEY = "date";
+
+    /**
+     * Returns a list of ContentValues of visits ready for insertion for a provided History GUID.
+     * Visits must have data and type. See <code>getVisitContentValues</code>.
+     *
+     * @param guid History GUID to use when inserting visit records
+     * @param visits <code>JSONArray</code> list of (date, type) tuples for visits
+     * @return visits ready for insertion
+     */
+    public static ContentValues[] getVisitsContentValues(@NonNull String guid, @NonNull JSONArray visits) {
+        final ContentValues[] visitsToStore = new ContentValues[visits.size()];
+        final int visitCount = visits.size();
+
+        if (visitCount == 0) {
+            return visitsToStore;
+        }
+
+        for (int i = 0; i < visitCount; i++) {
+            visitsToStore[i] = getVisitContentValues(
+                    guid, (JSONObject) visits.get(i), DEFAULT_IS_LOCAL_VALUE);
+        }
+        return visitsToStore;
+    }
+
+    /**
+     * Maps up to <code>limit</code> visits for a given history GUID to an array of JSONObjects with "date" and "type" keys
+     *
+     * @param contentClient <code>ContentProviderClient</code> to use for querying Visits table
+     * @param guid History GUID for which to return visits
+     * @param limit Will return at most this number of visits
+     * @return <code>JSONArray</code> of all visits found for given History GUID
+     */
+    public static JSONArray getRecentHistoryVisitsForGUID(@NonNull ContentProviderClient contentClient,
+                                                          @NonNull String guid, int limit) throws RemoteException {
+        final JSONArray visits = new JSONArray();
+
+        final Cursor cursor = contentClient.query(
+                visitsUriWithLimit(limit),
+                new String[] {Visits.VISIT_TYPE, Visits.DATE_VISITED},
+                Visits.HISTORY_GUID + " = ?",
+                new String[] {guid}, null);
+        if (cursor == null) {
+            return visits;
+        }
+        try {
+            if (!cursor.moveToFirst()) {
+                return visits;
+            }
+
+            final int dateVisitedCol = cursor.getColumnIndexOrThrow(Visits.DATE_VISITED);
+            final int visitTypeCol = cursor.getColumnIndexOrThrow(Visits.VISIT_TYPE);
+
+            while (!cursor.isAfterLast()) {
+                insertTupleIntoVisitsUnchecked(visits,
+                        cursor.getInt(visitTypeCol),
+                        cursor.getLong(dateVisitedCol)
+                );
+                cursor.moveToNext();
+            }
+        } finally {
+            cursor.close();
+        }
+
+        return visits;
+    }
+
+    /**
+     * Constructs <code>ContentValues</code> object for a visit based on passed in parameters.
+     *
+     * @param visit <code>JSONObject</code> containing visit type and visit date keys for the visit
+     * @param guid History GUID with with to associate this visit
+     * @param isLocal Whether or not to mark this visit as local
+     * @return <code>ContentValues</code> with all visit values necessary for database insertion
+     * @throws IllegalArgumentException if visit object is missing date or type keys
+     */
+    public static ContentValues getVisitContentValues(@NonNull String guid, @NonNull JSONObject visit, boolean isLocal) {
+        if (!visit.containsKey(SYNC_DATE_KEY) || !visit.containsKey(SYNC_TYPE_KEY)) {
+            throw new IllegalArgumentException("Visit missing required keys");
+        }
+
+        final ContentValues cv = new ContentValues();
+        cv.put(Visits.HISTORY_GUID, guid);
+        cv.put(Visits.IS_LOCAL, isLocal ? 1 : 0);
+        cv.put(Visits.VISIT_TYPE, (Long) visit.get(SYNC_TYPE_KEY));
+        cv.put(Visits.DATE_VISITED, (Long) visit.get(SYNC_DATE_KEY));
+
+        return cv;
+    }
+
+    @SuppressWarnings("unchecked")
+    private static void insertTupleIntoVisitsUnchecked(JSONArray visits, Integer type, Long date) {
+        final JSONObject visit = new JSONObject();
+        visit.put(SYNC_TYPE_KEY, type);
+        visit.put(SYNC_DATE_KEY, date);
+        visits.add(visit);
+    }
+
+    private static Uri visitsUriWithLimit(int limit) {
+        return BrowserContractHelpers.VISITS_CONTENT_URI
+                .buildUpon()
+                .appendQueryParameter("limit", Integer.toString(limit))
+                .build();
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/android/VisitsHelperTest.java
@@ -0,0 +1,139 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+package org.mozilla.gecko.sync.repositories.android;
+
+import android.content.ContentProviderClient;
+import android.content.ContentValues;
+import android.net.Uri;
+
+import junit.framework.Assert;
+
+import org.json.simple.JSONArray;
+import org.json.simple.JSONObject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mozilla.gecko.background.testhelpers.TestRunner;
+import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.db.BrowserProvider;
+import org.robolectric.shadows.ShadowContentResolver;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(TestRunner.class)
+public class VisitsHelperTest {
+    @Test
+    public void testBulkInsertRemoteVisits() throws Exception {
+        JSONArray toInsert = new JSONArray();
+        Assert.assertEquals(0, VisitsHelper.getVisitsContentValues("testGUID", toInsert).length);
+
+        JSONObject visit = new JSONObject();
+        Long date = Long.valueOf(123432552344l);
+        visit.put("date", date);
+        visit.put("type", 2l);
+        toInsert.add(visit);
+
+        JSONObject visit2 = new JSONObject();
+        visit2.put("date", date + 1000);
+        visit2.put("type", 5l);
+        toInsert.add(visit2);
+
+        ContentValues[] cvs = VisitsHelper.getVisitsContentValues("testGUID", toInsert);
+        Assert.assertEquals(2, cvs.length);
+        ContentValues cv1 = cvs[0];
+        ContentValues cv2 = cvs[1];
+        Assert.assertEquals(Integer.valueOf(2), cv1.getAsInteger(BrowserContract.Visits.VISIT_TYPE));
+        Assert.assertEquals(Integer.valueOf(5), cv2.getAsInteger(BrowserContract.Visits.VISIT_TYPE));
+
+        Assert.assertEquals(date, cv1.getAsLong("date"));
+        Assert.assertEquals(Long.valueOf(date + 1000), cv2.getAsLong(BrowserContract.Visits.DATE_VISITED));
+    }
+
+    @Test
+    public void testGetRecentHistoryVisitsForGUID() throws Exception {
+        Uri historyTestUri = testUri(BrowserContract.History.CONTENT_URI);
+        Uri visitsTestUri = testUri(BrowserContract.Visits.CONTENT_URI);
+
+        BrowserProvider provider = new BrowserProvider();
+        provider.onCreate();
+        ShadowContentResolver.registerProvider(BrowserContract.AUTHORITY_URI.toString(), provider);
+
+        final ShadowContentResolver cr = new ShadowContentResolver();
+        ContentProviderClient historyClient = cr.acquireContentProviderClient(BrowserContractHelpers.HISTORY_CONTENT_URI);
+        ContentProviderClient visitsClient = cr.acquireContentProviderClient(BrowserContractHelpers.VISITS_CONTENT_URI);
+
+        ContentValues historyItem = new ContentValues();
+        historyItem.put(BrowserContract.History.URL, "https://www.mozilla.org");
+        historyItem.put(BrowserContract.History.GUID, "testGUID");
+        historyClient.insert(historyTestUri, historyItem);
+
+        Long baseDate = System.currentTimeMillis();
+        for (int i = 0; i < 30; i++) {
+            ContentValues visitItem = new ContentValues();
+            visitItem.put(BrowserContract.Visits.HISTORY_GUID, "testGUID");
+            visitItem.put(BrowserContract.Visits.DATE_VISITED, baseDate - i * 100);
+            visitItem.put(BrowserContract.Visits.VISIT_TYPE, 1);
+            visitItem.put(BrowserContract.Visits.IS_LOCAL, 1);
+            visitsClient.insert(visitsTestUri, visitItem);
+        }
+
+        // test that limit worked, that sorting is correct, and that both date and type are present
+        JSONArray recentVisits = VisitsHelper.getRecentHistoryVisitsForGUID(visitsClient, "testGUID", 10);
+        Assert.assertEquals(10, recentVisits.size());
+        for (int i = 0; i < recentVisits.size(); i++) {
+            JSONObject v = (JSONObject) recentVisits.get(i);
+            Long date = (Long) v.get("date");
+            Integer type = (Integer) v.get("type");
+            Assert.assertEquals(Long.valueOf(baseDate - i * 100), date);
+            Assert.assertEquals(Integer.valueOf(1), type);
+        }
+    }
+
+    @Test
+    public void testGetVisitContentValues() throws Exception {
+        JSONObject visit = new JSONObject();
+        Long date = Long.valueOf(123432552344l);
+        visit.put("date", date);
+        visit.put("type", Long.valueOf(2));
+
+        ContentValues cv = VisitsHelper.getVisitContentValues("testGUID", visit, true);
+        assertTrue(cv.containsKey(BrowserContract.Visits.VISIT_TYPE));
+        assertTrue(cv.containsKey(BrowserContract.Visits.DATE_VISITED));
+        assertTrue(cv.containsKey(BrowserContract.Visits.HISTORY_GUID));
+        assertTrue(cv.containsKey(BrowserContract.Visits.IS_LOCAL));
+        assertEquals(4, cv.size());
+
+        assertEquals(date, cv.getAsLong(BrowserContract.Visits.DATE_VISITED));
+        assertEquals(Long.valueOf(2), cv.getAsLong(BrowserContract.Visits.VISIT_TYPE));
+        assertEquals("testGUID", cv.getAsString(BrowserContract.Visits.HISTORY_GUID));
+        assertEquals(Integer.valueOf(1), cv.getAsInteger(BrowserContract.Visits.IS_LOCAL));
+
+        cv = VisitsHelper.getVisitContentValues("testGUID", visit, false);
+        assertEquals(Integer.valueOf(0), cv.getAsInteger(BrowserContract.Visits.IS_LOCAL));
+
+        try {
+            JSONObject visit2 = new JSONObject();
+            visit.put("date", date);
+            VisitsHelper.getVisitContentValues("testGUID", visit2, false);
+            assertTrue("Must check that visit type key is present", false);
+        } catch (IllegalArgumentException e) {}
+
+        try {
+            JSONObject visit3 = new JSONObject();
+            visit.put("type", Long.valueOf(2));
+            VisitsHelper.getVisitContentValues("testGUID", visit3, false);
+            assertTrue("Must check that visit date key is present", false);
+        } catch (IllegalArgumentException e) {}
+
+        try {
+            JSONObject visit4 = new JSONObject();
+            VisitsHelper.getVisitContentValues("testGUID", visit4, false);
+            assertTrue("Must check that visit type and date keys are present", false);
+        } catch (IllegalArgumentException e) {}
+    }
+
+    private Uri testUri(Uri baseUri) {
+        return baseUri.buildUpon().appendQueryParameter(BrowserContract.PARAM_IS_TEST, "1").build();
+    }
+}
\ No newline at end of file