Report errors when parsing Manifest and expose them in developer console.

content/renderer/manifest/manifest_parser_unittest.cc

Index: content/renderer/manifest/manifest_parser_unittest.cc
diff --git a/content/renderer/manifest/manifest_parser_unittest.cc b/content/renderer/manifest/manifest_parser_unittest.cc
index 1e3770b4e74db62068f443688334c1d5824eaea0..08845336fa02aff9af4598674391fb1309929ec7 100644
--- a/content/renderer/manifest/manifest_parser_unittest.cc
+++ b/content/renderer/manifest/manifest_parser_unittest.cc
@@ -18,13 +18,22 @@ class ManifestParserTest : public testing::Test  {
   Manifest ParseManifest(const base::StringPiece& json,

[Peter Beverloo, 2014/11/27 13:24:31]

micro nit: you call this |data| elsewhere.

[mlamouri (slow - plz ping), 2014/11/27 13:49:09]

Done.

                          const GURL& document_url = default_document_url,
                          const GURL& manifest_url = default_manifest_url) {
-    return ManifestParser::Parse(json, document_url, manifest_url);
+    ManifestParser parser(json, document_url, manifest_url);
+    parser.Parse();
+    errors_ = parser.errors();
+    return parser.manifest();
+  }

[Peter Beverloo, 2014/11/27 13:24:31]

Whilst annoying (as indicated by your stumbling across chairs here), could you
add a separate method for the arguments, rather than providing defaults?

Manifest ParseManifestWithUrls(...);

[mlamouri (slow - plz ping), 2014/11/27 13:49:09]

Done.

+
+  const std::vector<std::string>& GetParseErrors() const {

[Peter Beverloo, 2014/11/27 13:24:31]

nit: this should be named errors().

[mlamouri (slow - plz ping), 2014/11/27 13:49:09]

Done.

+    return errors_;
   }
 
   static const GURL default_document_url;
   static const GURL default_manifest_url;
 
  private:
+  std::vector<std::string> errors_;
+
   DISALLOW_COPY_AND_ASSIGN(ManifestParserTest);
 };
 
