Pass all intent extras needed to render splash screen when launching WebAPK

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

Index: chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java
index 3aa065f0fe0210947d7c632edc999c206b5b2455..1e9b52c6c06c7387e2208fb456c814cf4e8a50fa 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java
@@ -70,14 +70,10 @@ public class WebappInfoTest extends InstrumentationTestCase {
     @SmallTest
     @Feature({"Webapps"})
     public void testIntentTitleFallBack() {
-        String id = "webapp id";
         String title = "webapp title";
-        String url = "about:blank";
 
-        Intent intent = new Intent();
-        intent.putExtra(ShortcutHelper.EXTRA_ID, id);
+        Intent intent = createIntentWithUrlAndId();
         intent.putExtra(ShortcutHelper.EXTRA_TITLE, title);
-        intent.putExtra(ShortcutHelper.EXTRA_URL, url);
 
         WebappInfo info = WebappInfo.create(intent);
         assertEquals(title, info.name());
@@ -87,14 +83,10 @@ public class WebappInfoTest extends InstrumentationTestCase {
     @SmallTest
     @Feature({"Webapps"})
     public void testIntentNameBlankNoTitle() {
-        String id = "webapp id";
         String shortName = "name";
-        String url = "about:blank";
 
-        Intent intent = new Intent();
-        intent.putExtra(ShortcutHelper.EXTRA_ID, id);
+        Intent intent = createIntentWithUrlAndId();
         intent.putExtra(ShortcutHelper.EXTRA_SHORT_NAME, shortName);
-        intent.putExtra(ShortcutHelper.EXTRA_URL, url);
 
         WebappInfo info = WebappInfo.create(intent);
         assertEquals("", info.name());
@@ -104,16 +96,12 @@ public class WebappInfoTest extends InstrumentationTestCase {
     @SmallTest
     @Feature({"Webapps"})
     public void testIntentShortNameFallBack() {
-        String id = "webapp id";
         String title = "webapp title";
         String shortName = "name";
-        String url = "about:blank";
 
-        Intent intent = new Intent();
-        intent.putExtra(ShortcutHelper.EXTRA_ID, id);
+        Intent intent = createIntentWithUrlAndId();
         intent.putExtra(ShortcutHelper.EXTRA_TITLE, title);
         intent.putExtra(ShortcutHelper.EXTRA_SHORT_NAME, shortName);
-        intent.putExtra(ShortcutHelper.EXTRA_URL, url);
 
         WebappInfo info = WebappInfo.create(intent);
         assertEquals(title, info.name());
@@ -123,16 +111,12 @@ public class WebappInfoTest extends InstrumentationTestCase {
     @SmallTest
     @Feature({"Webapps"})
     public void testIntentNameShortname() {
-        String id = "webapp id";
         String name = "longName";
         String shortName = "name";
-        String url = "about:blank";
 
-        Intent intent = new Intent();
-        intent.putExtra(ShortcutHelper.EXTRA_ID, id);
+        Intent intent = createIntentWithUrlAndId();
         intent.putExtra(ShortcutHelper.EXTRA_NAME, name);
         intent.putExtra(ShortcutHelper.EXTRA_SHORT_NAME, shortName);
-        intent.putExtra(ShortcutHelper.EXTRA_URL, url);
 
         WebappInfo info = WebappInfo.create(intent);
         assertEquals(name, info.name());
@@ -192,16 +176,12 @@ public class WebappInfoTest extends InstrumentationTestCase {
     @SmallTest
     @Feature({"Webapps"})
     public void testColorsIntentCreation() {
-        String id = "webapp id";
-        String url = "http://money.cnn.com";
         long themeColor = 0xFF00FF00L;
         long backgroundColor = 0xFF0000FFL;
 
-        Intent intent = new Intent();
+        Intent intent = createIntentWithUrlAndId();
         intent.putExtra(ShortcutHelper.EXTRA_THEME_COLOR, themeColor);
         intent.putExtra(ShortcutHelper.EXTRA_BACKGROUND_COLOR, backgroundColor);
-        intent.putExtra(ShortcutHelper.EXTRA_ID, id);
-        intent.putExtra(ShortcutHelper.EXTRA_URL, url);
 
         WebappInfo info = WebappInfo.create(intent);
         assertEquals(info.themeColor(), themeColor);
@@ -267,18 +247,57 @@ public class WebappInfoTest extends InstrumentationTestCase {
     @SmallTest
     @Feature({"WebApk"})
     public void testIntentWebApkPackageName() {
-        String id = WebApkConstants.WEBAPK_ID_PREFIX + "id";
-        String name = "longName";
-        String url = "http://www.foo.com/homepage";
         String packageName = WebApkConstants.WEBAPK_PACKAGE_PREFIX + ".foo";
 
-        Intent intent = new Intent();
-        intent.putExtra(ShortcutHelper.EXTRA_ID, id);
-        intent.putExtra(ShortcutHelper.EXTRA_NAME, name);
-        intent.putExtra(ShortcutHelper.EXTRA_URL, url);
+        Intent intent = createIntentWithUrlAndId();
         intent.putExtra(ShortcutHelper.EXTRA_WEBAPK_PACKAGE_NAME, packageName);
 
         WebappInfo info = WebappInfo.create(intent);
         assertEquals(packageName, info.webApkPackageName());
     }
+
+    @SmallTest
+    @Feature({"WebApk"})
+    public void testIntentWebApkDisplayMode() {
+        {
+            Intent intent = createIntentWithUrlAndId();
+            intent.putExtra(WebApkConstants.EXTRA_DISPLAY_MODE, "fullscreen");
+            WebappInfo info = WebappInfo.create(intent);
+            assertEquals(WebDisplayMode.Fullscreen, info.displayMode());
+        }
+        {
+            Intent intent = createIntentWithUrlAndId();
+            intent.putExtra(WebApkConstants.EXTRA_DISPLAY_MODE, "invalid");
+            WebappInfo info = WebappInfo.create(intent);
+            assertEquals(WebDisplayMode.Standalone, info.displayMode());
+        }
+    }

[dominickn, 2016/06/28 05:26:55]

Can you please add a testIntentDisplayModeAndOrientation, since parsing these
out of the intent now has two possible paths? Also, you should test in the
testIntentWebApk* tests that enum values in the intent are overridden by the
existence of the string.

[pkotwicz, 2016/06/28 17:56:21]

I have added tests for extracting the non-WebAPK display mode and orientation.

I have not added tests that the enum values in the intent are overridden by the
existence of the string. I chose the priority order randomly so I do not think
that it is worth testing the priority order.

[dominickn, 2016/06/29 00:45:54]

Even though you chose the order randomly, it's still important to unit test it,
particularly since it's destructive. You're setting a canonical value in
AndroidManifest.xml, so testing the overriding behaviour is necessary to ensure
that the canonical value stays canonical.

[pkotwicz, 2016/06/29 01:54:00]

I added unit tests :)

+
+    @SmallTest
+    @Feature({"WebApk"})
+    public void testIntentWebApkOrientation() {
+        {
+            Intent intent = createIntentWithUrlAndId();
+            intent.putExtra(WebApkConstants.EXTRA_ORIENTATION, "natural");
+            WebappInfo info = WebappInfo.create(intent);
+            assertEquals(ScreenOrientationValues.NATURAL, info.orientation());
+        }
+        {
+            Intent intent = createIntentWithUrlAndId();
+            intent.putExtra(WebApkConstants.EXTRA_ORIENTATION, "invalid");
+            WebappInfo info = WebappInfo.create(intent);
+            assertEquals(ScreenOrientationValues.DEFAULT, info.orientation());
+        }
+    }
+
+    /**
+     * Creates intent with url and id. If the url or id are not set WebappInfo#create() returns
+     * null.
+     */
+    private Intent createIntentWithUrlAndId() {
+        Intent intent = new Intent();
+        intent.putExtra(ShortcutHelper.EXTRA_ID, "web app id");
+        intent.putExtra(ShortcutHelper.EXTRA_URL, "about:blank");
+        return intent;
+    }
 }