[Android WebAPK] Daggerize WebApkUpdateManager

This CL:
- Adds @Inject constructor to WebApkUpdateManager
- Makes WebApkUpdateManager listen for activity destruction instead of having
WebApkActivity call WebApkUpdateManager#destroy()

BUG=1042147

Change-Id: I803900ffe2eaf7c8999dc54248676c93caf5b0bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2001360
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Glenn Hartmann <hartmanng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732667}

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

@@ -24,9 +24,6 @@ import org.chromium.webapk.lib.common.WebApkConstants;
  * UI-less Chrome.
  */
 public class WebApkActivity extends WebappActivity {
-    /** Manages whether to check update for the WebAPK, and starts update check if needed. */
-    private WebApkUpdateManager mUpdateManager;
-
     /** The start time that the activity becomes focused in milliseconds since boot. */
     private long mStartTime;
 
@@ -90,8 +87,7 @@ public class WebApkActivity extends WebappActivity {
         WebApkUma.recordShellApkVersion(info.shellApkVersion(), info.distributor());
         storage.incrementLaunchCount();
 
-        mUpdateManager = new WebApkUpdateManager(storage);
-        mUpdateManager.updateIfNeeded(getActivityTab(), info);
+        getComponent().resolveWebApkUpdateManager().updateIfNeeded(storage, info);
     }
 
     @Override
@@ -129,10 +125,6 @@ public class WebApkActivity extends WebappActivity {
 
     @Override
     protected void onDestroyInternal() {
-        if (mUpdateManager != null) {
-            mUpdateManager.destroy();
-        }
-
         // The common case is to be connected to just one WebAPK's services. For the sake of
         // simplicity disconnect from the services of all WebAPKs.
         ChromeWebApkHost.disconnectFromAllServices(true /* waitForPendingWork */);

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

@@ -17,8 +17,12 @@ import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
+import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.ShortcutHelper;
+import org.chromium.chrome.browser.dependency_injection.ActivityScope;
+import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
+import org.chromium.chrome.browser.lifecycle.Destroyable;
 import org.chromium.chrome.browser.metrics.WebApkUma;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.util.UrlUtilities;
@@ -29,16 +33,21 @@ import org.chromium.webapk.lib.client.WebApkVersion;
 
 import java.util.Map;
 
+import javax.inject.Inject;
+
 /**
  * WebApkUpdateManager manages when to check for updates to the WebAPK's Web Manifest, and sends
  * an update request to the WebAPK Server when an update is needed.
  */
-public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
+@ActivityScope
+public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer, Destroyable {
     private static final String TAG = "WebApkUpdateManager";
 
     // Maximum wait time for WebAPK update to be scheduled.
     private static final long UPDATE_TIMEOUT_MILLISECONDS = DateUtils.SECOND_IN_MILLIS * 30;
 
+    private final ChromeActivity mActivity;
+
     /** Whether updates are enabled. Some tests disable updates. */
     private static boolean sUpdatesEnabled = true;
 
@@ -46,7 +55,7 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
     private WebApkInfo mInfo;
 
     /** The WebappDataStorage with cached data about prior update requests. */
-    private final WebappDataStorage mStorage;
+    private WebappDataStorage mStorage;
 
     private WebApkUpdateDataFetcher mFetcher;
 
@@ -59,20 +68,24 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
         public void onResultFromNative(@WebApkInstallResult int result, boolean relaxUpdates);
     }
 
-    public WebApkUpdateManager(WebappDataStorage storage) {
-        mStorage = storage;
+    @Inject
+    public WebApkUpdateManager(
+            ChromeActivity activity, ActivityLifecycleDispatcher lifecycleDispatcher) {
+        mActivity = activity;
+        lifecycleDispatcher.register(this);
     }
 
     /**
      * Checks whether the WebAPK's Web Manifest has changed. Requests an updated WebAPK if the Web
      * Manifest has changed. Skips the check if the check was done recently.
-     * @param tab  The tab of the WebAPK.
-     * @param info The WebApkInfo of the WebAPK.
      */
-    public void updateIfNeeded(Tab tab, WebApkInfo info) {
+    public void updateIfNeeded(WebappDataStorage storage, WebApkInfo info) {
+        mStorage = storage;
         mInfo = info;
 
-        if (!shouldCheckIfWebManifestUpdated(mInfo)) return;
+        Tab tab = mActivity.getActivityTab();
+
+        if (tab == null || !shouldCheckIfWebManifestUpdated(mInfo)) return;
 
         mFetcher = buildFetcher();
         mFetcher.start(tab, mInfo, this);
@@ -85,6 +98,7 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
         }, UPDATE_TIMEOUT_MILLISECONDS);
     }
 
+    @Override
     public void destroy() {
         destroyFetcher();
         if (mUpdateFailureHandler != null) {

chrome/android/java/src/org/chromium/chrome/browser/webapps/dependency_injection/WebappActivityComponent.java

@@ -8,6 +8,7 @@ import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controll
 import org.chromium.chrome.browser.customtabs.dependency_injection.BaseCustomTabActivityComponent;
 import org.chromium.chrome.browser.dependency_injection.ActivityScope;
 import org.chromium.chrome.browser.dependency_injection.ChromeActivityCommonsModule;
+import org.chromium.chrome.browser.webapps.WebApkUpdateManager;
 import org.chromium.chrome.browser.webapps.WebappActivityTabController;
 
 import dagger.Subcomponent;
@@ -21,4 +22,5 @@ import dagger.Subcomponent;
 public interface WebappActivityComponent extends BaseCustomTabActivityComponent {
     TrustedWebActivityBrowserControlsVisibilityManager resolveBrowserControlsVisibilityManager();
     WebappActivityTabController resolveTabController();
+    WebApkUpdateManager resolveWebApkUpdateManager();
 }

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

@@ -18,9 +18,11 @@ import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
@@ -69,6 +71,7 @@ public class WebApkUpdateManagerTest {
     private static final long WEBAPK_THEME_COLOR = 2147483648L;
     private static final long WEBAPK_BACKGROUND_COLOR = 2147483648L;
 
+    private ChromeActivity mActivity;
     private Tab mTab;
 
     /**
@@ -79,8 +82,9 @@ public class WebApkUpdateManagerTest {
         private CallbackHelper mWaiter;
         private boolean mNeedsUpdate;
 
-        public TestWebApkUpdateManager(CallbackHelper waiter, WebappDataStorage storage) {
-            super(storage);
+        public TestWebApkUpdateManager(CallbackHelper waiter, ChromeActivity activity,
+                ActivityLifecycleDispatcher lifecycleDispatcher) {
+            super(activity, lifecycleDispatcher);
             mWaiter = waiter;
         }
 
@@ -141,7 +145,8 @@ public class WebApkUpdateManagerTest {
     public void setUp() throws Exception {
         mActivityTestRule.startMainActivityOnBlankPage();
         RecordHistogram.setDisabledForTests(true);
-        mTab = mActivityTestRule.getActivity().getActivityTab();
+        mActivity = mActivityTestRule.getActivity();
+        mTab = mActivity.getActivityTab();
 
         TestFetchStorageCallback callback = new TestFetchStorageCallback();
         WebappRegistry.getInstance().register(WEBAPK_ID, callback);
@@ -156,10 +161,12 @@ public class WebApkUpdateManagerTest {
      /** Checks whether a WebAPK update is needed. */
     private boolean checkUpdateNeeded(final CreationData creationData) throws Exception {
         CallbackHelper waiter = new CallbackHelper();
-        WebappDataStorage storage = WebappRegistry.getInstance().getWebappDataStorage(WEBAPK_ID);
-        final TestWebApkUpdateManager updateManager = new TestWebApkUpdateManager(waiter, storage);
+        final TestWebApkUpdateManager updateManager =
+                new TestWebApkUpdateManager(waiter, mActivity, mActivity.getLifecycleDispatcher());
 
         TestThreadUtils.runOnUiThreadBlocking(() -> {
+            WebappDataStorage storage =
+                    WebappRegistry.getInstance().getWebappDataStorage(WEBAPK_ID);
             WebApkInfo info = WebApkInfo.create(
                     "", creationData.scope, null, null, null, creationData.name,
                     creationData.shortName, creationData.displayMode, creationData.orientation, 0,
@@ -172,7 +179,7 @@ public class WebApkUpdateManagerTest {
                     1 /* webApkVersionCode */
 
             );
-            updateManager.updateIfNeeded(mTab, info);
+            updateManager.updateIfNeeded(storage, info);
         });
         waiter.waitForCallback(0);