Bug 946857 - Fold into part2: review nits. draft
authorNick Alexander <nalexander@mozilla.com>
Mon, 15 Feb 2016 16:14:31 -0800
changeset 331125 4ccf6d5e04a479c22d025a9117d63c9db257cc79
parent 331124 cf314f56271190c64e3016a3ea312ed2cbce5f35
child 514306 cbe6edc960d1d0f534e9985bb72de1104a8c4592
push id10903
push usernalexander@mozilla.com
push dateTue, 16 Feb 2016 00:14:47 +0000
bugs946857
milestone47.0a1
Bug 946857 - Fold into part2: review nits. MozReview-Commit-ID: LmPwIvebfrr
mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
mobile/android/base/java/org/mozilla/gecko/db/LoginsProvider.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BrowserContractHelpers.java
mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testLoginsProvider.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
@@ -416,18 +416,18 @@ final class BrowserDatabaseHelper extend
         createSearchHistoryTable(db);
         createReadingListTable(db, TABLE_READING_LIST);
         didCreateCurrentReadingListTable = true;      // Mostly correct, in the absence of transactions.
         createReadingListIndices(db, TABLE_READING_LIST);
 
         createDeletedLoginsTable(db, TABLE_DELETED_LOGINS);
         createDisabledHostsTable(db, TABLE_DISABLED_HOSTS);
         createLoginsTable(db, TABLE_LOGINS);
+        createLoginsTableIndices(db, TABLE_LOGINS);
         didCreateLoginsTable = true;
-        createLoginsTableIndices(db, TABLE_LOGINS);
     }
 
     /**
      * Copies the tabs and clients tables out of the given tabs.db file and into the destinationDB.
      *
      * @param tabsDBFile Path to existing tabs.db.
      * @param destinationDB The destination database.
      */
