Update WebAPK after the user closes WebAPK

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

Index: chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java
index 813540f1564554177557f0e116d6f284fb1568e0..f944963eff0004d5478f54459667cac01e30e7bd 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java
@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.webapps;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import static org.chromium.webapk.lib.client.WebApkVersion.CURRENT_SHELL_APK_VERSION;
@@ -20,23 +22,29 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowApplication;
 import org.robolectric.shadows.ShadowBitmap;
 
+import org.chromium.base.Callback;
 import org.chromium.base.CommandLine;
 import org.chromium.base.ContextUtils;
+import org.chromium.base.PathUtils;
 import org.chromium.blink_public.platform.WebDisplayMode;
 import org.chromium.chrome.browser.DisableHistogramsRule;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content_public.common.ScreenOrientationValues;
+import org.chromium.testing.local.CustomShadowAsyncTask;
 import org.chromium.testing.local.LocalRobolectricTestRunner;
 import org.chromium.webapk.lib.common.WebApkConstants;
 import org.chromium.webapk.lib.common.WebApkMetaDataKeys;
 import org.chromium.webapk.test.WebApkTestHelper;
 
+import java.io.File;
+import java.io.FileOutputStream;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -44,7 +52,7 @@ import java.util.Map;
  * Unit tests for WebApkUpdateManager.
  */
 @RunWith(LocalRobolectricTestRunner.class)