@@ -36,6 +45,11 @@ const GURL ManifestParserTest::default_manifest_url(
 TEST_F(ManifestParserTest, EmptyStringNull) {
   Manifest manifest = ParseManifest("");
 
+  // This Manifest is not a valid JSON object, it's a parsing error.
+  EXPECT_EQ(1u, GetParseErrors().size());
+  EXPECT_EQ("Manifest parsing error: Line: 1, column: 1, Unexpected token.",
+            GetParseErrors()[0]);
+
   // A parsing error is equivalent to an empty manifest.
   ASSERT_TRUE(manifest.IsEmpty());
   ASSERT_TRUE(manifest.name.is_null());
@@ -48,6 +62,9 @@ TEST_F(ManifestParserTest, EmptyStringNull) {
 TEST_F(ManifestParserTest, ValidNoContentParses) {
   Manifest manifest = ParseManifest("{}");
 
+  // Empty Manifest is not a parsing error.
+  EXPECT_EQ(0u, GetParseErrors().size());
+
   // Check that all the fields are null in that case.
   ASSERT_TRUE(manifest.IsEmpty());
   ASSERT_TRUE(manifest.name.is_null());
@@ -57,30 +74,66 @@ TEST_F(ManifestParserTest, ValidNoContentParses) {
   ASSERT_EQ(manifest.orientation, blink::WebScreenOrientationLockDefault);
 }
 
+TEST_F(ManifestParserTest, MultipleErrorsReporting) {
+  Manifest manifest = ParseManifest("{ \"name\": 42, \"short_name\": 4,"
+      "\"orientation\": {}, \"display\": \"foo\", \"start_url\": null,"
+      "\"icons\": {} }");
+
+  EXPECT_EQ(6u, GetParseErrors().size());
+
+  EXPECT_EQ("Manifest parsing error: property 'name' ignored,"
+            " type string expected.",
+            GetParseErrors()[0]);
+  EXPECT_EQ("Manifest parsing error: property 'short_name' ignored,"
+            " type string expected.",
+            GetParseErrors()[1]);
+  EXPECT_EQ("Manifest parsing error: property 'start_url' ignored,"
+            " type string expected.",
+            GetParseErrors()[2]);
+  EXPECT_EQ("Manifest parsing error: unknown 'display' value ignored.",
+            GetParseErrors()[3]);
+  EXPECT_EQ("Manifest parsing error: property 'orientation' ignored,"
+            " type string expected.",
+            GetParseErrors()[4]);
+  EXPECT_EQ("Manifest parsing error: property 'icons' ignored, "
+            "type array expected.",
+            GetParseErrors()[5]);
+}
+
 TEST_F(ManifestParserTest, NameParseRules) {
   // Smoke test.
   {
     Manifest manifest = ParseManifest("{ \"name\": \"foo\" }");
     ASSERT_TRUE(EqualsASCII(manifest.name.string(), "foo"));
     ASSERT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
   {
     Manifest manifest = ParseManifest("{ \"name\": \"  foo  \" }");
     ASSERT_TRUE(EqualsASCII(manifest.name.string(), "foo"));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"name\": {} }");
     ASSERT_TRUE(manifest.name.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'name' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"name\": 42 }");
     ASSERT_TRUE(manifest.name.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'name' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 }
 
@@ -90,24 +143,34 @@ TEST_F(ManifestParserTest, ShortNameParseRules) {
     Manifest manifest = ParseManifest("{ \"short_name\": \"foo\" }");
     ASSERT_TRUE(EqualsASCII(manifest.short_name.string(), "foo"));
     ASSERT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
   {
     Manifest manifest = ParseManifest("{ \"short_name\": \"  foo  \" }");
     ASSERT_TRUE(EqualsASCII(manifest.short_name.string(), "foo"));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"short_name\": {} }");
     ASSERT_TRUE(manifest.short_name.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'short_name' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"short_name\": 42 }");
     ASSERT_TRUE(manifest.short_name.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'short_name' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 }
 
@@ -118,6 +181,7 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
     ASSERT_EQ(manifest.start_url.spec(),
               default_document_url.Resolve("land.html").spec());
     ASSERT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Whitespaces.
@@ -125,18 +189,27 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
     Manifest manifest = ParseManifest("{ \"start_url\": \"  land.html  \" }");
     ASSERT_EQ(manifest.start_url.spec(),
               default_document_url.Resolve("land.html").spec());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"start_url\": {} }");
     ASSERT_TRUE(manifest.start_url.is_empty());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'start_url' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"start_url\": 42 }");
     ASSERT_TRUE(manifest.start_url.is_empty());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'start_url' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Absolute start_url, same origin with document.
@@ -146,6 +219,7 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
                       GURL("http://foo.com/manifest.json"),
                       GURL("http://foo.com/index.html"));
     ASSERT_EQ(manifest.start_url.spec(), "http://foo.com/land.html");
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Absolute start_url, cross origin with document.
@@ -155,6 +229,10 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
                       GURL("http://foo.com/manifest.json"),
                       GURL("http://foo.com/index.html"));
     ASSERT_TRUE(manifest.start_url.is_empty());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'start_url' ignored, should "
+              "be same origin as Document.",
+              GetParseErrors()[0]);
   }
 
   // Resolving has to happen based on the manifest_url.
@@ -164,6 +242,7 @@ TEST_F(ManifestParserTest, StartURLParseRules) {
                       GURL("http://foo.com/landing/manifest.json"),
                       GURL("http://foo.com/index.html"));
     ASSERT_EQ(manifest.start_url.spec(), "http://foo.com/landing/land.html");
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 }
 
@@ -173,60 +252,78 @@ TEST_F(ManifestParserTest, DisplayParserRules) {
     Manifest manifest = ParseManifest("{ \"display\": \"browser\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
     EXPECT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"  browser  \" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"display\": {} }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'display' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"display\": 42 }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'display' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Parse fails if string isn't known.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"browser_something\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_UNSPECIFIED);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: unknown 'display' value ignored.",
+              GetParseErrors()[0]);
   }
 
   // Accept 'fullscreen'.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"fullscreen\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_FULLSCREEN);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'fullscreen'.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"standalone\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_STANDALONE);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'minimal-ui'.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"minimal-ui\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_MINIMAL_UI);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'browser'.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"browser\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Case insensitive.
   {
     Manifest manifest = ParseManifest("{ \"display\": \"BROWSER\" }");
     EXPECT_EQ(manifest.display, Manifest::DISPLAY_MODE_BROWSER);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 }
 
