Call AddToHomescreenDataFetcher::Observer callbacks when manifest fetch times out

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

Index: chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
index 4e67dcdc036c03d065b43caa3f5c66d8126cffe3..239f0518e12f6d77071f3dab3c220924eb997911 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
@@ -9,6 +9,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.graphics.Bitmap;
 import android.os.Environment;
+import android.test.suitebuilder.annotation.MediumTest;
 import android.test.suitebuilder.annotation.SmallTest;
 
 import org.chromium.base.ThreadUtils;
@@ -24,12 +25,14 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.chrome.test.ChromeActivityTestCaseBase;
 import org.chromium.chrome.test.util.browser.TabLoadObserver;
+import org.chromium.content.browser.test.util.CallbackHelper;
 import org.chromium.content.browser.test.util.Criteria;
 import org.chromium.content.browser.test.util.CriteriaHelper;
 import org.chromium.content.common.ContentSwitches;
 import org.chromium.net.test.EmbeddedTestServer;
 
 import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Tests org.chromium.chrome.browser.webapps.AddToHomescreenManager and its C++ counterpart.
@@ -112,6 +115,48 @@ public class AddToHomescreenManagerTest extends ChromeActivityTestCaseBase<Chrom
     }
 
     /**
+     * Test AddToHomescreenManager subclass which tracks whether the native callbacks
+     * (a proxy for the AddToHomescreenDataFetcher::Observer callbacks) have been called.
+     */
+    private static class AddToHomescreenManagerCallbackTracker extends AddToHomescreenManager {
+        public CallbackHelper mCallbackHelper;
+        public boolean mDialogShown = false;
+        public boolean mGotUserTitle = false;
+        public boolean mReadyToAdd = false;
+
+        public AddToHomescreenManagerCallbackTracker(Activity activity, Tab tab) {
+            super(activity, tab);
+
+            mCallbackHelper = new CallbackHelper();
+
+            AddToHomescreenManager.Observer observer = new AddToHomescreenManager.Observer() {
+                @Override
+                public void onUserTitleAvailable(String title) {
+                    mGotUserTitle = true;
+                    mCallbackHelper.notifyCalled();
+                }
+
+                @Override
+                public void onReadyToAdd(Bitmap icon) {
+                    mReadyToAdd = true;
+                    mCallbackHelper.notifyCalled();
+                }
+            };
+            setObserver(observer);
+        }
+
+        public void waitForCallbacks(int numCallbacks, int numSecondsTimeout) throws Exception {
+            mCallbackHelper.waitForCallback(0, numCallbacks, numSecondsTimeout, TimeUnit.SECONDS);
+        }
+
+        @Override
+        public void showDialog() {
+            mDialogShown = true;
+            mCallbackHelper.notifyCalled();
+        }
+    }
+
+    /**
      * Test AddToHomescreenManager subclass which mocks showing the add-to-homescreen dialog and
      * adds the shortcut to the home screen once it is ready.
      */
@@ -278,24 +323,50 @@ public class AddToHomescreenManagerTest extends ChromeActivityTestCaseBase<Chrom
         }
     }
 
+    /**
+     * Tests whether the AddToHomescreen native callbacks are called when:
+     * - WebAPKs are enabled.
+     * - the Web Manifest fetch times out.
+     */
+    @MediumTest
+    @Feature("{Webapp}")
+    public void testAddWebappShortcutManifestFetchTimesOut() throws Exception {

[dominickn, 2016/09/19 05:41:44]

Can you test both the non-WebAPK and WebAPK paths here (make this a non-test
method, and then have two test methods, once of which sets the command line flag
to enable WebAPKs)? The WebAPK path calls ShowDialog in
OnDidDetermineWebApkCompatibility.

Can you also check a non-PWA, and validate that none of the callbacks are
called?

[pkotwicz, 2016/09/19 23:34:02]

I have added a test for the non-WebAPK case.

Shouldn't AddToHomescreenManager#Observer#onUserTitleAvailable() and
AddToHomescreenManager#Observer#onReadyToAdd() still be called for non-PWAs?

[dominickn, 2016/09/20 00:15:12]

Sorry, mistyped. But OnDidDetermineWebApkCompatibility shouldn't be called - so
you should check for 2 callbacks, not 3 :)

[pkotwicz, 2016/09/20 01:52:35]

I have added a test for a non-PWA

+        ChromeWebApkHost.initForTesting(true);
+
+        EmbeddedTestServer testServer = EmbeddedTestServer.createAndStartFileServer(
+                getInstrumentation().getContext(), Environment.getExternalStorageDirectory());
+        // Register handler for "slow?1000" URL.

[dominickn, 2016/09/19 05:41:44]

Nit: it's slow?10000 in the test file

+        testServer.addDefaultHandlers(testServer.getURL("/chrome/test/data/banners"));
+
+        TabLoadObserver tabLoadObserver = new TabLoadObserver(mTab);
+        tabLoadObserver.fullyLoadUrl(
+                testServer.getURL("/chrome/test/data/banners/manifest_times_out_test_page.html"));
+
+        AddToHomescreenManagerCallbackTracker manager =
+                new AddToHomescreenManagerCallbackTracker(mActivity, mTab);
+        startManagerOnUiThread(manager);
+
+        // The AddToHomescreenDataFetcher timeout fires after 4 seconds. Wait 10 seconds to ensure
+        // that we do not miss the AddToHomescreenDataFetcher timeout timer firing.
+        manager.waitForCallbacks(3, 10);
+
+        assertTrue(manager.mDialogShown);
+        // This callback enables the text field in the add-to-homescreen dialog.
+        assertTrue(manager.mGotUserTitle);
+        // This callback enables the "Add" button in the add-to-homescreen dialog.
+        assertTrue(manager.mReadyToAdd);
+
+        destroyManagerOnUiThread(manager);
+    }
+
     private void loadUrl(String url, String expectedPageTitle) throws Exception {
         new TabLoadObserver(mTab, expectedPageTitle, null).fullyLoadUrl(url);
     }
 
-    private void addShortcutToTab(final Tab tab, final String title) throws Exception {
+    private void addShortcutToTab(Tab tab, String title) throws Exception {
         // Add the shortcut.
-        Callable<AddToHomescreenManager> callable =
-                new Callable<AddToHomescreenManager>() {
-            @Override
-            public AddToHomescreenManager call() {
-                AddToHomescreenManager manager =
-                        new TestAddToHomescreenManager(mActivity, tab, title);
-                manager.start();
-                return manager;
-            }
-        };
-        final AddToHomescreenManager manager =
-                ThreadUtils.runOnUiThreadBlockingNoException(callable);
+        TestAddToHomescreenManager manager = new TestAddToHomescreenManager(mActivity, tab, title);
+        startManagerOnUiThread(manager);
 
         // Make sure that the shortcut was added.
         CriteriaHelper.pollUiThread(new Criteria() {
@@ -305,6 +376,19 @@ public class AddToHomescreenManagerTest extends ChromeActivityTestCaseBase<Chrom
             }
         });
 
+        destroyManagerOnUiThread(manager);
+    }
+
+    private void startManagerOnUiThread(final AddToHomescreenManager manager) {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                manager.start();
+            }
+        });
+    }
+
+    private void destroyManagerOnUiThread(final AddToHomescreenManager manager) {
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
             @Override
             public void run() {