--- a/mobile/android/base/java/org/mozilla/gecko/db/LoginsProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LoginsProvider.java
@@ -1,47 +1,42 @@
 /* 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.db;
 
-import org.mozilla.gecko.db.BrowserContract.DeletedLogins;
-import org.mozilla.gecko.db.BrowserContract.LoginsDisabledHosts;
-import org.mozilla.gecko.db.BrowserContract.Logins;
-import org.mozilla.gecko.sync.Utils;
-
 import android.content.ContentUris;
 import android.content.ContentValues;
 import android.content.UriMatcher;
 import android.database.Cursor;
 import android.database.DatabaseUtils;
 import android.database.MatrixCursor;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteQueryBuilder;
 import android.net.Uri;
+import android.support.annotation.NonNull;
 import android.text.TextUtils;
 import android.util.Base64;
 
+import org.mozilla.gecko.db.BrowserContract.DeletedLogins;
+import org.mozilla.gecko.db.BrowserContract.Logins;
+import org.mozilla.gecko.db.BrowserContract.LoginsDisabledHosts;
+import org.mozilla.gecko.sync.Utils;
+
 import java.io.UnsupportedEncodingException;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-import java.util.Arrays;
+import java.security.GeneralSecurityException;
 import java.util.HashMap;
 
 import javax.crypto.Cipher;
-import javax.crypto.NoSuchPaddingException;
-import javax.crypto.spec.IvParameterSpec;
-import javax.crypto.spec.SecretKeySpec;
+import javax.crypto.NullCipher;
 
 import static org.mozilla.gecko.db.BrowserContract.DeletedLogins.TABLE_DELETED_LOGINS;
+import static org.mozilla.gecko.db.BrowserContract.Logins.TABLE_LOGINS;
 import static org.mozilla.gecko.db.BrowserContract.LoginsDisabledHosts.TABLE_DISABLED_HOSTS;
-import static org.mozilla.gecko.db.BrowserContract.Logins.TABLE_LOGINS;
 
 public class LoginsProvider extends SharedBrowserDatabaseProvider {
 
     private static final int LOGINS = 100;
     private static final int LOGINS_ID = 101;
     private static final int DELETED_LOGINS = 102;
     private static final int DELETED_LOGINS_ID = 103;
     private static final int DISABLED_HOSTS = 104;
@@ -53,23 +48,16 @@ public class LoginsProvider extends Shar
     private static final HashMap<String, String> DISABLED_HOSTS_PROJECTION_MAP;
 
     private static final String DEFAULT_LOGINS_SORT_ORDER = Logins.HOSTNAME + " ASC";
     private static final String DEFAULT_DELETED_LOGINS_SORT_ORDER = DeletedLogins.TIME_DELETED + " ASC";
     private static final String DEFAULT_DISABLED_HOSTS_SORT_ORDER = LoginsDisabledHosts.HOSTNAME + " ASC";
     private static final String WHERE_GUID_IS_NULL = DeletedLogins.GUID + " IS NULL";
     private static final String WHERE_GUID_IS_VALUE = DeletedLogins.GUID + " = ?";
 
-    private static final String TRANSFORMATION = "AES/CBC/PKCS5Padding";
-    private static final String ALGORITHM_SHA_256 = "SHA-256";
-    private static final String SECRET_KEY = "mozilla";
-    private static final String INTIAL_VECTOR = "firefox";
-    private static final String UTF_8 = "UTF-8";
-    private static final String ALGORITHM_AES = "AES";
-
     protected static final String INDEX_LOGINS_HOSTNAME = "login_hostname_index";
     protected static final String INDEX_LOGINS_HOSTNAME_FORM_SUBMIT_URL = "login_hostname_formSubmitURL_index";
     protected static final String INDEX_LOGINS_HOSTNAME_HTTP_REALM = "login_hostname_httpRealm_index";
 
     static {
         URI_MATCHER.addURI(BrowserContract.LOGINS_AUTHORITY, "logins", LOGINS);
         URI_MATCHER.addURI(BrowserContract.LOGINS_AUTHORITY, "logins/#", LOGINS_ID);
         URI_MATCHER.addURI(BrowserContract.LOGINS_AUTHORITY, "deleted-logins", DELETED_LOGINS);
@@ -112,25 +100,25 @@ public class LoginsProvider extends Shar
     }
 
     @Override
     protected Uri insertInTransaction(Uri uri, ContentValues values) {
         trace("Calling insert in transaction on URI: " + uri);
 
         final int match = URI_MATCHER.match(uri);
         final SQLiteDatabase db = getWritableDatabase(uri);
-        long id = -1;
-        String guid = null;
+        final long id;
+        String guid;
 
         setupDefaultValues(values, uri);
         switch (match) {
             case LOGINS:
                 removeDeletedLoginsByGUIDInTransaction(values, db);
                 // Encrypt sensitive data.
-                encryptLogins(values);
+                encryptContentValueFields(values);
                 guid = values.getAsString(Logins.GUID);
                 debug("Inserting login in database with GUID: " + guid);
                 id = db.insertOrThrow(TABLE_LOGINS, Logins.GUID, values);
                 break;
 
             case DELETED_LOGINS:
                 guid = values.getAsString(DeletedLogins.GUID);
                 debug("Inserting deleted-login in database with GUID: " + guid);
@@ -169,22 +157,21 @@ public class LoginsProvider extends Shar
         switch (match) {
             case LOGINS_ID:
                 trace("Delete on LOGINS_ID: " + uri);
                 selection = DBUtils.concatenateWhere(selection, selectColumn(TABLE_LOGINS, Logins._ID));
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[]{Long.toString(ContentUris.parseId(uri))});
                 // Store the deleted client in deleted-logins table.
                 final String guid = getLoginGUIDByID(selection, selectionArgs, db);
-                if (TextUtils.isEmpty(guid)) {
+                if (guid == null) {
                     // No matching logins found for the id.
                     return 0;
-
                 }
-                boolean isInsertSuccessful = storeDeletedLoginForGUIDInTranscation(guid, db);
+                boolean isInsertSuccessful = storeDeletedLoginForGUIDInTransaction(guid, db);
                 if (!isInsertSuccessful) {
                     // Failed to insert into deleted-logins, return early.
                     return 0;
                 }
             // fall through
             case LOGINS:
                 trace("Delete on LOGINS: " + uri);
                 table = TABLE_LOGINS;
@@ -236,17 +223,17 @@ public class LoginsProvider extends Shar
                 selection = DBUtils.concatenateWhere(selection, selectColumn(TABLE_LOGINS, Logins._ID));
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[]{Long.toString(ContentUris.parseId(uri))});
 
             case LOGINS:
                 trace("Update on LOGINS: " + uri);
                 table = TABLE_LOGINS;
                 // Encrypt sensitive data.
-                encryptLogins(values);
+                encryptContentValueFields(values);
                 break;
 
             default:
                 throw new UnsupportedOperationException("Unknown update URI " + uri);
         }
 
         trace("Updating " + table + " on URI: " + uri);
         return db.update(table, values, selection, selectionArgs);
@@ -323,24 +310,25 @@ public class LoginsProvider extends Shar
                 break;
 
             default:
                 throw new UnsupportedOperationException("Unknown query URI " + uri);
         }
 
         trace("Running built query.");
         Cursor cursor = qb.query(db, projection, selection, selectionArgs, groupBy, null, sortOrder, limit);
-        cursor = decryptLogins(cursor);
+        // If decryptManyCursorRows does not return the original cursor, it closes it, so there's
+        // no need to close here.
+        cursor = decryptManyCursorRows(cursor);
         cursor.setNotificationUri(getContext().getContentResolver(), BrowserContract.LOGINS_AUTHORITY_URI);
-
         return cursor;
     }
 
     @Override
-    public String getType(Uri uri) {
+    public String getType(@NonNull Uri uri) {
         final int match = URI_MATCHER.match(uri);
 
         switch (match) {
             case LOGINS:
                 return Logins.CONTENT_TYPE;
 
             case LOGINS_ID:
                 return Logins.CONTENT_ITEM_TYPE;
@@ -362,35 +350,31 @@ public class LoginsProvider extends Shar
         }
     }
 
     /**
      * Caller is responsible for invoking this method inside a transaction.
      */
     private String getLoginGUIDByID(final String selection, final String[] selectionArgs, final SQLiteDatabase db) {
         final Cursor cursor = db.query(Logins.TABLE_LOGINS, new String[]{Logins.GUID}, selection, selectionArgs, null, null, DEFAULT_LOGINS_SORT_ORDER);
-        if (cursor == null) {
-            return null;
-        }
-
         try {
             if (!cursor.moveToFirst()) {
                 return null;
             }
-            return cursor.getString(cursor.getColumnIndex(Logins.GUID));
+            return cursor.getString(cursor.getColumnIndexOrThrow(Logins.GUID));
         } finally {
             cursor.close();
         }
     }
 
     /**
      * Caller is responsible for invoking this method inside a transaction.
      */