@@ -236,48 +333,64 @@ TEST_F(ManifestParserTest, OrientationParserRules) {
     Manifest manifest = ParseManifest("{ \"orientation\": \"natural\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockNatural);
     EXPECT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"natural\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockNatural);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": {} }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockDefault);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'orientation' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if name isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": 42 }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockDefault);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'orientation' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Parse fails if string isn't known.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"naturalish\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockDefault);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: unknown 'orientation' value ignored.",
+              GetParseErrors()[0]);
   }
 
   // Accept 'any'.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"any\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockAny);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'natural'.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"natural\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockNatural);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'landscape'.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"landscape\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockLandscape);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'landscape-primary'.
@@ -286,6 +399,7 @@ TEST_F(ManifestParserTest, OrientationParserRules) {
         ParseManifest("{ \"orientation\": \"landscape-primary\" }");
     EXPECT_EQ(manifest.orientation,
               blink::WebScreenOrientationLockLandscapePrimary);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'landscape-secondary'.
@@ -294,12 +408,14 @@ TEST_F(ManifestParserTest, OrientationParserRules) {
         ParseManifest("{ \"orientation\": \"landscape-secondary\" }");
     EXPECT_EQ(manifest.orientation,
               blink::WebScreenOrientationLockLandscapeSecondary);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'portrait'.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"portrait\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockPortrait);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'portrait-primary'.
@@ -308,6 +424,7 @@ TEST_F(ManifestParserTest, OrientationParserRules) {
       ParseManifest("{ \"orientation\": \"portrait-primary\" }");
     EXPECT_EQ(manifest.orientation,
               blink::WebScreenOrientationLockPortraitPrimary);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Accept 'portrait-secondary'.
@@ -316,12 +433,14 @@ TEST_F(ManifestParserTest, OrientationParserRules) {
         ParseManifest("{ \"orientation\": \"portrait-secondary\" }");
     EXPECT_EQ(manifest.orientation,
               blink::WebScreenOrientationLockPortraitSecondary);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Case insensitive.
   {
     Manifest manifest = ParseManifest("{ \"orientation\": \"LANDSCAPE\" }");
     EXPECT_EQ(manifest.orientation, blink::WebScreenOrientationLockLandscape);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 }
 
@@ -331,6 +450,7 @@ TEST_F(ManifestParserTest, IconsParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [] }");
     EXPECT_EQ(manifest.icons.size(), 0u);
     EXPECT_TRUE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Smoke test: if empty icon, empty list.
@@ -338,6 +458,7 @@ TEST_F(ManifestParserTest, IconsParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ {} ] }");
     EXPECT_EQ(manifest.icons.size(), 0u);
     EXPECT_TRUE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Smoke test: icon with invalid src, empty list.
@@ -345,6 +466,7 @@ TEST_F(ManifestParserTest, IconsParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ { \"icons\": [] } ] }");
     EXPECT_EQ(manifest.icons.size(), 0u);
     EXPECT_TRUE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Smoke test: if icon with empty src, it will be present in the list.
@@ -353,6 +475,7 @@ TEST_F(ManifestParserTest, IconsParseRules) {
     EXPECT_EQ(manifest.icons.size(), 1u);
     EXPECT_EQ(manifest.icons[0].src.spec(), "http://foo.com/index.html");
     EXPECT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Smoke test: if one icons with valid src, it will be present in the list.
@@ -362,6 +485,7 @@ TEST_F(ManifestParserTest, IconsParseRules) {
     EXPECT_EQ(manifest.icons.size(), 1u);
     EXPECT_EQ(manifest.icons[0].src.spec(), "http://foo.com/foo.jpg");
     EXPECT_FALSE(manifest.IsEmpty());
+    EXPECT_EQ(0u, GetParseErrors().size());

[Peter Beverloo, 2014/11/27 13:24:32]

nit: since you have the "GetParseErrors().size()" call 79 times, it'd make sense
to just have a "GetErrorCount()" method.

[mlamouri (slow - plz ping), 2014/11/27 13:49:09]

Done.

   }
 }
 
@@ -372,6 +496,7 @@ TEST_F(ManifestParserTest, IconSrcParseRules) {
         ParseManifest("{ \"icons\": [ {\"src\": \"foo.png\" } ] }");
     EXPECT_EQ(manifest.icons[0].src.spec(),
               default_document_url.Resolve("foo.png").spec());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Whitespaces.
@@ -380,18 +505,27 @@ TEST_F(ManifestParserTest, IconSrcParseRules) {
         ParseManifest("{ \"icons\": [ {\"src\": \"   foo.png   \" } ] }");
     EXPECT_EQ(manifest.icons[0].src.spec(),
               default_document_url.Resolve("foo.png").spec());
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": {} } ] }");
     EXPECT_TRUE(manifest.icons.empty());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'src' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": 42 } ] }");
     EXPECT_TRUE(manifest.icons.empty());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'src' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Resolving has to happen based on the document_url.
@@ -401,6 +535,7 @@ TEST_F(ManifestParserTest, IconSrcParseRules) {
                       GURL("http://foo.com/landing/index.html"));
     EXPECT_EQ(manifest.icons[0].src.spec(),
               "http://foo.com/landing/icons/foo.png");
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 }
 
@@ -410,6 +545,7 @@ TEST_F(ManifestParserTest, IconTypeParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"type\": \"foo\" } ] }");
     EXPECT_TRUE(EqualsASCII(manifest.icons[0].type.string(), "foo"));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
@@ -417,6 +553,7 @@ TEST_F(ManifestParserTest, IconTypeParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
                                       " \"type\": \"  foo  \" } ] }");
     EXPECT_TRUE(EqualsASCII(manifest.icons[0].type.string(), "foo"));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if property isn't a string.
@@ -424,6 +561,10 @@ TEST_F(ManifestParserTest, IconTypeParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"type\": {} } ] }");
     EXPECT_TRUE(manifest.icons[0].type.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'type' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Don't parse if property isn't a string.
@@ -431,6 +572,10 @@ TEST_F(ManifestParserTest, IconTypeParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"type\": 42 } ] }");
     EXPECT_TRUE(manifest.icons[0].type.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'type' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 }
 
@@ -440,6 +585,7 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": 42 } ] }");
     EXPECT_EQ(manifest.icons[0].density, 42);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Decimal value.
@@ -447,6 +593,7 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": 2.5 } ] }");
     EXPECT_EQ(manifest.icons[0].density, 2.5);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Parse fail if it isn't a float.
@@ -454,6 +601,10 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": {} } ] }");
     EXPECT_EQ(manifest.icons[0].density, Manifest::Icon::kDefaultDensity);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: icon 'density' ignored, "
+              "must be float greater than 0.",
+              GetParseErrors()[0]);
   }
 
   // Parse fail if it isn't a float.
