Bug 742104 - enable form history sync; minor improvements. r=rnewman
authorNick Alexander <nalexander@mozilla.com>
Fri, 13 Apr 2012 19:41:40 -0700
changeset 91654 b43e6857d7eea2af09793adf872076732809abc1
parent 91653 1dde63bcf17e5e6a1cd5737b03ea71da63d94ee3
child 91655 bbfe59782089095eb6a3b96102af172fc7f40d41
push id22465
push usermak77@bonardo.net
push dateSat, 14 Apr 2012 11:58:29 +0000
treeherdermozilla-central@6880c195054f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs742104
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 742104 - enable form history sync; minor improvements. r=rnewman
mobile/android/base/sync/InfoCollections.java
mobile/android/base/sync/repositories/android/FormHistoryRepositorySession.java
mobile/android/base/sync/repositories/domain/FormHistoryRecord.java
mobile/android/base/sync/stage/FormHistoryServerSyncStage.java
--- a/mobile/android/base/sync/InfoCollections.java
+++ b/mobile/android/base/sync/InfoCollections.java
@@ -1,16 +1,15 @@
 /* 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;
 
 import java.io.IOException;
-import java.net.URISyntaxException;
 import java.util.HashMap;
 import java.util.Map.Entry;
 import java.util.Set;
 
 import org.json.simple.parser.ParseException;
 import org.mozilla.gecko.sync.delegates.InfoCollectionsDelegate;
 import org.mozilla.gecko.sync.net.SyncStorageRecordRequest;
 import org.mozilla.gecko.sync.net.SyncStorageRequestDelegate;
--- a/mobile/android/base/sync/repositories/android/FormHistoryRepositorySession.java
+++ b/mobile/android/base/sync/repositories/android/FormHistoryRepositorySession.java
@@ -32,18 +32,23 @@ import android.content.Context;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.RemoteException;
 
 public class FormHistoryRepositorySession extends
     StoreTrackingRepositorySession {
   public static String LOG_TAG = "FormHistoryRepoSess";
 
-  protected static Uri FORM_HISTORY_CONTENT_URI = BrowserContractHelpers.FORM_HISTORY_CONTENT_URI;
-  protected static Uri DELETED_FORM_HISTORY_CONTENT_URI = BrowserContractHelpers.DELETED_FORM_HISTORY_CONTENT_URI;
+  /**
+   * Number of records to insert in one batch.
+   */
+  public static final int INSERT_ITEM_THRESHOLD = 200;
+
+  private static Uri FORM_HISTORY_CONTENT_URI = BrowserContractHelpers.FORM_HISTORY_CONTENT_URI;
+  private static Uri DELETED_FORM_HISTORY_CONTENT_URI = BrowserContractHelpers.DELETED_FORM_HISTORY_CONTENT_URI;
 
   public static class FormHistoryRepository extends Repository {
 
     @Override
     public void createSession(RepositorySessionCreationDelegate delegate,
                               Context context) {
       try {
         final FormHistoryRepositorySession session = new FormHistoryRepositorySession(this, context);
@@ -74,17 +79,19 @@ public class FormHistoryRepositorySessio
     if (client == null) {
       throw new NoContentProviderException(uri);
     }
     return client;
   }
 
   protected void releaseProviders() {
     try {
-      formsProvider.release();
+      if (formsProvider != null) {
+        formsProvider.release();
+      }
     } catch (Exception e) {
     }
   }
 
   // Only used for testing.
   public ContentProviderClient getFormsProvider() {
     return formsProvider;
   }
@@ -165,55 +172,55 @@ public class FormHistoryRepositorySessio
 
         String guidsArray[] = guids.toArray(new String[0]);
         delegate.onGuidsSinceSucceeded(guidsArray);
       }
     };
     delegateQueue.execute(command);
   }
 
