Avoid downloading the metadata in quick succession.

Bug: 17668298
Bug: 8651816
Bug: 17709442
Change-Id: Ib232f9d95d7c01a028be85d10f974a5a570c0ed5

4 files changed, 79 insertions, 26 deletions

diff --git a/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java b/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java
index 41916b614..568c80abd 100644
--- a/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java
+++ b/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java
@@ -76,19 +76,29 @@ public final class DictionaryService extends Service {
      * How often, in milliseconds, we want to update the metadata. This is a
      * floor value; actually, it may happen several hours later, or even more.
      */
-    private static final long UPDATE_FREQUENCY = TimeUnit.DAYS.toMillis(4);
+    private static final long UPDATE_FREQUENCY_MILLIS = TimeUnit.DAYS.toMillis(4);
 
     /**
      * We are waked around midnight, local time. We want to wake between midnight and 6 am,
      * roughly. So use a random time between 0 and this delay.
      */
-    private static final int MAX_ALARM_DELAY = (int)TimeUnit.HOURS.toMillis(6);
+    private static final int MAX_ALARM_DELAY_MILLIS = (int)TimeUnit.HOURS.toMillis(6);
 
     /**
      * How long we consider a "very long time". If no update took place in this time,
      * the content provider will trigger an update in the background.
      */
-    private static final long VERY_LONG_TIME = TimeUnit.DAYS.toMillis(14);
+    private static final long VERY_LONG_TIME_MILLIS = TimeUnit.DAYS.toMillis(14);
+
+    /**
+     * After starting a download, how long we wait before considering it may be stuck. After this
+     * period is elapsed, if the keyboard tries to download again, then we cancel and re-register
+     * the request; if it's within this time, we just leave it be.
+     * It's important to note that we do not re-submit the request merely because the time is up.
+     * This is only to decide whether to cancel the old one and re-requesting when the keyboard
+     * fires a new request for the same data.
+     */
+    public static final long NO_CANCEL_DOWNLOAD_PERIOD_MILLIS = TimeUnit.SECONDS.toMillis(30);
 
     /**
      * An executor that serializes tasks given to it.
@@ -188,16 +198,16 @@ public final class DictionaryService extends Service {
      */
     private static void checkTimeAndMaybeSetupUpdateAlarm(final Context context) {
         // Of all clients, if the one that hasn't been updated for the longest
-        // is still more recent than UPDATE_FREQUENCY, do nothing.
-        if (!isLastUpdateAtLeastThisOld(context, UPDATE_FREQUENCY)) return;
+        // is still more recent than UPDATE_FREQUENCY_MILLIS, do nothing.
+        if (!isLastUpdateAtLeastThisOld(context, UPDATE_FREQUENCY_MILLIS)) return;
 
         PrivateLog.log("Date changed - registering alarm");
         AlarmManager alarmManager = (AlarmManager)context.getSystemService(Context.ALARM_SERVICE);
 
-        // Best effort to wake between midnight and MAX_ALARM_DELAY in the morning.
+        // Best effort to wake between midnight and MAX_ALARM_DELAY_MILLIS in the morning.
         // It doesn't matter too much if this is very inexact.
         final long now = System.currentTimeMillis();
-        final long alarmTime = now + new Random().nextInt(MAX_ALARM_DELAY);
+        final long alarmTime = now + new Random().nextInt(MAX_ALARM_DELAY_MILLIS);
         final Intent updateIntent = new Intent(DictionaryPackConstants.UPDATE_NOW_INTENT_ACTION);
         final PendingIntent pendingIntent = PendingIntent.getBroadcast(context, 0,
                 updateIntent, PendingIntent.FLAG_CANCEL_CURRENT);
@@ -223,11 +233,11 @@ public final class DictionaryService extends Service {
     /**
      * Refreshes data if it hasn't been refreshed in a very long time.
      *
-     * This will check the last update time, and if it's been more than VERY_LONG_TIME,
+     * This will check the last update time, and if it's been more than VERY_LONG_TIME_MILLIS,
      * update metadata now - and possibly take subsequent update actions.
      */
     public static void updateNowIfNotUpdatedInAVeryLongTime(final Context context) {
-        if (!isLastUpdateAtLeastThisOld(context, VERY_LONG_TIME)) return;
+        if (!isLastUpdateAtLeastThisOld(context, VERY_LONG_TIME_MILLIS)) return;
         UpdateHandler.tryUpdate(context, false);
     }
 
diff --git a/java/src/com/android/inputmethod/dictionarypack/DownloadIdAndStartDate.java b/java/src/com/android/inputmethod/dictionarypack/DownloadIdAndStartDate.java
new file mode 100644
index 000000000..6247a15e2
--- /dev/null
+++ b/java/src/com/android/inputmethod/dictionarypack/DownloadIdAndStartDate.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.inputmethod.dictionarypack;
+
+/**
+ * A simple container of download ID and download start date.
+ */
+public class DownloadIdAndStartDate {
+    public final long mId;
+    public final long mStartDate;
+    public DownloadIdAndStartDate(final long id, final long startDate) {
+        mId = id;
+        mStartDate = startDate;
+    }
+}
diff --git a/java/src/com/android/inputmethod/dictionarypack/MetadataDbHelper.java b/java/src/com/android/inputmethod/dictionarypack/MetadataDbHelper.java
index e9dde4245..c9e8f9118 100644
--- a/java/src/com/android/inputmethod/dictionarypack/MetadataDbHelper.java
+++ b/java/src/com/android/inputmethod/dictionarypack/MetadataDbHelper.java
@@ -433,18 +433,18 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
      *
      * @param context a context instance to open the database on
      * @param uri the URI to retrieve the metadata download ID of
-     * @return the metadata download ID, or NOT_AN_ID if no download is in progress
+     * @return the download id and start date, or null if the URL is not known
      */
-    public static long getMetadataDownloadIdForURI(final Context context,
-            final String uri) {
+    public static DownloadIdAndStartDate getMetadataDownloadIdAndStartDateForURI(
+            final Context context, final String uri) {
         SQLiteDatabase defaultDb = getDb(context, null);
         final Cursor cursor = defaultDb.query(CLIENT_TABLE_NAME,
-                new String[] { CLIENT_PENDINGID_COLUMN },
+                new String[] { CLIENT_PENDINGID_COLUMN, CLIENT_LAST_UPDATE_DATE_COLUMN },
                 CLIENT_METADATA_URI_COLUMN + " = ?", new String[] { uri },
                 null, null, null, null);
         try {
-            if (!cursor.moveToFirst()) return UpdateHandler.NOT_AN_ID;
-            return cursor.getInt(0); // Only one column, return it
+            if (!cursor.moveToFirst()) return null;
+            return new DownloadIdAndStartDate(cursor.getInt(0), cursor.getLong(1));
         } finally {
             cursor.close();
         }
@@ -922,6 +922,7 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
             final long downloadId) {
         final ContentValues values = new ContentValues();
         values.put(CLIENT_PENDINGID_COLUMN, downloadId);
+        values.put(CLIENT_LAST_UPDATE_DATE_COLUMN, System.currentTimeMillis());
         final SQLiteDatabase defaultDb = getDb(context, "");
         final Cursor cursor = MetadataDbHelper.queryClientIds(context);
         if (null == cursor) return;
diff --git a/java/src/com/android/inputmethod/dictionarypack/UpdateHandler.java b/java/src/com/android/inputmethod/dictionarypack/UpdateHandler.java
index d8c5f3165..90a750493 100644
--- a/java/src/com/android/inputmethod/dictionarypack/UpdateHandler.java
+++ b/java/src/com/android/inputmethod/dictionarypack/UpdateHandler.java
@@ -251,12 +251,16 @@ public final class UpdateHandler {
                 res.getBoolean(R.bool.metadata_downloads_visible_in_download_UI));
 
         final DownloadManagerWrapper manager = new DownloadManagerWrapper(context);
-        cancelUpdateWithDownloadManager(context, metadataUri, manager);
+        if (maybeCancelUpdateAndReturnIfStillRunning(context, metadataUri, manager,
+                DictionaryService.NO_CANCEL_DOWNLOAD_PERIOD_MILLIS)) {
+            // We already have a recent download in progress. Don't register a new download.
+            return;
+        }
         final long downloadId;
         synchronized (sSharedIdProtector) {
             downloadId = manager.enqueue(metadataRequest);
             DebugLogUtils.l("Metadata download requested with id", downloadId);
-            // If there is already a download in progress, it's been there for a while and
+            // If there is still a download in progress, it's been there for a while and
             // there is probably something wrong with download manager. It's best to just
             // overwrite the id and request it again. If the old one happens to finish
             // anyway, we don't know about its ID any more, so the downloadFinished
@@ -267,21 +271,29 @@ public final class UpdateHandler {
     }
 
     /**
-     * Cancels downloading a file, if there is one for this URI.
+     * Cancels downloading a file if there is one for this URI and it's too long.
      *
      * If we are not currently downloading the file at this URI, this is a no-op.
      *
      * @param context the context to open the database on
      * @param metadataUri the URI to cancel
      * @param manager an wrapped instance of DownloadManager
+     * @param graceTime if there was a download started less than this many milliseconds, don't
+     *  cancel and return true
+     * @return whether the download is still active
      */
-    private static void cancelUpdateWithDownloadManager(final Context context,
-            final String metadataUri, final DownloadManagerWrapper manager) {
+    private static boolean maybeCancelUpdateAndReturnIfStillRunning(final Context context,
+            final String metadataUri, final DownloadManagerWrapper manager, final long graceTime) {
         synchronized (sSharedIdProtector) {
-            final long metadataDownloadId =
-                    MetadataDbHelper.getMetadataDownloadIdForURI(context, metadataUri);
-            if (NOT_AN_ID == metadataDownloadId) return;
-            manager.remove(metadataDownloadId);
+            final DownloadIdAndStartDate metadataDownloadIdAndStartDate =
+                    MetadataDbHelper.getMetadataDownloadIdAndStartDateForURI(context, metadataUri);
+            if (null == metadataDownloadIdAndStartDate) return false;
+            if (NOT_AN_ID == metadataDownloadIdAndStartDate.mId) return false;
+            if (metadataDownloadIdAndStartDate.mStartDate + graceTime
+                    > System.currentTimeMillis()) {
+                return true;
+            }
+            manager.remove(metadataDownloadIdAndStartDate.mId);
             writeMetadataDownloadId(context, metadataUri, NOT_AN_ID);
         }
         // Consider a cancellation as a failure. As such, inform listeners that the download
@@ -289,6 +301,7 @@ public final class UpdateHandler {
         for (UpdateEventListener listener : linkedCopyOfList(sUpdateEventListeners)) {
             listener.downloadedMetadata(false);
         }
+        return false;
     }
 
     /**
@@ -303,7 +316,7 @@ public final class UpdateHandler {
     public static void cancelUpdate(final Context context, final String clientId) {
         final DownloadManagerWrapper manager = new DownloadManagerWrapper(context);
         final String metadataUri = MetadataDbHelper.getMetadataUriAsString(context, clientId);
-        cancelUpdateWithDownloadManager(context, metadataUri, manager);
+        maybeCancelUpdateAndReturnIfStillRunning(context, metadataUri, manager, 0 /* graceTime */);
     }
 
     /**
@@ -387,7 +400,7 @@ public final class UpdateHandler {
             // If any of these is metadata, we should update the DB
             boolean hasMetadata = false;
             for (DownloadRecord record : downloadRecords) {
-                if (null == record.mAttributes) {
+                if (record.isMetadata()) {
                     hasMetadata = true;
                     break;
                 }