Add support for 'display' in Manifest.

BUG=366145

Review URL: https://codereview.chromium.org/563083004

Cr-Commit-Position: refs/heads/master@{#295438}

5 files changed, 143 insertions, 32 deletions

diff --git a/content/common/manifest_manager_messages.h b/content/common/manifest_manager_messages.h
index a415a9f..2775c21 100644
--- a/content/common/manifest_manager_messages.h
+++ b/content/common/manifest_manager_messages.h
@@ -14,10 +14,14 @@
 
 #define IPC_MESSAGE_START ManifestManagerMsgStart
 
+IPC_ENUM_TRAITS_MAX_VALUE(content::Manifest::DisplayMode,
+                          content::Manifest::DISPLAY_MODE_BROWSER)
+
 IPC_STRUCT_TRAITS_BEGIN(content::Manifest)
   IPC_STRUCT_TRAITS_MEMBER(name)
   IPC_STRUCT_TRAITS_MEMBER(short_name)
   IPC_STRUCT_TRAITS_MEMBER(start_url)
+  IPC_STRUCT_TRAITS_MEMBER(display)
 IPC_STRUCT_TRAITS_END()
 
 // The browser process requests for the manifest linked with the associated
diff --git a/content/public/common/manifest.cc b/content/public/common/manifest.cc
index f1dd57e..d9fa2b2 100644
--- a/content/public/common/manifest.cc
+++ b/content/public/common/manifest.cc
@@ -8,14 +8,18 @@ namespace content {
 
 const size_t Manifest::kMaxIPCStringLength = 4 * 1024;
 
-Manifest::Manifest() {
+Manifest::Manifest()
+    : display(DISPLAY_MODE_UNSPECIFIED) {
 }
 
 Manifest::~Manifest() {
 }
 
 bool Manifest::IsEmpty() const {
-  return name.is_null() && short_name.is_null();
+  return name.is_null() &&
+         short_name.is_null() &&
+         start_url.is_empty() &&
+         display == DISPLAY_MODE_UNSPECIFIED;
 }
 
 } // namespace content
diff --git a/content/public/common/manifest.h b/content/public/common/manifest.h
index 1b9b3c7..c790bab 100644
--- a/content/public/common/manifest.h
+++ b/content/public/common/manifest.h
@@ -15,6 +15,14 @@ namespace content {
 // described in the "Manifest for Web Application" document:
 // http://w3c.github.io/manifest/
 struct CONTENT_EXPORT Manifest {
+  enum DisplayMode {
+    DISPLAY_MODE_UNSPECIFIED,
+    DISPLAY_MODE_FULLSCREEN,
+    DISPLAY_MODE_STANDALONE,
+    DISPLAY_MODE_MINIMAL_UI,
+    DISPLAY_MODE_BROWSER
+  };
+
   Manifest();
   ~Manifest();
 
@@ -31,6 +39,10 @@ struct CONTENT_EXPORT Manifest {
   // Empty if the parsing failed or the field was not present.
   GURL start_url;
 
+  // Set to DISPLAY_MODE_UNSPECIFIED if the parsing failed or the field was not
+  // present.
+  DisplayMode display;
+
   // Maximum length for all the strings inside the Manifest when it is sent over
   // IPC. The renderer process should truncate the strings before sending the
   // Manifest and the browser process must do the same when receiving it.
diff --git a/content/renderer/manifest/manifest_parser.cc b/content/renderer/manifest/manifest_parser.cc
index 19b69a5..b7b7d5a 100644
--- a/content/renderer/manifest/manifest_parser.cc
+++ b/content/renderer/manifest/manifest_parser.cc
@@ -11,23 +11,38 @@
 #include "base/values.h"
 #include "content/public/common/manifest.h"
 
+namespace content {
+
 namespace {
 
-// Parses the 'name' field of the manifest, as defined in:
-// http://w3c.github.io/manifest/#dfn-steps-for-processing-the-name-member
-// Returns the parsed string if any, a null string if the parsing failed.
-base::NullableString16 ParseName(const base::DictionaryValue& dictionary)  {
-  if (!dictionary.HasKey("name"))
+enum TrimType {
+  Trim,
+  NoTrim
+};
+
+base::NullableString16 ParseString(const base::DictionaryValue& dictionary,
+                                   const std::string& key,
+                                   TrimType trim) {
+  if (!dictionary.HasKey(key))
     return base::NullableString16();
 
-  base::string16 name;
-  if (!dictionary.GetString("name", &name)) {
-    // TODO(mlamouri): provide a custom message to the developer console.
+  base::string16 value;
+  if (!dictionary.GetString(key, &value)) {
+    // TODO(mlamouri): provide a custom message to the developer console about
+    // the property being incorrectly set.
     return base::NullableString16();
   }
 
-  base::TrimWhitespace(name, base::TRIM_ALL, &name);
-  return base::NullableString16(name, false);
+  if (trim == Trim)
+    base::TrimWhitespace(value, base::TRIM_ALL, &value);
+  return base::NullableString16(value, false);
+}
+
+// Parses the 'name' field of the manifest, as defined in:
+// http://w3c.github.io/manifest/#dfn-steps-for-processing-the-name-member
+// Returns the parsed string if any, a null string if the parsing failed.
+base::NullableString16 ParseName(const base::DictionaryValue& dictionary)  {
+  return ParseString(dictionary, "name", Trim);
 }
 
 // Parses the 'short_name' field of the manifest, as defined in:
@@ -35,17 +50,7 @@ base::NullableString16 ParseName(const base::DictionaryValue& dictionary)  {
 // Returns the parsed string if any, a null string if the parsing failed.
 base::NullableString16 ParseShortName(
     const base::DictionaryValue& dictionary)  {
-  if (!dictionary.HasKey("short_name"))
-    return base::NullableString16();
-
-  base::string16 short_name;
-  if (!dictionary.GetString("short_name", &short_name)) {
-    // TODO(mlamouri): provide a custom message to the developer console.
-    return base::NullableString16();
-  }
-
-  base::TrimWhitespace(short_name, base::TRIM_ALL, &short_name);
-  return base::NullableString16(short_name, false);
+  return ParseString(dictionary, "short_name", Trim);
 }
 
 // Parses the 'start_url' field of the manifest, as defined in:
@@ -54,16 +59,13 @@ base::NullableString16 ParseShortName(
 GURL ParseStartURL(const base::DictionaryValue& dictionary,
                    const GURL& manifest_url,
                    const GURL& document_url) {
-  if (!dictionary.HasKey("start_url"))
-    return GURL();
+  base::NullableString16 start_url_str =
+      ParseString(dictionary, "start_url", NoTrim);
 
-  base::string16 start_url_str;
-  if (!dictionary.GetString("start_url", &start_url_str)) {
-    // TODO(mlamouri): provide a custom message to the developer console.
+  if (start_url_str.is_null())
     return GURL();
-  }
 
-  GURL start_url = manifest_url.Resolve(start_url_str);
+  GURL start_url = manifest_url.Resolve(start_url_str.string());
   if (!start_url.is_valid())
     return GURL();
 
@@ -75,9 +77,29 @@ GURL ParseStartURL(const base::DictionaryValue& dictionary,
   return start_url;
 }
 
-} // anonymous namespace
+// Parses the 'display' field of the manifest, as defined in:
+// http://w3c.github.io/manifest/#dfn-steps-for-processing-the-display-member
+// Returns the parsed DisplayMode if any, DISPLAY_MODE_UNSPECIFIED if the
+// parsing failed.
+Manifest::DisplayMode ParseDisplay(const base::DictionaryValue& dictionary) {
+  base::NullableString16 display = ParseString(dictionary, "display", Trim);
+
+  if (display.is_null())
+    return Manifest::DISPLAY_MODE_UNSPECIFIED;
+
+  if (LowerCaseEqualsASCII(display.string(), "fullscreen"))
+    return Manifest::DISPLAY_MODE_FULLSCREEN;
+  else if (LowerCaseEqualsASCII(display.string(), "standalone"))
+    return Manifest::DISPLAY_MODE_STANDALONE;
+  else if (LowerCaseEqualsASCII(display.string(), "minimal-ui"))
+    return Manifest::DISPLAY_MODE_MINIMAL_UI;
+  else if (LowerCaseEqualsASCII(display.string(), "browser"))
+    return Manifest::DISPLAY_MODE_BROWSER;
+  else
+    return Manifest::DISPLAY_MODE_UNSPECIFIED;
+}
 
-namespace content {
+} // anonymous namespace
 
 Manifest ManifestParser::Parse(const base::StringPiece& json,
                                const GURL& manifest_url,
@@ -106,6 +128,7 @@ Manifest ManifestParser::Parse(const base::StringPiece& json,
   manifest.name = ParseName(*dictionary);
   manifest.short_name = ParseShortName(*dictionary);
   manifest.start_url = ParseStartURL(*dictionary, manifest_url, document_url);
+  manifest.display = ParseDisplay(*dictionary);
 
   return manifest;
 }
diff --git a/content/renderer/manifest/manifest_parser_unittest.cc b/content/renderer/manifest/manifest_parser_unittest.cc
index 0e500ae..2497cd2 100644
--- a/content/renderer/manifest/manifest_parser_unittest.cc
+++ b/content/renderer/manifest/manifest_parser_unittest.cc
@@ -41,6 +41,7 @@ TEST_F(ManifestParserTest, EmptyStringNull) {
   ASSERT_TRUE(manifest.name.is_null());
   ASSERT_TRUE(manifest.short_name.is_null());
   ASSERT_TRUE(manifest.start_url.is_empty());
+  ASSERT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
 }
 
 TEST_F(ManifestParserTest, ValidNoContentParses) {
@@ -51,6 +52,7 @@ TEST_F(ManifestParserTest, ValidNoContentParses) {
   ASSERT_TRUE(manifest.name.is_null());
   ASSERT_TRUE(manifest.short_name.is_null());
   ASSERT_TRUE(manifest.start_url.is_empty());
+  ASSERT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
 }
 
 TEST_F(ManifestParserTest, NameParseRules) {
@@ -58,6 +60,7 @@ TEST_F(ManifestParserTest, NameParseRules) {
   {
     Manifest manifest = ParseManifest("{ \"name\": \"foo\" }");
     ASSERT_TRUE(EqualsASCII(manifest.name.string(), "foo"));
+    ASSERT_FALSE(manifest.IsEmpty());
   }
 
   // Trim whitespaces.
@@ -84,6 +87,7 @@ TEST_F(ManifestParserTest, ShortNameParseRules) {
   {
     Manifest manifest = ParseManifest("{ \"short_name\": \"foo\" }");
     ASSERT_TRUE(EqualsASCII(manifest.short_name.string(), "foo"));
+    ASSERT_FALSE(manifest.IsEmpty());
   }
 
   // Trim whitespaces.
@@ -111,6 +115,7 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
     Manifest manifest = ParseManifest("{ \"start_url\": \"land.html\" }");
     ASSERT_EQ(manifest.start_url.spec(),
               default_document_url.Resolve("land.html").spec());
+    ASSERT_FALSE(manifest.IsEmpty());
   }
 
   // Whitespaces.
@@ -160,4 +165,67 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
   }
 }
 
+TEST_F(ManifestParserTest, DisplayParserRules) {
+  // Smoke test.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"browser\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+    EXPECT_FALSE(manifest.IsEmpty());
+  }
+
+  // Trim whitespaces.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"  browser  \" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+  }
+
+  // Don't parse if name isn't a string.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": {} }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
+  }
+
+  // Don't parse if name isn't a string.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": 42 }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
+  }
+
+  // Parse fails if string isn't known.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"browser_something\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
+  }
+
+  // Accept 'fullscreen'.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"fullscreen\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_FULLSCREEN);
+  }
+
+  // Accept 'fullscreen'.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"standalone\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_STANDALONE);
+  }
+
+  // Accept 'minimal-ui'.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"minimal-ui\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_MINIMAL_UI);
+  }
+
+  // Accept 'browser'.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"browser\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+  }
+
+  // Case insensitive.
+  {
+    Manifest manifest = ParseManifest("{ \"display\": \"BROWSER\" }");
+    EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+  }
+}
+
 } // namespace content