Add support for Omaha cohorts to the component updater.

components/update_client/update_response.cc

Index: components/update_client/update_response.cc
diff --git a/components/update_client/update_response.cc b/components/update_client/update_response.cc
index e89a774ae7a77627b0800cf8b6218642c96de385..b6c8c44b8c27683fac680fc91fac9ec202842b49 100644
--- a/components/update_client/update_response.cc
+++ b/components/update_client/update_response.cc
@@ -80,6 +80,18 @@ static std::vector<xmlNode*> GetChildren(xmlNode* root, const char* name) {
   return result;
 }
 
+// Returns whether the attribute is present.
+static bool HasAttribute(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 true;
+    }
+  }
+  return false;
+}
+
 // 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);
@@ -268,6 +280,14 @@ bool ParseUpdateCheckTag(xmlNode* updatecheck,
 bool ParseAppTag(xmlNode* app,
                  UpdateResponse::Result* result,
                  std::string* error) {
+  // Read cohort information.
+  result->set_cohort = HasAttribute(app, "cohort");
+  result->cohort = GetAttribute(app, "cohort");

[Sorin Jianu, 2016/08/18 21:15:15]

Would it make sense to dynamically allocate the cohort if it has the attribute
and not use the boolean?

[waffles, 2016/08/18 22:29:23]

According to the spec, if the server serializes any attribute, the client is
supposed to write that value. If it serializes no attribute, the client is not
supposed to alter its value.

So consider:

Client sends <app cohort="foo" ...>
Server responds <app cohort="" ...>
Client must set cohort to "".

vs.

Client sends <app cohort="foo" ...>
Server responses <app ...> (no "cohort" attribute).
Client stays in cohort "foo".

In hindsight this was unnecessary complexity in the spec. Omaha didn't implement
it this way, and so the server was changed to always serialize the attribute,
echoing back the client's existing value if it doesn't need to change. Once we
took this solution we assumed there would never be any race between the client
adjusting values (e.g. cohorthint) while there was an in-flight update-check.

I would be comfortable adjusting the protocol to state that compatible servers
will always send these three attributes, and that clients should clear their
cohort/name/hint if the server doesn't send it (with the idea that sending
nothing == sending ""). Let me know if you want to go that way.

+  result->set_cohort_hint = HasAttribute(app, "cohorthint");
+  result->cohort_hint = GetAttribute(app, "cohorthint");
+  result->set_cohort_name = HasAttribute(app, "cohortname");
+  result->cohort_name = GetAttribute(app, "cohortname");
+
   // Read the crx id.
   result->extension_id = GetAttribute(app, "appid");
   if (result->extension_id.empty()) {