-  protected FormHistoryRecord retrieveDuringFetch(Cursor cursor) {
+  protected static FormHistoryRecord retrieveDuringFetch(final Cursor cursor) {
     // A simple and efficient way to distinguish two tables.
     if (cursor.getColumnCount() == BrowserContractHelpers.FormHistoryColumns.length) {
       return formHistoryRecordFromCursor(cursor);
     } else {
       return deletedFormHistoryRecordFromCursor(cursor);
     }
   }
 
-  protected FormHistoryRecord formHistoryRecordFromCursor(Cursor cursor) {
+  protected static FormHistoryRecord formHistoryRecordFromCursor(final Cursor cursor) {
     String guid = RepoUtils.getStringFromCursor(cursor, FormHistory.GUID);
     String collection = "forms";
     FormHistoryRecord record = new FormHistoryRecord(guid, collection, 0, false);
 
     record.fieldName = RepoUtils.getStringFromCursor(cursor, FormHistory.FIELD_NAME);
     record.fieldValue = RepoUtils.getStringFromCursor(cursor, FormHistory.VALUE);
     record.androidID = RepoUtils.getLongFromCursor(cursor, FormHistory.ID);
     record.lastModified = RepoUtils.getLongFromCursor(cursor, FormHistory.FIRST_USED) / 1000; // Convert microseconds to milliseconds.
     record.deleted = false;
 
     record.log(LOG_TAG);
     return record;
   }
 
-  protected FormHistoryRecord deletedFormHistoryRecordFromCursor(Cursor cursor) {
+  protected static FormHistoryRecord deletedFormHistoryRecordFromCursor(final Cursor cursor) {
     String guid = RepoUtils.getStringFromCursor(cursor, DeletedFormHistory.GUID);
     String collection = "forms";
     FormHistoryRecord record = new FormHistoryRecord(guid, collection, 0, false);
 
     record.guid = RepoUtils.getStringFromCursor(cursor, DeletedFormHistory.GUID);
     record.androidID = RepoUtils.getLongFromCursor(cursor, DeletedFormHistory.ID);
     record.lastModified = RepoUtils.getLongFromCursor(cursor, DeletedFormHistory.TIME_DELETED);
     record.deleted = true;
 
     record.log(LOG_TAG);
     return record;
   }
 
-  protected void fetchFromCursor(Cursor cursor, RecordFilter filter, final RepositorySessionFetchRecordsDelegate delegate)
+  protected static void fetchFromCursor(final Cursor cursor, final RecordFilter filter, final RepositorySessionFetchRecordsDelegate delegate)
       throws NullCursorException {
     Logger.debug(LOG_TAG, "Fetch from cursor");
     if (cursor == null) {
       throw new NullCursorException(null);
     }
     try {
       if (!cursor.moveToFirst()) {
         return;
@@ -265,22 +272,22 @@ public class FormHistoryRepositorySessio
 
         delegate.onFetchCompleted(end);
       }
     };
 
     delegateQueue.execute(command);
   }
 
-  protected String regularBetween(long start, long end) {
+  protected static String regularBetween(long start, long end) {
     return FormHistory.FIRST_USED + " >= " + Long.toString(1000 * start) + " AND " +
            FormHistory.FIRST_USED + " <= " + Long.toString(1000 * end); // Microseconds.
   }
 
-  protected String deletedBetween(long start, long end) {
+  protected static String deletedBetween(long start, long end) {
     return DeletedFormHistory.TIME_DELETED + " >= " + Long.toString(start) + " AND " +
            DeletedFormHistory.TIME_DELETED + " <= " + Long.toString(end); // Milliseconds.
   }
 
   @Override
   public void fetchSince(final long timestamp, final RepositorySessionFetchRecordsDelegate delegate) {
     Logger.info(LOG_TAG, "Running fetchSince(" + timestamp + ").");
 
@@ -406,103 +413,91 @@ public class FormHistoryRepositorySessio
    * the server.
    * <p>
    * We purge the record's GUID from the regular and deleted tables.
    *
    * @param existingRecord
    *          The local <code>Record</code> to replace.
    * @throws RemoteException
    */
-  protected void deleteExistingRecord(Record existingRecord)
-      throws RemoteException {
+  protected void deleteExistingRecord(Record existingRecord) throws RemoteException {
     if (existingRecord.deleted) {
       formsProvider.delete(DELETED_FORM_HISTORY_CONTENT_URI, GUID_IS, new String[] { existingRecord.guid });
       return;
     }
     formsProvider.delete(FORM_HISTORY_CONTENT_URI, GUID_IS, new String[] { existingRecord.guid });
   }
 
-  protected ContentValues contentValuesForRegularRecord(Record rawRecord) {
+  protected static ContentValues contentValuesForRegularRecord(Record rawRecord) {
     if (rawRecord.deleted) {
       throw new IllegalArgumentException("Deleted record passed to insertNewRegularRecord.");
     }
 
     FormHistoryRecord record = (FormHistoryRecord) rawRecord;
     ContentValues cv = new ContentValues();
     cv.put(FormHistory.GUID, record.guid);
     cv.put(FormHistory.FIELD_NAME, record.fieldName);
     cv.put(FormHistory.VALUE, record.fieldValue);
     cv.put(FormHistory.FIRST_USED, 1000 * record.lastModified); // Microseconds.
     return cv;
   }
 
   protected Object recordsBufferMonitor = new Object();
   protected ArrayList<ContentValues> recordsBuffer = new ArrayList<ContentValues>();
-  private static final int INSERT_ITEM_THRESHOLD = 5;
 
   protected void enqueueRegularRecord(Record record) {
     synchronized (recordsBufferMonitor) {
       if (recordsBuffer.size() >= INSERT_ITEM_THRESHOLD) {
         // Insert the existing contents, then enqueue.
-        flushInsertQueue();
+        try {
+          flushInsertQueue();
+        } catch (Exception e) {
+          delegate.onRecordStoreFailed(e);
+          return;
+        }
       }
       // Store the ContentValues, rather than the record.
       recordsBuffer.add(contentValuesForRegularRecord(record));
     }
   }
 
-  public class RecordInsertRunnable implements Runnable {
-    ContentValues[] queue;
-
-    public RecordInsertRunnable(ArrayList<ContentValues> queue) {
-      ContentValues[] values = new ContentValues[queue.size()];
-      this.queue = queue.toArray(values);
-    }
-
-    @Override
-    public void run() {
-      if (queue == null || queue.length == 0) {
-        Logger.debug(LOG_TAG, "No form history items to insert: RecordInsertRunnable returning immediately.");
-        return;
-      }
-
-      try {
-        Logger.debug(LOG_TAG, "Inserting " + queue.length + " form history items...");
-        long before = System.currentTimeMillis();
-        formsProvider.bulkInsert(FORM_HISTORY_CONTENT_URI, queue);
-        long after = System.currentTimeMillis();
-        Logger.debug(LOG_TAG, "Inserting " + queue.length + " form history items... DONE (" + (after - before) + " milliseconds).");
-      } catch (RemoteException e) {
-        // TODO: REALLY HANDLE THIS ERROR by making this not be a Runnable and handling errors inline.
-        e.printStackTrace();
-      }
-    }
-  }
-
   // Should always be called from storeWorkQueue.
-  protected void flushInsertQueue() {
+  protected void flushInsertQueue() throws RemoteException {
     synchronized (recordsBufferMonitor) {
       if (recordsBuffer.size() > 0) {
-        final ArrayList<ContentValues> outgoing = recordsBuffer;
+        final ContentValues[] outgoing = recordsBuffer.toArray(new ContentValues[0]);
         recordsBuffer = new ArrayList<ContentValues>();
-        new RecordInsertRunnable(outgoing).run();
+
+        if (outgoing == null || outgoing.length == 0) {
+          Logger.debug(LOG_TAG, "No form history items to insert; returning immediately.");
+          return;
+        }
+
+        long before = System.currentTimeMillis();
+        formsProvider.bulkInsert(FORM_HISTORY_CONTENT_URI, outgoing);
+        long after = System.currentTimeMillis();
+        Logger.debug(LOG_TAG, "Inserted " + outgoing.length + " form history items in (" + (after - before) + " milliseconds).");
       }
     }
   }
 
   @Override
   public void storeDone() {
     Runnable command = new Runnable() {
       @Override
       public void run() {
         Logger.debug(LOG_TAG, "Checking for residual form history items to insert.");
-        synchronized (recordsBufferMonitor) {
-          flushInsertQueue();
+        try {
+          synchronized (recordsBufferMonitor) {
+            flushInsertQueue();
+          }
+          storeDone(now());
+        } catch (Exception e) {
+          delegate.onRecordStoreFailed(e);
         }
-        storeDone(now()); // XXX?
       }
     };
     storeWorkQueue.execute(command);
   }
 
   /**
    * Called when a regular record with locally unknown GUID has been fetched
    * from the server.
--- a/mobile/android/base/sync/repositories/domain/FormHistoryRecord.java
+++ b/mobile/android/base/sync/repositories/domain/FormHistoryRecord.java
@@ -17,45 +17,47 @@ import org.mozilla.gecko.sync.repositori
  * {@link http://mxr.mozilla.org/services-central/source/services-central/services/sync/modules/engines/forms.js}
  */
 public class FormHistoryRecord extends Record {
   private static final String LOG_TAG = "FormHistoryRecord";
 
   public static final String  COLLECTION_NAME = "forms";
   private static final String PAYLOAD_NAME    = "name";
   private static final String PAYLOAD_VALUE   = "value";
+  public static final long FORMS_TTL = 60 * 24 * 60 * 60; // 60 days in seconds.
 
   /**
    * The name of the saved form field.
    */
   public String fieldName;
 
   /**
    * The value of the saved form field.
    */
   public String fieldValue;
 
   public FormHistoryRecord(String guid, String collection, long lastModified, boolean deleted) {
     super(guid, collection, lastModified, deleted);
+    this.ttl = FORMS_TTL;
   }
 
   public FormHistoryRecord(String guid, String collection, long lastModified) {
-    super(guid, collection, lastModified, false);
+    this(guid, collection, lastModified, false);
   }
 
   public FormHistoryRecord(String guid, String collection) {
-    super(guid, collection, 0, false);
+    this(guid, collection, 0, false);
   }
 
   public FormHistoryRecord(String guid) {
-    super(guid, COLLECTION_NAME, 0, false);
+    this(guid, COLLECTION_NAME, 0, false);
   }
 
   public FormHistoryRecord() {
-    super(Utils.generateGuid(), COLLECTION_NAME, 0, false);
+    this(Utils.generateGuid(), COLLECTION_NAME, 0, false);
   }
 
   @Override
   public Record copyWithIDs(String guid, long androidID) {
     FormHistoryRecord out = new FormHistoryRecord(guid, this.collection, this.lastModified, this.deleted);
     out.androidID = androidID;
     out.sortIndex = this.sortIndex;
 
--- a/mobile/android/base/sync/stage/FormHistoryServerSyncStage.java
+++ b/mobile/android/base/sync/stage/FormHistoryServerSyncStage.java
@@ -59,14 +59,9 @@ public class FormHistoryServerSyncStage 
       return r;
     }
   }
 
   @Override
   protected RecordFactory getRecordFactory() {
     return new FormHistoryRecordFactory();
   }
-
-  @Override
-  public boolean isEnabled() {
-    return false;
-  }
 }