Do extensions update manifest XML parsing in a sandboxed process.

This involves moving the xml parsing code from static functions in 
extension_updater.cc to a UpdateManifest class, and switching from 
logging any errors directly to collecting them up and passing them 
across the IPC channel.


BUG=http://crbug.com/12677
TEST=extensions auto-update should still work correctly


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

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

7 files changed, 222 insertions, 423 deletions

diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index ad3e6f3..fe19ba5 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -11,22 +11,24 @@
 #include "base/file_util.h"
 #include "base/file_version_info.h"
 #include "base/rand_util.h"
+#include "base/scoped_vector.h"
 #include "base/sha2.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "base/thread.h"
+#include "base/version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/utility_process_host.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
-#include "chrome/common/libxml_utils.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/escape.h"
 #include "net/url_request/url_request_status.h"
-#include "libxml/tree.h"
 
 using base::RandDouble;
 using base::RandInt;
@@ -36,17 +38,13 @@ using prefs::kExtensionBlacklistUpdateVersion;
 using prefs::kLastExtensionsUpdateCheck;
 using prefs::kNextExtensionsUpdateCheck;
 
-const char* ExtensionUpdater::kExpectedGupdateProtocol = "2.0";
-const char* ExtensionUpdater::kExpectedGupdateXmlns =
-    "http://www.google.com/update2/response";
-
 // NOTE: HTTPS is used here to ensure the response from omaha can be trusted.
 // The response contains a url for fetching the blacklist and a hash value
 // for validation.
 const char* ExtensionUpdater::kBlacklistUpdateUrl =
     "https://clients2.google.com/service/update2/crx";
 
-// Update AppID for extesnion blacklist.
+// Update AppID for extension blacklist.
 const char* ExtensionUpdater::kBlacklistAppID = "com.google.crx.blacklist";
 
 // Wait at least 5 minutes after browser startup before we do any checks. If you
@@ -111,9 +109,10 @@ class ExtensionUpdaterFileHandler
 ExtensionUpdater::ExtensionUpdater(ExtensionUpdateService* service,
                                    PrefService* prefs,
                                    int frequency_seconds,
-                                   MessageLoop* file_io_loop)
+                                   MessageLoop* file_io_loop,
+                                   MessageLoop* io_loop)
     : service_(service), frequency_seconds_(frequency_seconds),
-      file_io_loop_(file_io_loop), prefs_(prefs),
+      file_io_loop_(file_io_loop), io_loop_(io_loop), prefs_(prefs),
       file_handler_(new ExtensionUpdaterFileHandler(MessageLoop::current(),
                                                     file_io_loop_)) {
   Init();
@@ -221,6 +220,77 @@ void ExtensionUpdater::OnURLFetchComplete(
   }
 }
 
