Store URLs in WebappDataStorage, and purge them when history is cleared.

chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.webapps;
 
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.os.AsyncTask;
 
@@ skipping 31 matching lines @@
      */
     public interface FetchCallback {
         public void onWebappIdsRetrieved(Set<String> readObject);
     }
 
     /**
      * Registers the existence of a web app and creates the SharedPreference for it.
      * @param context  Context to open the registry with.
      * @param webappId The id of the web app to register.
      */
-    public static void registerWebapp(final Context context, final String webappId) {
+    public static void registerWebapp(final Context context, final String webappId,
+            final String originUrl) {
         new AsyncTask<Void, Void, Void>() {
             @Override
             protected final Void doInBackground(Void... nothing) {
                 SharedPreferences preferences = openSharedPreferences(context);
                 Set<String> webapps = new HashSet<String>(getRegisteredWebappIds(preferences));
                 boolean added = webapps.add(webappId);
                 assert added;
 
-                // Update the last used time of the {@link WebappDataStorage} so we can guarantee
-                // that the used time will be set (ie. != WebappDataStorage.INVALID_LAST_USED) if a
-                // web app appears in the registry.
-                new WebappDataStorage(context, webappId).updateLastUsedTime();
+                // Update the last used time of the {@link WebappDataStorage} so we can
+                // guarantee that the used time will be set (ie. !=
+                // WebappDataStorage.INVALID_LAST_USED) if a web app appears in the registry.

[gone, 2016/03/08 22:40:39]

nit: Rework this comment so that i.e. != WebappDataStorage.INVALID_LAST_USED
fits on the same line.  It's hard to parse.

[dominickn, 2016/03/09 08:18:32]

Done.

+                WebappDataStorage storage = new WebappDataStorage(context, webappId);
+                storage.updateOriginUrl(originUrl);
+                storage.updateLastUsedTime(System.currentTimeMillis());
                 preferences.edit().putStringSet(KEY_WEBAPP_SET, webapps).apply();
                 return null;
             }
         }.execute();
     }
 
     /**
      * Asynchronously retrieves the list of web app IDs which this registry is aware of.
      * @param context  Context to open the registry with.
      * @param callback Called when the set has been retrieved. The set may be empty.
      */
     @VisibleForTesting
     public static void getRegisteredWebappIds(final Context context, final FetchCallback callback) {
         new AsyncTask<Void, Void, Set<String>>() {
             @Override
             protected final Set<String> doInBackground(Void... nothing) {
                 return getRegisteredWebappIds(openSharedPreferences(context));
             }
 
             @Override
             protected final void onPostExecute(Set<String> result) {
                 callback.onWebappIdsRetrieved(result);
             }
         }.execute();
     }
 
     /**
-     * Deletes the data for all "old" web apps. i.e. web apps which have not been opened by the user
-     * in the last 3 months. Cleanup is run, at most, once a month.
+     * Deletes the data for all "old" web apps.
+     * "Old" web apps have not been opened by the user in the last 3 months, or have had their last
+     * used time set to 0 by the user clearing their history. Cleanup is run, at most, once a month.

[gone, 2016/03/08 22:40:39]

nit: newline between this and @param

[dominickn, 2016/03/09 08:18:32]

Done.

      * @param context     Context to open the registry with.
      * @param currentTime The current time which will be checked to decide if the task should be run
      *                    and if a web app should be cleaned up.
      */
     static void unregisterOldWebapps(final Context context, final long currentTime) {
         new AsyncTask<Void, Void, Void>() {
             @Override
             protected final Void doInBackground(Void... nothing) {
                 SharedPreferences preferences = openSharedPreferences(context);
                 long lastCleanup = preferences.getLong(KEY_LAST_CLEANUP, 0);
@@ skipping 45 matching lines @@
     @CalledByNative
     static void unregisterAllWebapps(Context context, final long callbackPointer) {
         unregisterAllWebapps(context, new Runnable() {
             @Override
             public void run() {
                 nativeOnWebappsUnregistered(callbackPointer);
             }
         });
     }
 
+    /**
+     * Deletes the origin URL and sets the last used time to 0 for all web apps.
+     */
+    @VisibleForTesting
+    static void clearWebappHistory(final Context context, final Runnable callback) {
+        new AsyncTask<Void, Void, Void>() {
+            @Override
+            protected final Void doInBackground(Void... nothing) {
+                SharedPreferences preferences = openSharedPreferences(context);
+                for (String id : getRegisteredWebappIds(preferences)) {
+                    WebappDataStorage.clearHistory(context, id);
+                }
+                return null;
+            }
+
+            @Override
+            protected final void onPostExecute(Void nothing) {
+                if (callback == null) return;

[gone, 2016/03/08 22:40:39]

Is this callback == null check here only for tests?  Seems like it's the kind of
thing you should assert never happens, then change the test to pass in a
runnable that checks if it gets called.

[dominickn, 2016/03/09 08:18:32]

I just copied from the block above this one. I'll refactor both.

+                callback.run();
+            }
+        }.execute();
+    }
+
+    @CalledByNative
+    static void clearWebappHistory(Context context, final long callbackPointer) {

[gone, 2016/03/08 22:40:39]

If this is only called by native code, use private static void.

[dominickn, 2016/03/09 08:18:32]

I did this in patchset #3, but it fails to compile on the
android_clang_dbg_recipe bot with an "uncalled private method" error from
findbugs. I think I'll need to remove the private?

[gone, 2016/03/10 23:27:21]

Strange... it's definitely called, isn't it?  I mean, it even compiled and
presumably ran when you did it locally.  Maybe findbugs is having trouble
unraveling it.  I'm fine with removing private.

[dominickn, 2016/03/11 05:14:42]

Done.

+        clearWebappHistory(context, new Runnable() {
+            @Override
+            public void run() {
+                nativeOnClearedWebappHistory(callbackPointer);
+            }
+        });
+    }
+
     private static SharedPreferences openSharedPreferences(Context context) {
         return context.getSharedPreferences(REGISTRY_FILE_NAME, Context.MODE_PRIVATE);
     }
 
     private static Set<String> getRegisteredWebappIds(SharedPreferences preferences) {
         return preferences.getStringSet(KEY_WEBAPP_SET, Collections.<String>emptySet());
     }
 
     private WebappRegistry() {
     }
 
     private static native void nativeOnWebappsUnregistered(long callbackPointer);
-}
+    private static native void nativeOnClearedWebappHistory(long callbackPointer);
+}