-    private boolean storeDeletedLoginForGUIDInTranscation(final String guid, final SQLiteDatabase db) {
-        if (TextUtils.isEmpty(guid)) {
+    private boolean storeDeletedLoginForGUIDInTransaction(final String guid, final SQLiteDatabase db) {
+        if (guid == null) {
             return false;
         }
         final ContentValues values = new ContentValues();
         values.put(DeletedLogins.GUID, guid);
         values.put(DeletedLogins.TIME_DELETED, System.currentTimeMillis());
         return db.insert(TABLE_DELETED_LOGINS, DeletedLogins.GUID, values) > 0;
     }
 
@@ -420,19 +404,21 @@ public class LoginsProvider extends Shar
                     throw new IllegalArgumentException("Must provide GUID for deleted-login");
                 }
                 break;
 
             case LOGINS:
                 values.put(Logins.TIME_CREATED, now);
                 // Generate GUID for new login. Don't override specified GUIDs.
                 if (!values.containsKey(Logins.GUID)) {
-                    String guid = Utils.generateGuid();
+                    final String guid = Utils.generateGuid();
                     values.put(Logins.GUID, guid);
                 }
+                // The database happily accepts strings for long values; this just lets us re-use
+                // the existing helper method.
                 String nowString = Long.toString(now);
                 DBUtils.replaceKey(values, null, Logins.HTTP_REALM, null);
                 DBUtils.replaceKey(values, null, Logins.FORM_SUBMIT_URL, null);
                 DBUtils.replaceKey(values, null, Logins.ENC_TYPE, "0");
                 DBUtils.replaceKey(values, null, Logins.TIME_LAST_USED, nowString);
                 DBUtils.replaceKey(values, null, Logins.TIME_PASSWORD_CHANGED, nowString);
                 DBUtils.replaceKey(values, null, Logins.TIMES_USED, "0");
                 break;
@@ -443,42 +429,41 @@ public class LoginsProvider extends Shar
                 }
                 break;
 
             default:
                 throw new UnsupportedOperationException("Unknown URI in setupDefaultValues " + uri);
         }
     }
 