+// Utility class to handle doing xml parsing in a sandboxed utility process.
+class SafeManifestParser : public UtilityProcessHost::Client {
+ public:
+  SafeManifestParser(const std::string& xml, ExtensionUpdater* updater,
+                     MessageLoop* updater_loop, MessageLoop* io_loop)
+      : xml_(xml), updater_loop_(updater_loop), io_loop_(io_loop),
+        updater_(updater) {
+  }
+
+  ~SafeManifestParser() {}
+
+  // Posts a task over to the IO loop to start the parsing of xml_ in a
+  // utility process.
+  void Start() {
+    DCHECK(MessageLoop::current() == updater_loop_);
+    io_loop_->PostTask(FROM_HERE,
+        NewRunnableMethod(this, &SafeManifestParser::ParseInSandbox,
+            g_browser_process->resource_dispatcher_host()));
+  }
+
+  // Creates the sandboxed utility process and tells it to start parsing.
+  void ParseInSandbox(ResourceDispatcherHost* rdh) {
+    DCHECK(MessageLoop::current() == io_loop_);
+
+    // TODO(asargent) we shouldn't need to do this branch here - instead
+    // UtilityProcessHost should handle it for us. (http://crbug.com/19192)
+    if (rdh && !CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSingleProcess)) {
+      UtilityProcessHost* host = new UtilityProcessHost(
+          rdh, this, updater_loop_);
+      host->StartUpdateManifestParse(xml_);
+    } else {
+      UpdateManifest manifest;
+      if (manifest.Parse(xml_)) {
+        updater_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+            &SafeManifestParser::OnParseUpdateManifestSucceeded,
+            manifest.results()));
+      } else {
+        updater_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+            &SafeManifestParser::OnParseUpdateManifestFailed,
+            manifest.errors()));
+      }
+    }
+  }
+
+  // Callback from the utility process when parsing succeeded.
+  virtual void OnParseUpdateManifestSucceeded(
+      const UpdateManifest::ResultList& list) {
+    DCHECK(MessageLoop::current() == updater_loop_);
+    updater_->HandleManifestResults(list);
+  }
+
+  // Callback from the utility process when parsing failed.
+  virtual void OnParseUpdateManifestFailed(const std::string& error_message) {
+    DCHECK(MessageLoop::current() == updater_loop_);
+    LOG(WARNING) << "Error parsing update manifest:\n" << error_message;
+  }
+
+ private:
+  const std::string& xml_;
+
+  // The MessageLoop we use to call back the ExtensionUpdater.
+  MessageLoop* updater_loop_;
+
+  // The MessageLoop where we create the utility process.
+  MessageLoop* io_loop_;
+
+  scoped_refptr<ExtensionUpdater> updater_;
+};
+
+
 void ExtensionUpdater::OnManifestFetchComplete(const GURL& url,
                                                const URLRequestStatus& status,
                                                int response_code,
@@ -228,17 +298,9 @@ void ExtensionUpdater::OnManifestFetchComplete(const GURL& url,
   // We want to try parsing the manifest, and if it indicates updates are
   // available, we want to fire off requests to fetch those updates.
   if (status.status() == URLRequestStatus::SUCCESS && response_code == 200) {
-    ScopedVector<ParseResult> parsed;
-    // TODO(asargent) - We should do the xml parsing in a sandboxed process.
-    // (http://crbug.com/12677).
-    if (Parse(data, &parsed.get())) {
-      std::vector<int> updates = DetermineUpdates(parsed.get());
-      for (size_t i = 0; i < updates.size(); i++) {
-        ParseResult* update = parsed[updates[i]];
-        FetchUpdatedExtension(update->extension_id, update->crx_url,
-          update->package_hash, update->version->GetString());
-      }
-    }
+    scoped_refptr<SafeManifestParser>  safe_parser =
+        new SafeManifestParser(data, this, MessageLoop::current(), io_loop_);
+    safe_parser->Start();
   } else {
     // TODO(asargent) Do exponential backoff here. (http://crbug.com/12546).
     LOG(INFO) << "Failed to fetch manifst '" << url.possibly_invalid_spec() <<
@@ -254,6 +316,16 @@ void ExtensionUpdater::OnManifestFetchComplete(const GURL& url,
   }
 }
 
+void ExtensionUpdater::HandleManifestResults(
+    const UpdateManifest::ResultList& results) {
+  std::vector<int> updates = DetermineUpdates(results);
+  for (size_t i = 0; i < updates.size(); i++) {
+    const UpdateManifest::Result* update = &(results.at(updates[i]));
+    FetchUpdatedExtension(update->extension_id, update->crx_url,
+        update->package_hash, update->version);
+  }
+}
+
 void ExtensionUpdater::ProcessBlacklist(const std::string& data) {
   // Verify sha256 hash value.
   char sha256_hash_value[base::SHA256_LENGTH];
@@ -365,7 +437,7 @@ void AppendExtensionInfo(std::string* str, const Extension& extension) {
 // Creates a blacklist update url.
 GURL ExtensionUpdater::GetBlacklistUpdateUrl(const std::wstring& version) {
   std::string blklist_info = StringPrintf("id=%s&v=%s&uc", kBlacklistAppID,
-    WideToASCII(version).c_str());
+      WideToASCII(version).c_str());
   return GURL(StringPrintf("%s?x=%s", kBlacklistUpdateUrl,
                            EscapeQueryParamValue(blklist_info).c_str()));
 }
@@ -435,206 +507,6 @@ void ExtensionUpdater::TimerFired() {
   ScheduleNextCheck(TimeDelta::FromSeconds(frequency_seconds_));
 }
 
-static void ManifestParseError(const char* details, ...) {
-  va_list args;
-  va_start(args, details);
-  std::string message("Extension update manifest parse error: ");
-  StringAppendV(&message, details, args);
-  LOG(WARNING) << message;
-}
-
-// Checks whether a given node's name matches |expected_name| and
-// |expected_namespace|.
-static bool TagNameEquals(const xmlNode* node, const char* expected_name,
-                          const xmlNs* expected_namespace) {
-  if (node->ns != expected_namespace) {
-    return false;
-  }
-  return 0 == strcmp(expected_name, reinterpret_cast<const char*>(node->name));
-}
-
-// Returns child nodes of |root| with name |name| in namespace |xml_namespace|.
-static std::vector<xmlNode*> GetChildren(xmlNode* root, xmlNs* xml_namespace,
-                                         const char* name) {
-  std::vector<xmlNode*> result;
-  for (xmlNode* child = root->children; child != NULL; child = child->next) {
-    if (!TagNameEquals(child, name, xml_namespace)) {
-      continue;
-    }
-    result.push_back(child);
-  }
-  return result;
-}
-
-// Returns the value of a named attribute, or the empty string.
-static std::string GetAttribute(xmlNode* node, const char* attribute_name) {
-  const xmlChar* name = reinterpret_cast<const xmlChar*>(attribute_name);
-  for (xmlAttr* attr = node->properties; attr != NULL; attr = attr->next) {
-    if (!xmlStrcmp(attr->name, name) && attr->children &&
-        attr->children->content) {
-      return std::string(reinterpret_cast<const char*>(
-          attr->children->content));
-    }
-  }
-  return std::string();
-}
-
-// This is used for the xml parser to report errors. This assumes the context
-// is a pointer to a std::string where the error message should be appended.
-static void XmlErrorFunc(void *context, const char *message, ...) {
-  va_list args;
-  va_start(args, message);
-  std::string* error = static_cast<std::string*>(context);
-  StringAppendV(error, message, args);
-}
-
-// Utility class for cleaning up xml parser state when leaving a scope.
-class ScopedXmlParserCleanup {
- public:
-  ScopedXmlParserCleanup() : document_(NULL) {}
-  ~ScopedXmlParserCleanup() {
-    if (document_)
-      xmlFreeDoc(document_);
-    xmlCleanupParser();
-  }
-  void set_document(xmlDocPtr document) {
-    document_ = document;
-  }
-
- private:
-  xmlDocPtr document_;
-};
-
-// Returns a pointer to the xmlNs on |node| with the |expected_href|, or
-// NULL if there isn't one with that href.
-static xmlNs* GetNamespace(xmlNode* node, const char* expected_href) {
-  const xmlChar* href = reinterpret_cast<const xmlChar*>(expected_href);
-  for (xmlNs* ns = node->ns; ns != NULL; ns = ns->next) {
-    if (ns->href && !xmlStrcmp(ns->href, href)) {
-      return ns;
-    }
-  }
-  return NULL;
-}
-
-// This is a separate sub-class so that we can have access to the private
-// ParseResult struct, but avoid making the .h file include the xml api headers.
-class ExtensionUpdater::ParseHelper {
- public:
-  // Helper function for ExtensionUpdater::Parse that reads in values for a
-  // single <app> tag. It returns a boolean indicating success or failure.
-  static bool ParseSingleAppTag(xmlNode* app_node, xmlNs* xml_namespace,
-                                ParseResult* result) {
-    // Read the extension id.
-    result->extension_id = GetAttribute(app_node, "appid");
-    if (result->extension_id.length() == 0) {
-      ManifestParseError("Missing appid on app node");
-      return false;
-    }
-
-    // Get the updatecheck node.
-    std::vector<xmlNode*> updates = GetChildren(app_node, xml_namespace,
-                                                "updatecheck");
-    if (updates.size() > 1) {
-      ManifestParseError("Too many updatecheck tags on app (expecting only 1)");
-      return false;
-    }
-    if (updates.size() == 0) {
-      ManifestParseError("Missing updatecheck on app");
-      return false;
-    }
-    xmlNode *updatecheck = updates[0];
-
-    // Find the url to the crx file.
-    result->crx_url = GURL(GetAttribute(updatecheck, "codebase"));
-    if (!result->crx_url.is_valid()) {
-      ManifestParseError("Invalid codebase url");
-      return false;
-    }
-
-    // Get the version.
-    std::string tmp = GetAttribute(updatecheck, "version");
-    if (tmp.length() == 0) {
-      ManifestParseError("Missing version for updatecheck");
-      return false;
-    }
-    result->version.reset(Version::GetVersionFromString(tmp));
-    if (!result->version.get()) {
-      ManifestParseError("Invalid version");
-      return false;
-    }
-
-    // Get the minimum browser version (not required).
-    tmp = GetAttribute(updatecheck, "prodversionmin");
-    if (tmp.length()) {
-      result->browser_min_version.reset(Version::GetVersionFromString(tmp));
-      if (!result->browser_min_version.get()) {
-        ManifestParseError("Invalid prodversionmin");
-        return false;
-      }
-    }
-
-    // package_hash is optional. It is only required for blacklist. It is a
-    // sha256 hash of the package in hex format.
-    result->package_hash = GetAttribute(updatecheck, "hash");
-    return true;
-  }
-};
-
-bool ExtensionUpdater::Parse(const std::string& manifest_xml,
-                                ParseResultList* results) {
-  std::string xml_errors;
-  ScopedXmlErrorFunc error_func(&xml_errors, &XmlErrorFunc);
-  ScopedXmlParserCleanup xml_cleanup;
-
-  xmlDocPtr document = xmlParseDoc(
-      reinterpret_cast<const xmlChar*>(manifest_xml.c_str()));
-  if (!document) {
-    ManifestParseError(xml_errors.c_str());
-    return false;
-  }
-  xml_cleanup.set_document(document);
-
-  xmlNode *root = xmlDocGetRootElement(document);
-  if (!root) {
-    ManifestParseError("Missing root node");
-    return false;
-  }
-
-  // Look for the required namespace declaration.
-  xmlNs* gupdate_ns = GetNamespace(root, kExpectedGupdateXmlns);
-  if (!gupdate_ns) {
-    ManifestParseError("Missing or incorrect xmlns on gupdate tag");
-    return false;
-  }
-
-  if (!TagNameEquals(root, "gupdate", gupdate_ns)) {
-    ManifestParseError("Missing gupdate tag");
-    return false;
-  }
-
-  // Check for the gupdate "protocol" attribute.
-  if (GetAttribute(root, "protocol") != kExpectedGupdateProtocol) {
-    ManifestParseError("Missing/incorrect protocol on gupdate tag "
-        "(expected '%s')", kExpectedGupdateProtocol);
-    return false;
-  }
-
-  // Parse each of the <app> tags.
-  ScopedVector<ParseResult> tmp_results;
-  std::vector<xmlNode*> apps = GetChildren(root, gupdate_ns, "app");
-  for (unsigned int i = 0; i < apps.size(); i++) {
-    ParseResult* current = new ParseResult();
-    tmp_results.push_back(current);
-    if (!ParseHelper::ParseSingleAppTag(apps[i], gupdate_ns, current)) {
-      return false;
-    }
-  }
-  results->insert(results->end(), tmp_results.begin(), tmp_results.end());
-  tmp_results.get().clear();
-
-  return true;
-}
 
 bool ExtensionUpdater::GetExistingVersion(const std::string& id,
                                           std::string* version) {
@@ -652,7 +524,7 @@ bool ExtensionUpdater::GetExistingVersion(const std::string& id,
 }
 
 std::vector<int> ExtensionUpdater::DetermineUpdates(
-    const ParseResultList& possible_updates) {
+    const std::vector<UpdateManifest::Result>& possible_updates) {
 
   std::vector<int> result;
 
@@ -661,22 +533,27 @@ std::vector<int> ExtensionUpdater::DetermineUpdates(
   scoped_ptr<Version> browser_version;
 
   for (size_t i = 0; i < possible_updates.size(); i++) {
-    ParseResult* update = possible_updates[i];
+    const UpdateManifest::Result* update = &possible_updates[i];
 
     std::string version;
     if (!GetExistingVersion(update->extension_id, &version)) {
       continue;
     }
+
     // If the update version is the same or older than what's already installed,
     // we don't want it.
     scoped_ptr<Version> existing_version(
-      Version::GetVersionFromString(version));
-    if (update->version.get()->CompareTo(*(existing_version.get())) <= 0) {
+        Version::GetVersionFromString(version));
+    scoped_ptr<Version> update_version(
+        Version::GetVersionFromString(update->version));
+
+    if (!update_version.get() ||
+        update_version->CompareTo(*(existing_version.get())) <= 0) {
       continue;
     }
 
     // If the update specifies a browser minimum version, do we qualify?
-    if (update->browser_min_version.get()) {
+    if (update->browser_min_version.length() > 0) {
       // First determine the browser version if we haven't already.
       if (!browser_version.get()) {
         scoped_ptr<FileVersionInfo> version_info(
@@ -686,13 +563,16 @@ std::vector<int> ExtensionUpdater::DetermineUpdates(
             version_info->product_version()));
         }
       }
-      if (browser_version.get() &&
-          update->browser_min_version->CompareTo(*browser_version.get()) > 0) {
+      scoped_ptr<Version> browser_min_version(
+          Version::GetVersionFromString(update->browser_min_version));
+      if (browser_version.get() && browser_min_version.get() &&
+          browser_min_version->CompareTo(*browser_version.get()) > 0) {
         // TODO(asargent) - We may want this to show up in the extensions UI
         // eventually. (http://crbug.com/12547).
         LOG(WARNING) << "Updated version of extension " << update->extension_id
           << " available, but requires chrome version "
-          << update->browser_min_version->GetString();
+          << update->browser_min_version;
+
         continue;
       }
     }
diff --git a/chrome/browser/extensions/extension_updater.h b/chrome/browser/extensions/extension_updater.h
index c450e8e..c82d1c4 100644
--- a/chrome/browser/extensions/extension_updater.h
+++ b/chrome/browser/extensions/extension_updater.h
@@ -12,13 +12,12 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
-#include "base/scoped_vector.h"
 #include "base/task.h"
 #include "base/time.h"
 #include "base/timer.h"
-#include "base/version.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/net/url_fetcher.h"
+#include "chrome/common/extensions/update_manifest.h"
 #include "googleurl/src/gurl.h"
 
 class Extension;
@@ -32,7 +31,8 @@ class PrefService;
 // ExtensionUpdater* updater = new ExtensionUpdater(my_extensions_service,
 //                                                  pref_service,
 //                                                  update_frequency_secs,
-//                                                  file_io_loop);
+//                                                  file_io_loop,
+//                                                  io_loop);
 // updater.Start();
 // ....
 // updater.Stop();
@@ -46,7 +46,8 @@ class ExtensionUpdater
   ExtensionUpdater(ExtensionUpdateService* service,
                    PrefService* prefs,
                    int frequency_seconds,
-                   MessageLoop* file_io_loop);
+                   MessageLoop* file_io_loop,
+                   MessageLoop* io_loop);
 
   virtual ~ExtensionUpdater();
 
@@ -60,34 +61,8 @@ class ExtensionUpdater
  private:
   friend class ExtensionUpdaterTest;
   friend class ExtensionUpdaterFileHandler;
-  class ParseHelper;
-
-  // An update manifest looks like this:
-  //
-  // <?xml version='1.0' encoding='UTF-8'?>
-  // <gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>
-  //  <app appid='12345'>
-  //   <updatecheck codebase='http://example.com/extension_1.2.3.4.crx'
-  //                version='1.2.3.4' prodversionmin='2.0.143.0'
-  //                hash="12345"/>
-  //  </app>
-  // </gupdate>
-  //
-  // The "appid" attribute of the <app> tag refers to the unique id of the
-  // extension. The "codebase" attribute of the <updatecheck> tag is the url to
-  // fetch the updated crx file, and the "prodversionmin" attribute refers to
-  // the minimum version of the chrome browser that the update applies to.
-  // The hash is only required for blacklist. It is a sha256 hash value against
-  // the payload in hex format.
-
-  // The result of parsing one <app> tag in an xml update check manifest.
-  struct ParseResult {
-    std::string extension_id;
-    scoped_ptr<Version> version;
-    scoped_ptr<Version> browser_min_version;
-    std::string package_hash;
-    GURL crx_url;
-  };
+  friend class SafeManifestParser;
+
 
   // We need to keep track of some information associated with a url
   // when doing a fetch.
@@ -107,10 +82,6 @@ class ExtensionUpdater
   static const int kManifestFetcherId = 1;
   static const int kExtensionFetcherId = 1;
 
-  // Constants for the update manifest.
-  static const char* kExpectedGupdateProtocol;
-  static const char* kExpectedGupdateXmlns;
-
   static const char* kBlacklistUpdateUrl;
   static const char* kBlacklistAppID;
 
@@ -165,20 +136,17 @@ class ExtensionUpdater
   void FetchUpdatedExtension(const std::string& id, const GURL& url,
     const std::string& hash, const std::string& version);
 
+  // Once a manifest is parsed, this starts fetches of any relevant crx files.
+  void HandleManifestResults(const UpdateManifest::ResultList& results);
+
   // Determines the version of an existing extension.
   // Returns true on success and false on failures.
   bool GetExistingVersion(const std::string& id, std::string* version);
 
-  typedef std::vector<ParseResult*> ParseResultList;
-
   // Given a list of potential updates, returns the indices of the ones that are
   // applicable (are actually a new version, etc.) in |result|.
-  std::vector<int> DetermineUpdates(const ParseResultList& possible_updates);
-
-  // Parses an update manifest xml string into ParseResult data. On success, it
-  // inserts new ParseResult items into |result| and returns true. On failure,
-  // it returns false and puts nothing into |result|.
-  static bool Parse(const std::string& manifest_xml, ParseResultList* result);
+  std::vector<int> DetermineUpdates(
+      const std::vector<UpdateManifest::Result>& possible_updates);
 
   // Creates a blacklist update url.
   static GURL GetBlacklistUpdateUrl(const std::wstring& version);
@@ -204,6 +172,9 @@ class ExtensionUpdater
   // The MessageLoop where we should do file I/O.
   MessageLoop* file_io_loop_;
 
+  // The IO loop for IPC.
+  MessageLoop* io_loop_;
+
   PrefService* prefs_;
 
   scoped_refptr<ExtensionUpdaterFileHandler> file_handler_;
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 9a55af1..724d736 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -6,7 +6,9 @@
 
 #include "base/file_util.h"
 #include "base/rand_util.h"
+#include "base/stl_util-inl.h"
 #include "base/string_util.h"
+#include "base/thread.h"
 #include "chrome/browser/extensions/extension_updater.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/net/test_url_fetcher_factory.h"
@@ -18,83 +20,8 @@
 #include "net/base/escape.h"
 #include "net/url_request/url_request_status.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "libxml/globals.h"
 
-const char *valid_xml =
-"<?xml version='1.0' encoding='UTF-8'?>"
-"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
-" <app appid='12345'>"
-"  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx'"
-"               version='1.2.3.4' prodversionmin='2.0.143.0' />"
-" </app>"
-"</gupdate>";
-
-const char *valid_xml_with_hash =
-"<?xml version='1.0' encoding='UTF-8'?>"
-"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
-" <app appid='12345'>"
-"  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx'"
-"               version='1.2.3.4' prodversionmin='2.0.143.0' "
-"               hash='1234'/>"
-" </app>"
-"</gupdate>";
-
-const char* missing_appid =
-"<?xml version='1.0'?>"
-"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
-" <app>"
-"  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx'"
-"               version='1.2.3.4' />"
-" </app>"
-"</gupdate>";
-
-const char* invalid_codebase =
-"<?xml version='1.0'?>"
-"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
-" <app appid='12345' status='ok'>"
-"  <updatecheck codebase='example.com/extension_1.2.3.4.crx'"
-"               version='1.2.3.4' />"
-" </app>"
-"</gupdate>";
-
-const char* missing_version =
-"<?xml version='1.0'?>"
-"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
-" <app appid='12345' status='ok'>"
-"  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx' />"
-" </app>"
-"</gupdate>";
-
-const char* invalid_version =
-"<?xml version='1.0'?>"
-"<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>"
-" <app appid='12345' status='ok'>"
-"  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx' "
-"               version='1.2.3.a'/>"
-" </app>"
-"</gupdate>";
-
-const char *uses_namespace_prefix =
-"<?xml version='1.0' encoding='UTF-8'?>"
-"<g:gupdate xmlns:g='http://www.google.com/update2/response' protocol='2.0'>"
-" <g:app appid='12345'>"
-"  <g:updatecheck codebase='http://example.com/extension_1.2.3.4.crx'"
-"               version='1.2.3.4' prodversionmin='2.0.143.0' />"
-" </g:app>"
-"</g:gupdate>";
-
-// Includes unrelated <app> tags from other xml namespaces - this should
-// not cause problems.
-const char *similar_tagnames =
-"<?xml version='1.0' encoding='UTF-8'?>"
-"<gupdate xmlns='http://www.google.com/update2/response'"
-"         xmlns:a='http://a' protocol='2.0'>"
-" <a:app/>"
-" <b:app xmlns:b='http://b' />"
-" <app appid='12345'>"
-"  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx'"
-"               version='1.2.3.4' prodversionmin='2.0.143.0' />"
-" </app>"
-"</gupdate>";
 
 
 // Do-nothing base class for further specialized test classes.
@@ -205,12 +132,22 @@ class ServiceForDownloadTests : public MockService {
     install_path_ = extension_path;
   }
 
+  virtual Extension* GetExtensionById(const std::string& id) {
+    last_inquired_extension_id_ = id;
+    return NULL;
+  }
+
   const std::string& extension_id() { return extension_id_; }
   const FilePath& install_path() { return install_path_; }
+  const std::string& last_inquired_extension_id() {
+    return last_inquired_extension_id_;
+  }
 
  private:
   std::string extension_id_;
   FilePath install_path_;
+  // The last extension_id that GetExtensionById was called with.
+  std::string last_inquired_extension_id_;
 };
 
 class ServiceForBlacklistTests : public MockService {
@@ -261,10 +198,7 @@ static void ExtractParameters(const std::string& params,
 // inside this class (which is a friend to ExtensionUpdater).
 class ExtensionUpdaterTest : public testing::Test {
  public:
-  static void expectParseFailure(const char *xml) {
-    ScopedVector<ExtensionUpdater::ParseResult> result;
-    EXPECT_FALSE(ExtensionUpdater::Parse(xml, &result.get()));
-  }
+
 
   static void SimulateTimerFired(ExtensionUpdater* updater) {
     EXPECT_TRUE(updater->timer_.IsRunning());
@@ -272,52 +206,17 @@ class ExtensionUpdaterTest : public testing::Test {
     updater->TimerFired();
   }
 
-  // Make a test ParseResult
-  static ExtensionUpdater::ParseResult* MakeParseResult(
+  // Adds a Result with the given data to results.
+  static void AddParseResult(
       const std::string& id,
       const std::string& version,
-      const std::string& url) {
-    ExtensionUpdater::ParseResult *result = new ExtensionUpdater::ParseResult;
-    result->extension_id = id;
-    result->version.reset(Version::GetVersionFromString(version));
-    result->crx_url = GURL(url);
-    return result;
-  }
-
-  static void TestXmlParsing() {
-    ExtensionErrorReporter::Init(false);
-
-    // Test parsing of a number of invalid xml cases
-    expectParseFailure("");
-    expectParseFailure(missing_appid);
-    expectParseFailure(invalid_codebase);
-    expectParseFailure(missing_version);
-    expectParseFailure(invalid_version);
-
-    // Parse some valid XML, and check that all params came out as expected
-    ScopedVector<ExtensionUpdater::ParseResult> results;
-    EXPECT_TRUE(ExtensionUpdater::Parse(valid_xml, &results.get()));
-    EXPECT_FALSE(results->empty());
-    ExtensionUpdater::ParseResult* firstResult = results->at(0);
-    EXPECT_EQ(GURL("http://example.com/extension_1.2.3.4.crx"),
-              firstResult->crx_url);
-    EXPECT_TRUE(firstResult->package_hash.empty());
-    scoped_ptr<Version> version(Version::GetVersionFromString("1.2.3.4"));
-    EXPECT_EQ(0, version->CompareTo(*firstResult->version.get()));
-    version.reset(Version::GetVersionFromString("2.0.143.0"));
-    EXPECT_EQ(0, version->CompareTo(*firstResult->browser_min_version.get()));
-
-    // Parse some xml that uses namespace prefixes.
-    results.reset();
-    EXPECT_TRUE(ExtensionUpdater::Parse(uses_namespace_prefix, &results.get()));
-    EXPECT_TRUE(ExtensionUpdater::Parse(similar_tagnames, &results.get()));
-
-    // Parse xml with hash value
-    results.reset();
-    EXPECT_TRUE(ExtensionUpdater::Parse(valid_xml_with_hash, &results.get()));
-    EXPECT_FALSE(results->empty());
-    firstResult = results->at(0);
-    EXPECT_EQ("1234", firstResult->package_hash);
+      const std::string& url,
+      std::vector<UpdateManifest::Result>* results) {
+    UpdateManifest::Result result;
+    result.extension_id = id;
+    result.version = version;
+    result.crx_url = GURL(url);
+    results->push_back(result);
   }
 
   static void TestExtensionUpdateCheckRequests() {
@@ -334,7 +233,7 @@ class ExtensionUpdaterTest : public testing::Test {
     MessageLoop message_loop;
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
-      new ExtensionUpdater(&service, prefs.get(), 60*60*24, &message_loop);
+      new ExtensionUpdater(&service, prefs.get(), 60*60*24, NULL, NULL);
     updater->Start();
 
     // Tell the update that it's time to do update checks.
@@ -377,7 +276,7 @@ class ExtensionUpdaterTest : public testing::Test {
     MessageLoop message_loop;
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
-      new ExtensionUpdater(&service, prefs.get(), 60*60*24, &message_loop);
+      new ExtensionUpdater(&service, prefs.get(), 60*60*24, NULL, NULL);
     updater->Start();
 
     // Tell the updater that it's time to do update checks.
@@ -421,10 +320,10 @@ class ExtensionUpdaterTest : public testing::Test {
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
       new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs,
-                           &message_loop);
+                           NULL, NULL);
 
     // Check passing an empty list of parse results to DetermineUpdates
-    ExtensionUpdater::ParseResultList updates;
+    std::vector<UpdateManifest::Result> updates;
     std::vector<int> updateable = updater->DetermineUpdates(updates);
     EXPECT_TRUE(updateable.empty());
 
@@ -433,14 +332,13 @@ class ExtensionUpdaterTest : public testing::Test {
     // installed and available at v2.0).
     scoped_ptr<Version> one(Version::GetVersionFromString("1.0"));
     EXPECT_TRUE(tmp[0]->version()->Equals(*one));
-    updates.push_back(MakeParseResult(tmp[0]->id(),
-        "1.1", "http://localhost/e1_1.1.crx"));
-    updates.push_back(MakeParseResult(tmp[1]->id(),
-        tmp[1]->VersionString(), "http://localhost/e2_2.0.crx"));
+    AddParseResult(tmp[0]->id(),
+        "1.1", "http://localhost/e1_1.1.crx", &updates);
+    AddParseResult(tmp[1]->id(),
+        tmp[1]->VersionString(), "http://localhost/e2_2.0.crx", &updates);
     updateable = updater->DetermineUpdates(updates);
     EXPECT_EQ(1u, updateable.size());
     EXPECT_EQ(0, updateable[0]);
-    STLDeleteElements(&updates);
     STLDeleteElements(&tmp);
   }
 
@@ -449,11 +347,16 @@ class ExtensionUpdaterTest : public testing::Test {
     TestURLFetcher* fetcher = NULL;
     URLFetcher::set_factory(&factory);
     ServiceForDownloadTests service;
-    MessageLoop message_loop;
+    MessageLoop ui_loop;
+    base::Thread file_thread("File Thread");
+    file_thread.Start();
+    base::Thread io_thread("IO Thread");
+    io_thread.Start();
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
       new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs,
-                           &message_loop);
+                           file_thread.message_loop(),
+                           io_thread.message_loop());
 
     GURL url1("http://localhost/manifest1");
     GURL url2("http://localhost/manifest2");
@@ -463,24 +366,45 @@ class ExtensionUpdaterTest : public testing::Test {
     updater->StartUpdateCheck(url1);
     updater->StartUpdateCheck(url2);
 
-    std::string manifest_data("invalid xml");
+    std::string invalid_xml = "invalid xml";
     fetcher = factory.GetFetcherByID(ExtensionUpdater::kManifestFetcherId);
     EXPECT_TRUE(fetcher != NULL && fetcher->delegate() != NULL);
     fetcher->delegate()->OnURLFetchComplete(
         fetcher, url1, URLRequestStatus(), 200, ResponseCookies(),
-        manifest_data);
+        invalid_xml);
 
     // Now that the first request is complete, make sure the second one has
     // been started.
+    const std::string kValidXml =
+        "<?xml version='1.0' encoding='UTF-8'?>"
+        "<gupdate xmlns='http://www.google.com/update2/response'"
+        "                protocol='2.0'>"
+        " <app appid='12345'>"
+        "  <updatecheck codebase='http://example.com/extension_1.2.3.4.crx'"
+        "               version='1.2.3.4' prodversionmin='2.0.143.0' />"
+        " </app>"
+        "</gupdate>";
     fetcher = factory.GetFetcherByID(ExtensionUpdater::kManifestFetcherId);
     EXPECT_TRUE(fetcher != NULL && fetcher->delegate() != NULL);
     fetcher->delegate()->OnURLFetchComplete(
         fetcher, url2, URLRequestStatus(), 200, ResponseCookies(),
-        manifest_data);
+        kValidXml);
+
+    // This should run the manifest parsing, then we want to make sure that our
+    // service was called with GetExtensionById with the matching id from
+    // kValidXml.
+    file_thread.Stop();
+    io_thread.Stop();
+    ui_loop.RunAllPending();
+    EXPECT_EQ("12345", service.last_inquired_extension_id());
+    xmlCleanupGlobals();
   }
 
   static void TestSingleExtensionDownloading() {
-    MessageLoop message_loop;
+    MessageLoop ui_loop;
+    base::Thread file_thread("File Thread");
+    file_thread.Start();
+
     TestURLFetcherFactory factory;
     TestURLFetcher* fetcher = NULL;
     URLFetcher::set_factory(&factory);
@@ -488,14 +412,12 @@ class ExtensionUpdaterTest : public testing::Test {
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
       new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs,
-                           &message_loop);
+                           file_thread.message_loop(), NULL);
 
     GURL test_url("http://localhost/extension.crx");
 
     std::string id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
-
     std::string hash = "";
-
     std::string version = "0.0.1";
 
     updater->FetchUpdatedExtension(id, test_url, hash, version);
@@ -508,7 +430,8 @@ class ExtensionUpdaterTest : public testing::Test {
         fetcher, test_url, URLRequestStatus(), 200, ResponseCookies(),
         extension_data);
 
-    message_loop.RunAllPending();
+    file_thread.Stop();
+    ui_loop.RunAllPending();
 
     // Expect that ExtensionUpdater asked the mock extensions service to install
     // a file with the test data for the right id.
@@ -531,7 +454,7 @@ class ExtensionUpdaterTest : public testing::Test {
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
       new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs,
-                           &message_loop);
+                           NULL, NULL);
     prefs.get()->
       RegisterStringPref(prefs::kExtensionBlacklistUpdateVersion, L"0");
     GURL test_url("http://localhost/extension.crx");
@@ -574,7 +497,7 @@ class ExtensionUpdaterTest : public testing::Test {
     ScopedTempPrefService prefs;
     scoped_refptr<ExtensionUpdater> updater =
       new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs,
-                           &message_loop);
+                           &message_loop, NULL);
 
     GURL url1("http://localhost/extension1.crx");
     GURL url2("http://localhost/extension2.crx");
@@ -630,10 +553,6 @@ class ExtensionUpdaterTest : public testing::Test {
 // actual test code to live in ExtenionUpdaterTest methods instead of TEST_F
 // subclasses where friendship with ExtenionUpdater is not inherited.
 
-TEST(ExtensionUpdaterTest, TestXmlParsing) {
-  ExtensionUpdaterTest::TestXmlParsing();
-}
-
 TEST(ExtensionUpdaterTest, TestExtensionUpdateCheckRequests) {
   ExtensionUpdaterTest::TestExtensionUpdateCheckRequests();
 }
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 89ef1fc..f571c08 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -8,6 +8,7 @@
 #include "base/file_util.h"
 #include "base/string_util.h"
 #include "base/values.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_file_util.h"
@@ -77,7 +78,7 @@ ExtensionsService::ExtensionsService(Profile* profile,
           switches::kExtensionsUpdateFrequency)));
     }
     updater_ = new ExtensionUpdater(this, prefs, update_frequency,
-                                    backend_loop_);
+        backend_loop_, ChromeThread::GetMessageLoop(ChromeThread::IO));
   }
 
   backend_ = new ExtensionsServiceBackend(install_directory_, frontend_loop);
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index 7f6e5ae..e1d600c 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -60,6 +60,9 @@ void SandboxedExtensionUnpacker::Start() {
 
   // If we are supposed to use a subprocess, copy the crx to the temp directory
   // and kick off the subprocess.
+  //
+  // TODO(asargent) we shouldn't need to do this branch here - instead
+  // UtilityProcessHost should handle it for us. (http://crbug.com/19192)
   if (rdh_ &&
       !CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess)) {
     ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE,
diff --git a/chrome/browser/utility_process_host.cc b/chrome/browser/utility_process_host.cc
index 285197a..32ef600 100644
--- a/chrome/browser/utility_process_host.cc
+++ b/chrome/browser/utility_process_host.cc
@@ -51,6 +51,14 @@ bool UtilityProcessHost::StartWebResourceUnpacker(const std::string& data) {
   return true;
 }
 
+bool UtilityProcessHost::StartUpdateManifestParse(const std::string& xml) {
+  if (!StartProcess(FilePath()))
+    return false;
+
+  Send(new UtilityMsg_ParseUpdateManifest(xml));
+  return true;
+}
+
 std::wstring UtilityProcessHost::GetUtilityProcessCmd() {
   return GetChildPath();
 }
@@ -142,5 +150,9 @@ void UtilityProcessHost::Client::OnMessageReceived(
                         Client::OnUnpackWebResourceSucceeded)
     IPC_MESSAGE_HANDLER(UtilityHostMsg_UnpackWebResource_Failed,
                         Client::OnUnpackWebResourceFailed)
+    IPC_MESSAGE_HANDLER(UtilityHostMsg_ParseUpdateManifest_Succeeded,
+                        Client::OnParseUpdateManifestSucceeded)
+    IPC_MESSAGE_HANDLER(UtilityHostMsg_ParseUpdateManifest_Failed,
+                        Client::OnParseUpdateManifestFailed)
   IPC_END_MESSAGE_MAP_EX()
 }
diff --git a/chrome/browser/utility_process_host.h b/chrome/browser/utility_process_host.h
index 329f856..c52c424 100644
--- a/chrome/browser/utility_process_host.h
+++ b/chrome/browser/utility_process_host.h
@@ -11,6 +11,7 @@
 #include "base/ref_counted.h"
 #include "base/task.h"
 #include "chrome/common/child_process_host.h"
+#include "chrome/common/extensions/update_manifest.h"
 #include "ipc/ipc_channel.h"
 
 class CommandLine;
@@ -54,6 +55,15 @@ class UtilityProcessHost : public ChildProcessHost {
     virtual void OnUnpackWebResourceFailed(
         const std::string& error_message) {}
 
+    // Called when an update manifest xml file was successfully parsed.
+    virtual void OnParseUpdateManifestSucceeded(
+        const UpdateManifest::ResultList& list) {}
+
+    // Called when an update manifest xml file failed parsing. |error_message|
+    // contains details suitable for logging.
+    virtual void OnParseUpdateManifestFailed(
+        const std::string& error_message) {}
+
    private:
     friend class UtilityProcessHost;
     void OnMessageReceived(const IPC::Message& message);
@@ -79,6 +89,9 @@ class UtilityProcessHost : public ChildProcessHost {
   // web resource server format(s).
   bool StartWebResourceUnpacker(const std::string& data);
 
+  // Start parsing an extensions auto-update manifest xml file.
+  bool StartUpdateManifestParse(const std::string& xml);
+
  protected:
   // Allow these methods to be overridden for tests.
   virtual std::wstring GetUtilityProcessCmd();