Don't bail early on extensions update.

When parsing the extension update manifest results, we bail out of there was an
error from any of the <app> tags in the xml. With the changes from

http://src.chromium.org/viewvc/chrome?view=rev&revision=42899

we now can get a "error-unknownApplication" for ids that Omaha doesn't know
about, so we need to not have that stop other updates.


BUG=40519
TEST=Install an extension you packaged yourself that doesn't have an 
update_url specified, and install an older verison of one in the gallery. When
you do an "update now" you should get the update for the gallery one.


Review URL: http://codereview.chromium.org/1633002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43891 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 49 insertions, 8 deletions

diff --git a/chrome/common/extensions/update_manifest.cc b/chrome/common/extensions/update_manifest.cc
index e461d7b..4394a7b 100644
--- a/chrome/common/extensions/update_manifest.cc
+++ b/chrome/common/extensions/update_manifest.cc
@@ -188,8 +188,10 @@ static bool ParseSingleAppTag(xmlNode* app_node, xmlNs* xml_namespace,
 bool UpdateManifest::Parse(const std::string& manifest_xml) {
   results_.list.resize(0);
   results_.daystart_elapsed_seconds = kNoDaystart;
+  errors_ = "";
 
   if (manifest_xml.length() < 1) {
+     ParseError("Empty xml");
     return false;
   }
 
@@ -247,9 +249,9 @@ bool UpdateManifest::Parse(const std::string& manifest_xml) {
     std::string error;
     if (!ParseSingleAppTag(apps[i], gupdate_ns, &current, &error)) {
       ParseError("%s", error.c_str());
-      return false;
+    } else {
+      results_.list.push_back(current);
     }
-    results_.list.push_back(current);
   }
 
   return true;
diff --git a/chrome/common/extensions/update_manifest.h b/chrome/common/extensions/update_manifest.h
index be0f643..81bd793 100644
--- a/chrome/common/extensions/update_manifest.h
+++ b/chrome/common/extensions/update_manifest.h
@@ -60,8 +60,8 @@ class UpdateManifest {
 
   // Parses an update manifest xml string into Result data. Returns a bool
   // indicating success or failure. On success, the results are available by
-  // calling results(), and on failure, the explanation for why is available
-  // by calling errors().
+  // calling results(). The details for any failures are available by calling
+  // errors().
   bool Parse(const std::string& manifest_xml);
 
   const Results& results() { return results_; }
diff --git a/chrome/common/extensions/update_manifest_unittest.cc b/chrome/common/extensions/update_manifest_unittest.cc
index fb7ee0e9..c38c435 100644
--- a/chrome/common/extensions/update_manifest_unittest.cc
+++ b/chrome/common/extensions/update_manifest_unittest.cc
@@ -105,18 +105,44 @@ static const char* kNoUpdate =
 " </app>"
 "</gupdate>";
 
+// Includes two <app> tags, one with an error.
+static const char* kTwoAppsOneError =
+"<?xml version='1.0' encoding='UTF-8'?>"
+"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
+" <app appid='aaaaaaaa' status='error-unknownApplication'>"
+"  <updatecheck status='error-unknownapplication'/>"
+" </app>"
+" <app appid='bbbbbbbb'>"
+"  <updatecheck codebase='http://example.com/b_3.1.crx' version='3.1'/>"
+" </app>"
+"</gupdate>";
+
 TEST(ExtensionUpdateManifestTest, TestUpdateManifest) {
   UpdateManifest parser;
 
   // Test parsing of a number of invalid xml cases
   EXPECT_FALSE(parser.Parse(""));
-  EXPECT_FALSE(parser.Parse(kMissingAppId));
-  EXPECT_FALSE(parser.Parse(kInvalidCodebase));
-  EXPECT_FALSE(parser.Parse(kMissingVersion));
-  EXPECT_FALSE(parser.Parse(kInvalidVersion));
+  EXPECT_FALSE(parser.errors().empty());
+
+  EXPECT_TRUE(parser.Parse(kMissingAppId));
+  EXPECT_TRUE(parser.results().list.empty());
+  EXPECT_FALSE(parser.errors().empty());
+
+  EXPECT_TRUE(parser.Parse(kInvalidCodebase));
+  EXPECT_TRUE(parser.results().list.empty());
+  EXPECT_FALSE(parser.errors().empty());
+
+  EXPECT_TRUE(parser.Parse(kMissingVersion));
+  EXPECT_TRUE(parser.results().list.empty());
+  EXPECT_FALSE(parser.errors().empty());
+
+  EXPECT_TRUE(parser.Parse(kInvalidVersion));
+  EXPECT_TRUE(parser.results().list.empty());
+  EXPECT_FALSE(parser.errors().empty());
 
   // Parse some valid XML, and check that all params came out as expected
   EXPECT_TRUE(parser.Parse(kValidXml));
+  EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
   const UpdateManifest::Result* firstResult = &parser.results().list.at(0);
   EXPECT_EQ(GURL("http://example.com/extension_1.2.3.4.crx"),
@@ -127,23 +153,36 @@ TEST(ExtensionUpdateManifestTest, TestUpdateManifest) {
 
   // Parse some xml that uses namespace prefixes.
   EXPECT_TRUE(parser.Parse(kUsesNamespacePrefix));
+  EXPECT_TRUE(parser.errors().empty());
   EXPECT_TRUE(parser.Parse(kSimilarTagnames));
+  EXPECT_TRUE(parser.errors().empty());
   xmlCleanupGlobals();
 
   // Parse xml with hash value
   EXPECT_TRUE(parser.Parse(valid_xml_with_hash));
+  EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
   firstResult = &parser.results().list.at(0);
   EXPECT_EQ("1234", firstResult->package_hash);
 
+  // Parse xml with a <daystart> element.
   EXPECT_TRUE(parser.Parse(kWithDaystart));
+  EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
   EXPECT_EQ(parser.results().daystart_elapsed_seconds, 456);
 
   // Parse a no-update response.
   EXPECT_TRUE(parser.Parse(kNoUpdate));
+  EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
   firstResult = &parser.results().list.at(0);
   EXPECT_EQ(firstResult->extension_id, "12345");
   EXPECT_EQ(firstResult->version, "");
+
+  // Parse xml with one error and one success <app> tag.
+  EXPECT_TRUE(parser.Parse(kTwoAppsOneError));
+  EXPECT_FALSE(parser.errors().empty());
+  EXPECT_EQ(1u, parser.results().list.size());
+  firstResult = &parser.results().list.at(0);
+  EXPECT_EQ(firstResult->extension_id, "bbbbbbbb");
 }