@@ -461,6 +612,10 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\":\"2\" } ] }");
     EXPECT_EQ(manifest.icons[0].density, Manifest::Icon::kDefaultDensity);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: icon 'density' ignored, "
+              "must be float greater than 0.",
+              GetParseErrors()[0]);
   }
 
   // Edge case: 1.0.
@@ -468,6 +623,7 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": 1.00 } ] }");
     EXPECT_EQ(manifest.icons[0].density, 1);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Edge case: values between 0.0 and 1.0 are allowed.
@@ -475,6 +631,7 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": 0.42 } ] }");
     EXPECT_EQ(manifest.icons[0].density, 0.42);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // 0 is an invalid value.
@@ -482,6 +639,10 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": 0.0 } ] }");
     EXPECT_EQ(manifest.icons[0].density, Manifest::Icon::kDefaultDensity);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: icon 'density' ignored, "
+              "must be float greater than 0.",
+              GetParseErrors()[0]);
   }
 
   // Negative values are invalid.
@@ -489,6 +650,10 @@ TEST_F(ManifestParserTest, IconDensityParseRules) {
     Manifest manifest =
         ParseManifest("{ \"icons\": [ {\"src\": \"\", \"density\": -2.5 } ] }");
     EXPECT_EQ(manifest.icons[0].density, Manifest::Icon::kDefaultDensity);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: icon 'density' ignored, "
+              "must be float greater than 0.",
+              GetParseErrors()[0]);
   }
 }
 