-@Config(manifest = Config.NONE)
+@Config(manifest = Config.NONE, shadows = {CustomShadowAsyncTask.class})
 public class WebApkUpdateManagerTest {
     @Rule
     public DisableHistogramsRule mDisableHistogramsRule = new DisableHistogramsRule();
@@ -102,16 +110,14 @@ public class WebApkUpdateManagerTest {
     }
 
     private static class TestWebApkUpdateManager extends WebApkUpdateManager {
-        private WebappDataStorage mStorage;
+        private Callback<Boolean> mStoreUpdateRequestCallback;
+        private WebApkUpdateManager.WebApkUpdateCallback mUpdateCallback;
         private TestWebApkUpdateDataFetcher mFetcher;
-        private boolean mUpdateRequested;
         private String mUpdateName;
         private boolean mDestroyedFetcher;
-        private boolean mIsWebApkForeground;
 
         public TestWebApkUpdateManager(WebappDataStorage storage) {
-            super(null, storage);
-            mStorage = storage;
+            super(storage);
         }
 
         /**
@@ -125,7 +131,7 @@ public class WebApkUpdateManagerTest {
          * Returns whether an update has been requested.
          */
         public boolean updateRequested() {
-            return mUpdateRequested;
+            return mStoreUpdateRequestCallback != null;
         }
 
         /**
@@ -139,6 +145,14 @@ public class WebApkUpdateManagerTest {
             return mDestroyedFetcher;
         }
 
+        public Callback<Boolean> getStoreUpdateRequestCallback() {
+            return mStoreUpdateRequestCallback;
+        }
+
+        public WebApkUpdateManager.WebApkUpdateCallback getUpdateCallback() {
+            return mUpdateCallback;
+        }
+
         @Override
         protected WebApkUpdateDataFetcher buildFetcher() {
             mFetcher = new TestWebApkUpdateDataFetcher();
@@ -146,21 +160,17 @@ public class WebApkUpdateManagerTest {
         }
 
         @Override
-        protected void buildUpdateRequestAndSchedule(WebApkInfo info, String primaryIconUrl,
-                String badgeIconUrl, boolean isManifestStale) {
+        protected void storeWebApkUpdateRequestToFile(String updateRequestPath, WebApkInfo info,
+                String primaryIconUrl, String badgeIconUrl, boolean isManifestStale,
+                Callback<Boolean> callback) {
+            mStoreUpdateRequestCallback = callback;
             mUpdateName = info.name();
-            String updateRequestPath = mStorage.createAndSetUpdateRequestFilePath(info);
-            scheduleUpdate(updateRequestPath);
         }
 
         @Override
-        protected void updateAsyncImpl(String updateRequestPath) {
-            mUpdateRequested = true;
-        }
-
-        @Override
-        protected boolean isInForeground() {
-            return mIsWebApkForeground;
+        protected void updateWebApkFromFile(
+                String updateRequestPath, WebApkUpdateCallback callback) {
+            mUpdateCallback = callback;
         }
 
         @Override
@@ -169,10 +179,6 @@ public class WebApkUpdateManagerTest {
             mDestroyedFetcher = true;
         }
 
-        public void setIsWebApkForeground(boolean isForeground) {
-            mIsWebApkForeground = isForeground;
-        }
-
         // Stubbed out because real implementation uses native.
         @Override
         protected boolean urlsMatchIgnoringFragments(String url1, String url2) {
@@ -311,6 +317,29 @@ public class WebApkUpdateManagerTest {
                 infoFromManifestData(fetchedManifestData), primaryIconUrl, badgeIconUrl);
     }
 
+    private static void tryCompletingUpdate(
+            TestWebApkUpdateManager updateManager, @WebApkInstallResult int result) {
+        Callback<Boolean> storeUpdateRequestCallback =
+                updateManager.getStoreUpdateRequestCallback();
+        if (storeUpdateRequestCallback == null) return;

[Xi Han, 2017/09/11 15:24:23]

Nit: add a new line here, and remove line 326.

+        storeUpdateRequestCallback.onResult(true);
+
+        updateManager.updateNow(Mockito.mock(Runnable.class));
+        WebApkUpdateManager.WebApkUpdateCallback updateCallback = updateManager.getUpdateCallback();
+        if (updateCallback == null) return;

[Xi Han, 2017/09/11 15:24:23]

Nit: add a new line here.

+        updateCallback.onResultFromNative(result, false /* relaxUpdates */);
+    }
+
+    private static void writeRandomTextToFile(String path) {
+        File file = new File(path);
+        new File(file.getParent()).mkdirs();
+        try (FileOutputStream out = new FileOutputStream(file)) {
+            String text = "something";
+            out.write(text.getBytes());
+        } catch (Exception e) {
+        }
+    }
+
     private static String randomIconUrl(ManifestData fetchedManifestData) {
         if (fetchedManifestData == null || fetchedManifestData.iconUrlToMurmur2HashMap.isEmpty()) {
             return null;
@@ -339,6 +368,7 @@ public class WebApkUpdateManagerTest {
     @Before
     public void setUp() {
         ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
+        PathUtils.setPrivateDataDirectorySuffix("chrome");
         CommandLine.init(null);
 
         registerWebApk(WEBAPK_PACKAGE_NAME, defaultManifestData(), CURRENT_SHELL_APK_VERSION);
@@ -457,7 +487,7 @@ public class WebApkUpdateManagerTest {
         onGotManifestData(updateManager, manifestData);
         assertTrue(updateManager.updateRequested());
 
-        // Chrome is killed. {@link WebApkUpdateManager#onBuiltWebApk} is never called.
+        // Chrome is killed. {@link WebApkUpdateCallback#onResultFromNative} is never called.
 
         // Check {@link WebappDataStorage} state.
         WebappDataStorage storage = getStorage(WEBAPK_PACKAGE_NAME);
@@ -466,6 +496,63 @@ public class WebApkUpdateManagerTest {
                 mClock.currentTimeMillis(), storage.getLastWebApkUpdateRequestCompletionTime());
     }
 
+    /**
+     * Test that the pending update file is deleted after update completes regardless of whether
+     * update succeeded.
+     */
+    @Test
+    public void testPendingUpdateFileDeletedAfterUpdateCompletion() {
+        mClock.advance(WebappDataStorage.UPDATE_INTERVAL);
+
+        WebappDataStorage storage = getStorage(WEBAPK_PACKAGE_NAME);
+        TestWebApkUpdateManager updateManager = new TestWebApkUpdateManager(storage);
+        updateIfNeeded(updateManager);
+
+        ManifestData manifestData = defaultManifestData();

[Xi Han, 2017/09/11 15:24:23]

The block from line 511 to line 518 looks the same as the block from line 536 to
line 548, can you put them in one helper function like: File
createPendingUpdateFile()?

If returning a File object doesn't work for the assumption in line 524, you can
simply return a string in the helper function.

+        manifestData.name = DIFFERENT_NAME;
+        onGotManifestData(updateManager, manifestData);
+        assertTrue(updateManager.updateRequested());
+        String updateRequestPath = storage.getPendingUpdateRequestPath();
+        assertNotNull(updateRequestPath);
+        writeRandomTextToFile(updateRequestPath);
+        assertTrue(new File(updateRequestPath).exists());
+
+        tryCompletingUpdate(updateManager, WebApkInstallResult.FAILURE);
+        ShadowApplication.getInstance().runBackgroundTasks();
+
+        assertNull(storage.getPendingUpdateRequestPath());
+        assertFalse(new File(updateRequestPath).exists());
+    }
+
+    /**
+     * Test that the pending update file is deleted if
+     * {@link WebApkUpdateManager#nativeStoreWebApkUpdateRequestToFile} creates the pending update
+     * file but fails.
+     */
+    @Test
+    public void testFileDeletedIfStoreWebApkUpdateRequestToFileFails() {
+        mClock.advance(WebappDataStorage.UPDATE_INTERVAL);
+
+        WebappDataStorage storage = getStorage(WEBAPK_PACKAGE_NAME);
+        TestWebApkUpdateManager updateManager = new TestWebApkUpdateManager(storage);
+        updateIfNeeded(updateManager);
+
+        ManifestData manifestData = defaultManifestData();
+        manifestData.name = DIFFERENT_NAME;
+        onGotManifestData(updateManager, manifestData);
+        assertTrue(updateManager.updateRequested());
+        String updateRequestPath = storage.getPendingUpdateRequestPath();
+        assertNotNull(updateRequestPath);
+        writeRandomTextToFile(updateRequestPath);
+        assertTrue(new File(updateRequestPath).exists());
+
+        updateManager.getStoreUpdateRequestCallback().onResult(false);
+        ShadowApplication.getInstance().runBackgroundTasks();
+
+        assertNull(storage.getPendingUpdateRequestPath());
+        assertFalse(new File(updateRequestPath).exists());
+    }
+
     /**
      * Test that an update with data from the WebAPK's Android manifest is done if:
      * - WebAPK's code is out of date
@@ -741,61 +828,6 @@ public class WebApkUpdateManagerTest {
         assertFalse(checkUpdateNeededForFetchedManifest(androidManifestData, fetchedManifestData));
     }
 
-    @Test
-    public void testForceUpdateWhenUncompletedUpdateRequestRechesMaximumTimes() {
-        mClock.advance(WebappDataStorage.UPDATE_INTERVAL);
-        ManifestData differentManifestData = defaultManifestData();
-        differentManifestData.name = DIFFERENT_NAME;
-        WebappDataStorage storage = getStorage(WEBAPK_PACKAGE_NAME);
-
-        for (int i = 0; i < 3; ++i) {
-            TestWebApkUpdateManager updateManager = new TestWebApkUpdateManager(storage);
-            updateManager.setIsWebApkForeground(true);
-            updateIfNeeded(updateManager);
-
-            onGotManifestData(updateManager, differentManifestData);
-            assertTrue(updateManager.getHasPendingUpdateForTesting());
-            assertFalse(updateManager.updateRequested());
-            assertEquals(i + 1, storage.getUpdateRequests());
-        }
-
-        TestWebApkUpdateManager updateManager = new TestWebApkUpdateManager(storage);
-        updateManager.setIsWebApkForeground(true);
-        updateIfNeeded(updateManager);
-
-        onGotManifestData(updateManager, differentManifestData);
-        assertFalse(updateManager.getHasPendingUpdateForTesting());
-        assertTrue(updateManager.updateRequested());
-        assertEquals(0, storage.getUpdateRequests());
-    }
-
-    @Test
-    public void testRequestUpdateAfterWebApkOnStopIsCalled() {
-        ManifestData differentManifestData = defaultManifestData();
-        differentManifestData.name = DIFFERENT_NAME;
-        WebappDataStorage storage = getStorage(WEBAPK_PACKAGE_NAME);
-
-        mClock.advance(WebappDataStorage.UPDATE_INTERVAL);
-        TestWebApkUpdateManager updateManager = new TestWebApkUpdateManager(storage);
-        updateManager.setIsWebApkForeground(true);
-        updateIfNeeded(updateManager);
-        assertTrue(updateManager.updateCheckStarted());
-
-        onGotManifestData(updateManager, differentManifestData);
-        assertTrue(updateManager.getHasPendingUpdateForTesting());
-        assertFalse(updateManager.updateRequested());
-        assertEquals(1, storage.getUpdateRequests());
-
-        // Since {@link WebApkActivity#OnStop()} calls {@link requestPendingUpdate()} to trigger an
-        // update request, we call it directly for testing.
-        updateManager.setIsWebApkForeground(false);
-        updateManager.requestPendingUpdate();
-
-        assertFalse(updateManager.getHasPendingUpdateForTesting());
-        assertTrue(updateManager.updateRequested());
-        assertEquals(0, storage.getUpdateRequests());
-    }
-
     /**
      * Tests that a WebAPK update is requested immediately if:
      * the Shell APK is out of date,
@@ -814,10 +846,7 @@ public class WebApkUpdateManagerTest {
         assertTrue(updateManager.updateCheckStarted());
         onGotManifestData(updateManager, defaultManifestData());
         assertTrue(updateManager.updateRequested());
-
-        WebappDataStorage storage = getStorage(WEBAPK_PACKAGE_NAME);
-        storage.updateTimeOfLastWebApkUpdateRequestCompletion();
-        storage.updateLastRequestedShellApkVersion(CURRENT_SHELL_APK_VERSION);
+        tryCompletingUpdate(updateManager, WebApkInstallResult.FAILURE);
 
         mClock.advance(1);
         updateIfNeeded(updateManager);