-    private void encryptLogins(final ContentValues values) {
+    private void encryptContentValueFields(final ContentValues values) {
         if (values.containsKey(Logins.ENCRYPTED_PASSWORD)) {
             final String res = encrypt(values.getAsString(Logins.ENCRYPTED_PASSWORD));
             values.put(Logins.ENCRYPTED_PASSWORD, res);
         }
 
         if (values.containsKey(Logins.ENCRYPTED_USERNAME)) {
             final String res = encrypt(values.getAsString(Logins.ENCRYPTED_USERNAME));
             values.put(Logins.ENCRYPTED_USERNAME, res);
         }
     }
 
-    private Cursor decryptLogins(final Cursor cursor) {
-        int passwordIndex = -1;
-        int usernameIndex = -1;
-
-        if (cursor == null) {
-            return cursor;
-        }
-
-        try {
-            passwordIndex = cursor.getColumnIndexOrThrow(Logins.ENCRYPTED_PASSWORD);
-        } catch(Exception ex) { }
-        try {
-            usernameIndex = cursor.getColumnIndexOrThrow(Logins.ENCRYPTED_USERNAME);
-        } catch(Exception ex) { }
+    /**
+     * Replace each password and username encrypted ciphertext with its equivalent decrypted
+     * plaintext in the given cursor.
+     * <p/>
+     * The encryption algorithm used to protect logins is unspecified; and further, a consumer of
+     * consumers should never have access to encrypted ciphertext.
+     *
+     * @param cursor containing at least one of password and username encrypted ciphertexts.
+     * @return a new {@link Cursor} with password and username decrypted plaintexts.
+     */
+    private Cursor decryptManyCursorRows(final Cursor cursor) {
+        final int passwordIndex = cursor.getColumnIndex(Logins.ENCRYPTED_PASSWORD);
+        final int usernameIndex = cursor.getColumnIndex(Logins.ENCRYPTED_USERNAME);
 
         if (passwordIndex == -1 && usernameIndex == -1) {
             return cursor;
         }
 
         // Special case, decrypt the encrypted username or password before returning the cursor.
         final MatrixCursor newCursor = new MatrixCursor(cursor.getColumnNames(), cursor.getColumnCount());
         try {
@@ -504,46 +489,32 @@ public class LoginsProvider extends Shar
         } finally {
             // Close the old cursor before returning the new one.
             cursor.close();
         }
 
         return newCursor;
     }
 
-    private String encrypt(String initialValue) {
-        if (TextUtils.isEmpty(initialValue)) {
-            return initialValue;
-        }
-
+    private String encrypt(@NonNull String initialValue) {
         try {
             final Cipher cipher = getCipher(Cipher.ENCRYPT_MODE);
-            return Base64.encodeToString(cipher.doFinal(initialValue.getBytes(UTF_8)), Base64.URL_SAFE);
+            return Base64.encodeToString(cipher.doFinal(initialValue.getBytes("UTF-8")), Base64.URL_SAFE);
         } catch (Exception e) {
             debug("encryption failed : " + e);
             throw new IllegalStateException("Logins encryption failed", e);
         }
     }
 
-    private String decrypt(String initialValue) {
-        if (TextUtils.isEmpty(initialValue)) {
-            return initialValue;
-        }
-
+    private String decrypt(@NonNull String initialValue) {
         try {
             final Cipher cipher = getCipher(Cipher.DECRYPT_MODE);
-            return new String(cipher.doFinal(Base64.decode(initialValue.getBytes(UTF_8), Base64.URL_SAFE)));
+            return new String(cipher.doFinal(Base64.decode(initialValue.getBytes("UTF-8"), Base64.URL_SAFE)));
         } catch (Exception e) {
             debug("Decryption failed : " + e);
             throw new IllegalStateException("Logins decryption failed", e);
         }
     }
 
-    private Cipher getCipher(int mode) throws NoSuchAlgorithmException, NoSuchPaddingException,
-            UnsupportedEncodingException, InvalidKeyException, InvalidAlgorithmParameterException {
-        final Cipher cipher = Cipher.getInstance(TRANSFORMATION);
-        final MessageDigest sha = MessageDigest.getInstance(ALGORITHM_SHA_256);
-        final byte[] secretKey = Arrays.copyOf(sha.digest(SECRET_KEY.getBytes(UTF_8)), 32);
-        final IvParameterSpec ivParameterSpec = new IvParameterSpec(Arrays.copyOf(sha.digest(INTIAL_VECTOR.getBytes(UTF_8)), 16));
-        cipher.init(mode, new SecretKeySpec(secretKey, ALGORITHM_AES), ivParameterSpec);
-        return cipher;
+    private Cipher getCipher(int mode) throws UnsupportedEncodingException, GeneralSecurityException {
+        return new NullCipher();
     }
 }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BrowserContractHelpers.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BrowserContractHelpers.java
@@ -35,16 +35,17 @@ public class BrowserContractHelpers exte
   public static final Uri HISTORY_CONTENT_URI              = withSyncAndDeletedAndProfile(History.CONTENT_URI);
   public static final Uri SCHEMA_CONTENT_URI               = withSyncAndDeletedAndProfile(Schema.CONTENT_URI);
   public static final Uri PASSWORDS_CONTENT_URI            = withSyncAndDeletedAndProfile(Passwords.CONTENT_URI);
   public static final Uri DELETED_PASSWORDS_CONTENT_URI    = withSyncAndDeletedAndProfile(DeletedPasswords.CONTENT_URI);
   public static final Uri FORM_HISTORY_CONTENT_URI         = withSyncAndProfile(FormHistory.CONTENT_URI);
   public static final Uri DELETED_FORM_HISTORY_CONTENT_URI = withSyncAndProfile(DeletedFormHistory.CONTENT_URI);
   public static final Uri TABS_CONTENT_URI                 = withSyncAndProfile(Tabs.CONTENT_URI);
   public static final Uri CLIENTS_CONTENT_URI              = withSyncAndProfile(Clients.CONTENT_URI);
+  public static final Uri LOGINS_CONTENT_URI               = withSyncAndProfile(Logins.CONTENT_URI);
 
   public static final String[] PasswordColumns = new String[] {
     Passwords.ID,
     Passwords.HOSTNAME,
     Passwords.HTTP_REALM,
     Passwords.FORM_SUBMIT_URL,
     Passwords.USERNAME_FIELD,
     Passwords.PASSWORD_FIELD,
--- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testLoginsProvider.java
+++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testLoginsProvider.java
@@ -8,26 +8,26 @@ import android.content.ContentProvider;
 import android.content.ContentUris;
 import android.content.ContentValues;
 import android.database.Cursor;
 import android.database.sqlite.SQLiteDatabase;
 import android.net.Uri;
 
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.db.BrowserContract.DeletedLogins;
+import org.mozilla.gecko.db.BrowserContract.Logins;
 import org.mozilla.gecko.db.BrowserContract.LoginsDisabledHosts;
-import org.mozilla.gecko.db.BrowserContract.Logins;
 import org.mozilla.gecko.db.LoginsProvider;
 
 import java.util.concurrent.Callable;
 
 import static org.mozilla.gecko.db.BrowserContract.CommonColumns._ID;
 import static org.mozilla.gecko.db.BrowserContract.DeletedLogins.TABLE_DELETED_LOGINS;
+import static org.mozilla.gecko.db.BrowserContract.Logins.TABLE_LOGINS;
 import static org.mozilla.gecko.db.BrowserContract.LoginsDisabledHosts.TABLE_DISABLED_HOSTS;
-import static org.mozilla.gecko.db.BrowserContract.Logins.TABLE_LOGINS;
 
 public class testLoginsProvider extends ContentProviderTest {
 
     private static final String DB_NAME = "browser.db";
 
     private final TestCase[] TESTS_TO_RUN = {
             new InsertLoginsTest(),
             new UpdateLoginsTest(),
@@ -89,103 +89,121 @@ public class testLoginsProvider extends 
     }
 
     /**
      * LoginsProvider insert logins test.
      */
     private class InsertLoginsTest extends TestCase {
         @Override
         public void test() throws Exception {
-            ContentValues contentValues = createLogins("http://www.example.com", "http://www.example.com",
+            ContentValues contentValues = createLogin("http://www.example.com", "http://www.example.com",
                     "http://www.example.com", "username1", "password1", "username1", "password1", "guid1");
             long id = ContentUris.parseId(mProvider.insert(BrowserContract.Logins.CONTENT_URI, contentValues));
-            verifyLoginsExists(contentValues, id);
+            verifyLoginExists(contentValues, id);
             Cursor cursor = mProvider.query(Logins.CONTENT_URI, null, Logins.GUID + " = ?", new String[] { "guid1" }, null);
-            verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "logins found");
+            verifyRowMatches(contentValues, cursor, "logins found");
+
+            // Empty ("") encrypted username and password are valid.
+            contentValues = createLogin("http://www.example.com", "http://www.example.com",
+                    "http://www.example.com", "username1", "password1", "", "", "guid2");
+            id = ContentUris.parseId(mProvider.insert(BrowserContract.Logins.CONTENT_URI, contentValues));
+            verifyLoginExists(contentValues, id);
+            cursor = mProvider.query(Logins.CONTENT_URI, null, Logins.GUID + " = ?", new String[] { "guid2" }, null);
+            verifyRowMatches(contentValues, cursor, "logins found");
         }
     }
 
     /**
      * LoginsProvider updates logins test.
      */
     private class UpdateLoginsTest extends TestCase {
         @Override
         public void test() throws Exception {
             final String guid1 = "guid1";
-            ContentValues contentValues = createLogins("http://www.example.com", "http://www.example.com",
+            ContentValues contentValues = createLogin("http://www.example.com", "http://www.example.com",
                     "http://www.example.com", "username1", "password1", "username1", "password1", guid1);
+            long timeBeforeCreated = System.currentTimeMillis();
             long id = ContentUris.parseId(mProvider.insert(BrowserContract.Logins.CONTENT_URI, contentValues));
-            verifyLoginsExists(contentValues, id);
+            long timeAfterCreated = System.currentTimeMillis();
+            verifyLoginExists(contentValues, id);
+
+            Cursor cursor = getLoginById(id);
+            try {
+                mAsserter.ok(cursor.moveToFirst(), "cursor is not empty", "");
+                verifyBounded(timeBeforeCreated, cursor.getLong(cursor.getColumnIndexOrThrow(Logins.TIME_CREATED)), timeAfterCreated);
+            } finally {
+                cursor.close();
+            }
 
             contentValues.put(BrowserContract.Logins.ENCRYPTED_USERNAME, "username2");
             contentValues.put(Logins.ENCRYPTED_PASSWORD, "password2");
 
             Uri updateUri = Logins.CONTENT_URI.buildUpon().appendPath(String.valueOf(id)).build();
             int numUpdated = mProvider.update(updateUri, contentValues, null, null);
             mAsserter.is(1, numUpdated, "Correct number updated");
-            verifyLoginsExists(contentValues, id);
+            verifyLoginExists(contentValues, id);
 
             contentValues.put(BrowserContract.Logins.ENCRYPTED_USERNAME, "username1");
             contentValues.put(Logins.ENCRYPTED_PASSWORD, "password1");
 
             updateUri = Logins.CONTENT_URI;
-            numUpdated = mProvider.update(updateUri, contentValues, Logins.GUID + " = ?", new String[] { guid1 });
+            numUpdated = mProvider.update(updateUri, contentValues, Logins.GUID + " = ?", new String[]{guid1});
             mAsserter.is(1, numUpdated, "Correct number updated");
-            verifyLoginsExists(contentValues, id);
+            verifyLoginExists(contentValues, id);
         }
     }
 
     /**
      * LoginsProvider deletion logins test.
      * - inserts a new logins
      * - deletes the logins and verify deleted-logins table has entry for deleted guid.
      */
     private class DeleteLoginsTest extends TestCase {
         @Override
         public void test() throws Exception {
             final String guid1 = "guid1";
-            ContentValues contentValues = createLogins("http://www.example.com", "http://www.example.com",
+            ContentValues contentValues = createLogin("http://www.example.com", "http://www.example.com",
                     "http://www.example.com", "username1", "password1", "username1", "password1", guid1);
             long id = ContentUris.parseId(mProvider.insert(Logins.CONTENT_URI, contentValues));
-            verifyLoginsExists(contentValues, id);
+            verifyLoginExists(contentValues, id);
 
             Uri deletedUri = Logins.CONTENT_URI.buildUpon().appendPath(String.valueOf(id)).build();
             int numDeleted = mProvider.delete(deletedUri, null, null);
             mAsserter.is(1, numDeleted, "Correct number deleted");
             verifyNoRowExists(Logins.CONTENT_URI, "No login entry found");
 
             contentValues = new ContentValues();
             contentValues.put(DeletedLogins.GUID, guid1);
             Cursor cursor = mProvider.query(DeletedLogins.CONTENT_URI, null, null, null, null);
-            verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "deleted-login found");
+            verifyRowMatches(contentValues, cursor, "deleted-login found");
             cursor = mProvider.query(DeletedLogins.CONTENT_URI, null, DeletedLogins.GUID + " = ?", new String[] { guid1 }, null);
-            verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "deleted-login found");
+            verifyRowMatches(contentValues, cursor, "deleted-login found");
         }
     }
 
     /**
      * LoginsProvider re-insert logins test.
      * - inserts a row into deleted-logins
      * - insert the same login (matching guid) and verify deleted-logins table is empty.
      */
     private class InsertDeletedLoginsTest extends TestCase {
         @Override
         public void test() throws Exception {
             ContentValues contentValues = new ContentValues();
             contentValues.put(DeletedLogins.GUID, "guid1");
             long id = ContentUris.parseId(mProvider.insert(DeletedLogins.CONTENT_URI, contentValues));
             final Uri insertedUri = DeletedLogins.CONTENT_URI.buildUpon().appendPath(String.valueOf(id)).build();
             Cursor cursor = mProvider.query(insertedUri, null, null, null, null);
-            verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "deleted-login found");
+            verifyRowMatches(contentValues, cursor, "deleted-login found");
             verifyNoRowExists(BrowserContract.Logins.CONTENT_URI, "No login entry found");
 
-            contentValues = createLogins("http://www.example.com", "http://www.example.com",
+            contentValues = createLogin("http://www.example.com", "http://www.example.com",
                     "http://www.example.com", "username1", "password1", "username1", "password1", "guid1");
             id = ContentUris.parseId(mProvider.insert(Logins.CONTENT_URI, contentValues));
-            verifyLoginsExists(contentValues, id);
+            verifyLoginExists(contentValues, id);
             verifyNoRowExists(DeletedLogins.CONTENT_URI, "No deleted-login entry found");
         }
     }
 
     /**
      * LoginsProvider insert Deleted logins test.
      * - inserts a row into deleted-login without GUID.
      */
@@ -211,17 +229,17 @@ public class testLoginsProvider extends 
         @Override
         public void test() throws Exception {
             final String hostname = "localhost";
             final ContentValues contentValues = new ContentValues();
             contentValues.put(LoginsDisabledHosts.HOSTNAME, hostname);
             mProvider.insert(LoginsDisabledHosts.CONTENT_URI, contentValues);
             final Uri insertedUri = LoginsDisabledHosts.CONTENT_URI.buildUpon().appendPath("hostname").appendPath(hostname).build();
             final Cursor cursor = mProvider.query(insertedUri, null, null, null, null);
-            verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "disabled-hosts found");
+            verifyRowMatches(contentValues, cursor, "disabled-hosts found");
 
             final Uri deletedUri = LoginsDisabledHosts.CONTENT_URI.buildUpon().appendPath("hostname").appendPath(hostname).build();
             final int numDeleted = mProvider.delete(deletedUri, null, null);
             mAsserter.is(1, numDeleted, "Correct number deleted");
             verifyNoRowExists(LoginsDisabledHosts.CONTENT_URI, "No disabled-hosts entry found");
         }
     }
 
@@ -245,17 +263,17 @@ public class testLoginsProvider extends 
 
     /**
      * LoginsProvider login insertion with default values test.
      * - insert a login missing GUID, FORM_SUBMIT_URL, HTTP_REALM and verify default values are set.
      */
     private class InsertLoginsWithDefaultValuesTest extends TestCase {
         @Override
         protected void test() throws Exception {
-            ContentValues contentValues = createLogins("http://www.example.com", "http://www.example.com",
+            ContentValues contentValues = createLogin("http://www.example.com", "http://www.example.com",
                     "http://www.example.com", "username1", "password1", "username1", "password1", null);
             // Remove GUID, HTTP_REALM, FORM_SUBMIT_URL from content values
             contentValues.remove(Logins.GUID);
             contentValues.remove(Logins.FORM_SUBMIT_URL);
             contentValues.remove(Logins.HTTP_REALM);
 
             long id = ContentUris.parseId(mProvider.insert(BrowserContract.Logins.CONTENT_URI, contentValues));
             Cursor cursor = getLoginById(id);
@@ -267,95 +285,99 @@ public class testLoginsProvider extends 
             mAsserter.is(cursor.getString(cursor.getColumnIndex(Logins.FORM_SUBMIT_URL)), null, "FORM_SUBMIT_URL is not null");
             mAsserter.isnot(cursor.getString(cursor.getColumnIndex(Logins.TIME_LAST_USED)), null, "TIME_LAST_USED is not null");
             mAsserter.isnot(cursor.getString(cursor.getColumnIndex(Logins.TIME_CREATED)), null, "TIME_CREATED is not null");
             mAsserter.isnot(cursor.getString(cursor.getColumnIndex(Logins.TIME_PASSWORD_CHANGED)), null, "TIME_PASSWORD_CHANGED is not null");
             mAsserter.is(cursor.getString(cursor.getColumnIndex(Logins.ENC_TYPE)), "0", "ENC_TYPE is 0");
             mAsserter.is(cursor.getString(cursor.getColumnIndex(Logins.TIMES_USED)), "0", "TIMES_USED is 0");
 
             // Verify other values.
-            verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "Updated login found");
+            verifyRowMatches(contentValues, cursor, "Updated login found");
         }
     }
 
     /**
      * LoginsProvider login insertion with duplicate GUID test.
      * - insert two different logins with same GUID and verify that only one login exists.
      */
     private class InsertLoginsWithDuplicateGuidFailureTest extends TestCase {
         @Override
         protected void test() throws Exception {
             final String guid = "guid1";
-            ContentValues contentValues = createLogins("http://www.example.com", "http://www.example.com",
+            ContentValues contentValues = createLogin("http://www.example.com", "http://www.example.com",
                     "http://www.example.com", "username1", "password1", "username1", "password1", guid);
             long id1 = ContentUris.parseId(mProvider.insert(BrowserContract.Logins.CONTENT_URI, contentValues));
-            verifyLoginsExists(contentValues, id1);
+            verifyLoginExists(contentValues, id1);
 
             // Insert another login with duplicate GUID.
-            contentValues = createLogins("http://www.example2.com", "http://www.example2.com",
+            contentValues = createLogin("http://www.example2.com", "http://www.example2.com",
                     "http://www.example2.com", "username2", "password2", "username2", "password2", guid);
             Uri insertUri = mProvider.insert(Logins.CONTENT_URI, contentValues);
             mAsserter.is(insertUri, null, "Duplicate Guid insertion id1");
 
             // Verify login with id1 still exists.
-            verifyLoginsExists(contentValues, id1);
+            verifyLoginExists(contentValues, id1);
         }
     }
 
     /**
      * LoginsProvider deletion by non-existent GUID test.
      * - delete a login with random GUID and verify that no entry was deleted.
      */
     private class DeleteLoginsByNonExistentGuidTest extends TestCase {
         @Override
         protected void test() throws Exception {
             Uri deletedUri = Logins.CONTENT_URI;
             int numDeleted = mProvider.delete(deletedUri, Logins.GUID + "= ?", new String[] { "guid1" });
             mAsserter.is(0, numDeleted, "Correct number deleted");
         }
     }
 
+    private void verifyBounded(long left, long middle, long right) {
+        mAsserter.ok(left <= middle, "Left <= middle", left + " <= " + middle);
+        mAsserter.ok(middle <= right, "Middle <= right", middle + " <= " + right);
+    }
+
     private Cursor getById(Uri uri, long id, String[] projection) {
         return mProvider.query(uri, projection,
                 _ID + " = ?",
                 new String[] { String.valueOf(id) },
                 null);
     }
 
     private Cursor getLoginById(long id) {
         return getById(Logins.CONTENT_URI, id, null);
     }
 
-
-    private void verifyLoginsExists(ContentValues contentValues, long id) {
+    private void verifyLoginExists(ContentValues contentValues, long id) {
         Cursor cursor = getLoginById(id);
-        verifyRowsMatches(contentValues, cursor, cursor.moveToFirst(), "Updated login found");
+        verifyRowMatches(contentValues, cursor, "Updated login found");
     }
 
-    private void verifyRowsMatches(ContentValues contentValues, Cursor cursor, boolean condition, String name) {
+    private void verifyRowMatches(ContentValues contentValues, Cursor cursor, String name) {
         try {
-            mAsserter.ok(condition, name, "");
+            mAsserter.ok(cursor.moveToFirst(), name, "cursor is not empty");
             CursorMatches(cursor, contentValues);
         } finally {
             cursor.close();
         }
     }
 
     private void verifyNoRowExists(Uri contentUri, String name) {
         Cursor cursor = mProvider.query(contentUri, null, null, null, null);
         try {
             mAsserter.is(0, cursor.getCount(), name);
         } finally {
             cursor.close();
         }
     }
 
-    private ContentValues createLogins(String hostname, String httpRealm, String formSubmitUrl,
-                                       String usernameField, String passwordField, String encryptedUsername,
-                                       String encryptedPassword, String guid) {
+    private ContentValues createLogin(String hostname, String httpRealm, String formSubmitUrl,
+                                      String usernameField, String passwordField, String encryptedUsername,
+                                      String encryptedPassword, String guid) {
         final ContentValues values = new ContentValues();
         values.put(Logins.HOSTNAME, hostname);
         values.put(Logins.HTTP_REALM, httpRealm);
         values.put(Logins.FORM_SUBMIT_URL, formSubmitUrl);
         values.put(Logins.USERNAME_FIELD, usernameField);
         values.put(Logins.PASSWORD_FIELD, passwordField);
         values.put(Logins.ENCRYPTED_USERNAME, encryptedUsername);
         values.put(Logins.ENCRYPTED_PASSWORD, encryptedPassword);