@@ -498,6 +663,7 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
         "\"sizes\": \"42x42\" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes.size(), 1u);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
@@ -505,20 +671,29 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
         "\"sizes\": \"  42x42  \" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes.size(), 1u);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
-  // Don't parse if name isn't a string.
+  // Ignore sizes if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
         "\"sizes\": {} } ] }");
     EXPECT_EQ(manifest.icons[0].sizes.size(), 0u);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'sizes' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
-  // Don't parse if name isn't a string.
+  // Ignore sizes if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
         "\"sizes\": 42 } ] }");
     EXPECT_EQ(manifest.icons[0].sizes.size(), 0u);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'sizes' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 
   // Smoke test: value correctly parsed.
@@ -527,6 +702,7 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
         "\"sizes\": \"42x42  48x48\" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes[0], gfx::Size(42, 42));
     EXPECT_EQ(manifest.icons[0].sizes[1], gfx::Size(48, 48));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // <WIDTH>'x'<HEIGHT> and <WIDTH>'X'<HEIGHT> are equivalent.
@@ -535,6 +711,7 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
         "\"sizes\": \"42X42  48X48\" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes[0], gfx::Size(42, 42));
     EXPECT_EQ(manifest.icons[0].sizes[1], gfx::Size(48, 48));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Twice the same value is parsed twice.
@@ -543,6 +720,7 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
         "\"sizes\": \"42X42  42x42\" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes[0], gfx::Size(42, 42));
     EXPECT_EQ(manifest.icons[0].sizes[1], gfx::Size(42, 42));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Width or height can't start with 0.
@@ -550,6 +728,9 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
         "\"sizes\": \"004X007  042x00\" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes.size(), 0u);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: found icon with no valid size.",
+              GetParseErrors()[0]);
   }
 
   // Width and height MUST contain digits.
@@ -557,6 +738,9 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
     Manifest manifest = ParseManifest("{ \"icons\": [ {\"src\": \"\","
         "\"sizes\": \"e4X1.0  55ax1e10\" } ] }");
     EXPECT_EQ(manifest.icons[0].sizes.size(), 0u);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: found icon with no valid size.",
+              GetParseErrors()[0]);
   }
 
   // 'any' is correctly parsed and transformed to gfx::Size(0,0).
@@ -569,6 +753,7 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
     EXPECT_EQ(manifest.icons[0].sizes[1], any);
     EXPECT_EQ(manifest.icons[0].sizes[2], any);
     EXPECT_EQ(manifest.icons[0].sizes[3], any);
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Some invalid width/height combinations.
@@ -577,6 +762,9 @@ TEST_F(ManifestParserTest, IconSizesParseRules) {
         "\"sizes\": \"x 40xx 1x2x3 x42 42xx42\" } ] }");
     gfx::Size any = gfx::Size(0, 0);
     EXPECT_EQ(manifest.icons[0].sizes.size(), 0u);
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: found icon with no valid size.",
+              GetParseErrors()[0]);
   }
 }
 
@@ -585,22 +773,32 @@ TEST_F(ManifestParserTest, GCMSenderIDParseRules) {
   {
     Manifest manifest = ParseManifest("{ \"gcm_sender_id\": \"foo\" }");
     EXPECT_TRUE(EqualsASCII(manifest.gcm_sender_id.string(), "foo"));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Trim whitespaces.
   {
     Manifest manifest = ParseManifest("{ \"gcm_sender_id\": \"  foo  \" }");
     EXPECT_TRUE(EqualsASCII(manifest.gcm_sender_id.string(), "foo"));
+    EXPECT_EQ(0u, GetParseErrors().size());
   }
 
   // Don't parse if property isn't a string.
   {
     Manifest manifest = ParseManifest("{ \"gcm_sender_id\": {} }");
     EXPECT_TRUE(manifest.gcm_sender_id.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'gcm_sender_id' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
   {
     Manifest manifest = ParseManifest("{ \"gcm_sender_id\": 42 }");
     EXPECT_TRUE(manifest.gcm_sender_id.is_null());
+    EXPECT_EQ(1u, GetParseErrors().size());
+    EXPECT_EQ("Manifest parsing error: property 'gcm_sender_id' ignored,"
+              " type string expected.",
+              GetParseErrors()[0]);
   }
 }