[Android Refactor] Merge WebappDataStorage#LAST_USED_UNSET and LAST_U…

…SED_INVALID

Having both LAST_USED_UNSET and LAST_USED_INVALID is unnecessary as
- the "last_used" SharedPreference should never be set to LAST_USED_INVALID
- there isn't any logic which checks whether "last_used" is LAST_USED_INVALID

Additionally, this CL renames LAST_USED_INVALID to TIMESTAMP_INVALID as
the getters for several time-based SharedPreferences other than "last_used"
return LAST_USED_INVALID if the preference is unset.

BUG=None

Review-Url: https://codereview.chromium.org/2927723003
Cr-Commit-Position: refs/heads/master@{#478067}

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

@@ -84,10 +84,9 @@ public class WebappDataStorage {
     // The default shell Apk version of WebAPKs.
     static final int DEFAULT_SHELL_APK_VERSION = 1;
 
-    // Unset/invalid constants for last used times and URLs. 0 is used as the null last used time as
-    // WebappRegistry assumes that this is always a valid timestamp.
-    static final long LAST_USED_UNSET = 0;
-    static final long LAST_USED_INVALID = -1;
+    // Invalid constants for timestamps and URLs. '0' is used as the invalid timestamp as
+    // WebappRegistry and WebApkUpdateManager assume that timestamps are always valid.
+    static final long TIMESTAMP_INVALID = 0;
     static final String URL_INVALID = "";
     static final int VERSION_INVALID = 0;
 
@@ -139,14 +138,8 @@ public long currentTimeMillis() {
      * Opens an instance of WebappDataStorage for the web app specified.
      * @param webappId The ID of the web app.
      */
-    static WebappDataStorage open(final String webappId) {
-        final WebappDataStorage storage = sFactory.create(webappId);
-        if (storage.getLastUsedTime() == LAST_USED_INVALID) {
-            // If the last used time is invalid then ensure that there is no data in the
-            // WebappDataStorage which needs to be cleaned up.
-            assert storage.isEmpty();
-        }
-        return storage;
+    static WebappDataStorage open(String webappId) {
+        return sFactory.create(webappId);
     }
 
     /**
@@ -321,10 +314,7 @@ void delete() {
     void clearHistory() {
         SharedPreferences.Editor editor = mPreferences.edit();
 
-        // The last used time is set to 0 to ensure that a valid value is always present.
-        // If the web app is not launched prior to the next cleanup, then its remaining data will be
-        // removed. Otherwise, the next launch from home screen will update the last used time.
-        editor.putLong(KEY_LAST_USED, LAST_USED_UNSET);
+        editor.remove(KEY_LAST_USED);
         editor.remove(KEY_URL);
         editor.remove(KEY_SCOPE);
         editor.remove(KEY_LAST_CHECK_WEB_MANIFEST_UPDATE_TIME);
@@ -364,7 +354,7 @@ public void updateSource(int source) {
      * Returns the last used time of this object, or -1 if it is not stored.
      */
     public long getLastUsedTime() {
-        return mPreferences.getLong(KEY_LAST_USED, LAST_USED_INVALID);
+        return mPreferences.getLong(KEY_LAST_USED, TIMESTAMP_INVALID);
     }
 
     /**
@@ -406,7 +396,7 @@ void updateTimeOfLastCheckForUpdatedWebManifest() {
      * updated. This time needs to be set when the WebAPK is registered.
      */
     private long getLastCheckForWebManifestUpdateTime() {
-        return mPreferences.getLong(KEY_LAST_CHECK_WEB_MANIFEST_UPDATE_TIME, LAST_USED_INVALID);
+        return mPreferences.getLong(KEY_LAST_CHECK_WEB_MANIFEST_UPDATE_TIME, TIMESTAMP_INVALID);
     }
 
     /**
@@ -423,7 +413,7 @@ void updateTimeOfLastWebApkUpdateRequestCompletion() {
      * This time needs to be set when the WebAPK is registered.
      */
     long getLastWebApkUpdateRequestCompletionTime() {
-        return mPreferences.getLong(KEY_LAST_UPDATE_REQUEST_COMPLETE_TIME, LAST_USED_INVALID);
+        return mPreferences.getLong(KEY_LAST_UPDATE_REQUEST_COMPLETE_TIME, TIMESTAMP_INVALID);
     }
 
     /**
@@ -485,7 +475,7 @@ int getLastRequestedShellApkVersion() {
      */
     boolean didPreviousUpdateSucceed() {
         long lastUpdateCompletionTime = getLastWebApkUpdateRequestCompletionTime();
-        if (lastUpdateCompletionTime == WebappDataStorage.LAST_USED_INVALID) {
+        if (lastUpdateCompletionTime == TIMESTAMP_INVALID) {
             return true;
         }
         return getDidLastWebApkUpdateRequestSucceed();

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

@@ -117,8 +117,10 @@ protected final WebappDataStorage doInBackground(Void... nothing) {
 
             @Override
             protected final void onPostExecute(WebappDataStorage storage) {
-                // Guarantee that last used time != WebappDataStorage.LAST_USED_INVALID. Must be
-                // run on the main thread as SharedPreferences.Editor.apply() is called.
+                // Update the last used time in order to prevent
+                // {@link WebappRegistry@unregisterOldWebapps()} from deleting the
+                // WebappDataStorage. Must be run on the main thread as
+                // SharedPreferences.Editor.apply() is called.
                 mStorages.put(webappId, storage);
                 mPreferences.edit().putStringSet(KEY_WEBAPP_SET, mStorages.keySet()).apply();
                 storage.updateLastUsedTime();

chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java

@@ -71,7 +71,7 @@ public class WebApkUpdateManagerTest {
 
     /** {@link WebappDataStorage#Clock} subclass which enables time to be manually advanced. */
     private static class MockClock extends WebappDataStorage.Clock {
-        // 0 has a special meaning: {@link WebappDataStorage#LAST_USED_UNSET}.
+        // 0 has a special meaning: {@link WebappDataStorage#TIMESTAMP_INVALID}.
         private long mTimeMillis = 1;
 
         public void advance(long millis) {

chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java

@@ -18,7 +18,6 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowLooper;
@@ -194,11 +193,10 @@ public void testWasLaunchedRecently() throws Exception {
         storage.updateLastUsedTime();
         assertTrue(storage.wasLaunchedRecently());
 
-        long lastUsedTime = mSharedPreferences.getLong(WebappDataStorage.KEY_LAST_USED,
-                WebappDataStorage.LAST_USED_INVALID);
+        long lastUsedTime = mSharedPreferences.getLong(
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
 
-        assertTrue(lastUsedTime != WebappDataStorage.LAST_USED_UNSET);
-        assertTrue(lastUsedTime != WebappDataStorage.LAST_USED_INVALID);
+        assertTrue(lastUsedTime != WebappDataStorage.TIMESTAMP_INVALID);
 
         // Move the last used time one day in the past.
         mSharedPreferences.edit()

chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java

@@ -17,7 +17,6 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowLooper;
@@ -127,8 +126,8 @@ public void testWebappRegistrationUpdatesLastUsed() throws Exception {
         long after = System.currentTimeMillis();
         SharedPreferences webAppPrefs = ContextUtils.getApplicationContext().getSharedPreferences(
                 WebappDataStorage.SHARED_PREFS_FILE_PREFIX + "test", Context.MODE_PRIVATE);
-        long actual = webAppPrefs.getLong(WebappDataStorage.KEY_LAST_USED,
-                WebappDataStorage.LAST_USED_INVALID);
+        long actual = webAppPrefs.getLong(
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
         assertTrue("Timestamp is out of range", actual <= after);
     }
 
@@ -247,8 +246,8 @@ public void testCleanupDoesNotRunTooOften() throws Exception {
         Set<String> actual = getRegisteredWebapps();
         assertEquals(new HashSet<>(Arrays.asList("oldWebapp")), actual);
 
-        long actualLastUsed = webAppPrefs.getLong(WebappDataStorage.KEY_LAST_USED,
-                WebappDataStorage.LAST_USED_INVALID);
+        long actualLastUsed = webAppPrefs.getLong(
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
         assertEquals(Long.MIN_VALUE, actualLastUsed);
 
         // The last cleanup time was set to 0 in setUp() so check that this hasn't changed.
@@ -280,8 +279,8 @@ public void testCleanupDoesNotRemoveRecentApps() throws Exception {
         Set<String> actual = getRegisteredWebapps();
         assertEquals(new HashSet<>(Arrays.asList("recentWebapp")), actual);
 
-        long actualLastUsed = webAppPrefs.getLong(WebappDataStorage.KEY_LAST_USED,
-                WebappDataStorage.LAST_USED_INVALID);
+        long actualLastUsed = webAppPrefs.getLong(
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
         assertEquals(lastUsed, actualLastUsed);
 
         long lastCleanup = mSharedPreferences.getLong(WebappRegistry.KEY_LAST_CLEANUP, -1);
@@ -311,9 +310,9 @@ public void testCleanupRemovesOldApps() throws Exception {
         Set<String> actual = getRegisteredWebapps();
         assertTrue(actual.isEmpty());
 
-        long actualLastUsed = webAppPrefs.getLong(WebappDataStorage.KEY_LAST_USED,
-                WebappDataStorage.LAST_USED_INVALID);
-        assertEquals(WebappDataStorage.LAST_USED_INVALID, actualLastUsed);
+        long actualLastUsed = webAppPrefs.getLong(
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
+        assertEquals(WebappDataStorage.TIMESTAMP_INVALID, actualLastUsed);
 
         long lastCleanup = mSharedPreferences.getLong(WebappRegistry.KEY_LAST_CLEANUP, -1);
         assertEquals(currentTime, lastCleanup);
@@ -421,11 +420,11 @@ public void testClearWebappHistory() throws Exception {
                 WebappDataStorage.SHARED_PREFS_FILE_PREFIX + "webapp2", Context.MODE_PRIVATE);
 
         long webapp1OriginalLastUsed = webapp2Prefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_UNSET);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
         long webapp2OriginalLastUsed = webapp2Prefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_UNSET);
-        assertTrue(webapp1OriginalLastUsed != WebappDataStorage.LAST_USED_UNSET);
-        assertTrue(webapp2OriginalLastUsed != WebappDataStorage.LAST_USED_UNSET);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
+        assertTrue(webapp1OriginalLastUsed != WebappDataStorage.TIMESTAMP_INVALID);
+        assertTrue(webapp2OriginalLastUsed != WebappDataStorage.TIMESTAMP_INVALID);
 
         // Clear data for |webapp1Url|.
         WebappRegistry.getInstance().clearWebappHistoryForUrlsImpl(
@@ -437,12 +436,12 @@ public void testClearWebappHistory() throws Exception {
         assertTrue(actual.contains("webapp2"));
 
         // Verify that the last used time for the first web app is
-        // WebappDataStorage.LAST_USED_UNSET, while for the second one it's unchanged.
+        // WebappDataStorage.TIMESTAMP_INVALID, while for the second one it's unchanged.
         long actualLastUsed = webapp1Prefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_UNSET);
-        assertEquals(WebappDataStorage.LAST_USED_UNSET, actualLastUsed);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
+        assertEquals(WebappDataStorage.TIMESTAMP_INVALID, actualLastUsed);
         actualLastUsed = webapp2Prefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_UNSET);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
         assertEquals(webapp2OriginalLastUsed, actualLastUsed);
 
         // Verify that the URL and scope for the first web app is WebappDataStorage.URL_INVALID,
@@ -463,13 +462,13 @@ public void testClearWebappHistory() throws Exception {
         // Clear data for all urls.
         WebappRegistry.getInstance().clearWebappHistoryForUrlsImpl(new UrlFilters.AllUrls());
 
-        // Verify that the last used time for both web apps is WebappDataStorage.LAST_USED_UNSET.
+        // Verify that the last used time for both web apps is WebappDataStorage.TIMESTAMP_INVALID.
         actualLastUsed = webapp1Prefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_UNSET);
-        assertEquals(WebappDataStorage.LAST_USED_UNSET, actualLastUsed);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
+        assertEquals(WebappDataStorage.TIMESTAMP_INVALID, actualLastUsed);
         actualLastUsed = webapp2Prefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_UNSET);
-        assertEquals(WebappDataStorage.LAST_USED_UNSET, actualLastUsed);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
+        assertEquals(WebappDataStorage.TIMESTAMP_INVALID, actualLastUsed);
 
         // Verify that the URL and scope for both web apps is WebappDataStorage.URL_INVALID.
         actualScope = webapp1Prefs.getString(
@@ -500,9 +499,8 @@ public void testGetAfterClearWebappHistory() throws Exception {
 
         // Verify that the last used time is valid.
         long actualLastUsed = webappPrefs.getLong(
-                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.LAST_USED_INVALID);
-        assertTrue(WebappDataStorage.LAST_USED_INVALID != actualLastUsed);
-        assertTrue(WebappDataStorage.LAST_USED_UNSET != actualLastUsed);
+                WebappDataStorage.KEY_LAST_USED, WebappDataStorage.TIMESTAMP_INVALID);
+        assertTrue(WebappDataStorage.TIMESTAMP_INVALID != actualLastUsed);
     }
 
     @Test