commit 00d79acf5b081e80978fb0e07caeae77b99f0372
author Alex Deymo <deymo@chromium.org> Mon Jun 29 15:41:49 2015 -0700
committer ChromeOS Commit Bot <chromeos-commit-bot@chromium.org> Tue Jun 30 12:22:23 2015 +0000
tree f6214867554c82b1141bf6ad562d407dffe94614
parent 914c446bfe8356f89b6a98174e5a073d237ea268

update_engine: Persist cohorts sent by omaha even if there's no update.

Omaha can set the cohorts in the <appid> tag even if there's no update and
update_engine should persist them. This patch fixes that case and adds a test
for it.

BUG=chromium:503728
TEST=Added unittest.

Change-Id: I23c13645557396704107a2a0c2c420f536a52b03
Reviewed-on: https://chromium-review.googlesource.com/282591
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>

omaha_request_action.cc

diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 6f32c45..dff8ad3 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -714,6 +714,14 @@
     }
   }
 
+  // We persist the cohorts sent by omaha even if the status is "noupdate".
+  if (parser_data->app_cohort_set)
+    PersistCohortData(kPrefsOmahaCohort, parser_data->app_cohort);
+  if (parser_data->app_cohorthint_set)
+    PersistCohortData(kPrefsOmahaCohortHint, parser_data->app_cohorthint);
+  if (parser_data->app_cohortname_set)
+    PersistCohortData(kPrefsOmahaCohortName, parser_data->app_cohortname);
+
   if (!ParseStatus(parser_data, output_object, completer))
     return false;
 
@@ -728,13 +736,6 @@
   if (!ParseParams(parser_data, output_object, completer))
     return false;
 
-  if (parser_data->app_cohort_set)
-    PersistCohortData(kPrefsOmahaCohort, parser_data->app_cohort);
-  if (parser_data->app_cohorthint_set)
-    PersistCohortData(kPrefsOmahaCohortHint, parser_data->app_cohorthint);
-  if (parser_data->app_cohortname_set)
-    PersistCohortData(kPrefsOmahaCohortName, parser_data->app_cohortname);
-
   return true;
 }
 

omaha_request_action_unittest.cc

diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index c6355b1..1131c9c 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -61,13 +61,16 @@
     string entity_str;
     if (include_entity)
       entity_str = "<!DOCTYPE response [<!ENTITY CrOS \"ChromeOS\">]>";
-    return base::StringPrintf(
-        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-        "%s<response protocol=\"3.0\">"
+    return
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
+        entity_str + "<response protocol=\"3.0\">"
         "<daystart elapsed_seconds=\"100\"/>"
-        "<app appid=\"%s\" status=\"ok\"><ping status=\"ok\"/>"
-        "<updatecheck status=\"noupdate\"/></app></response>",
-        entity_str.c_str(), app_id.c_str());
+        "<app appid=\"" + app_id + "\" " +
+        (include_cohorts ? "cohort=\"" + cohort + "\" cohorthint=\"" +
+         cohorthint + "\" cohortname=\"" + cohortname + "\" " : "") +
+        " status=\"ok\">"
+        "<ping status=\"ok\"/>"
+        "<updatecheck status=\"noupdate\"/></app></response>";
   }
 
   string GetUpdateResponse() const {
@@ -783,6 +786,36 @@
   EXPECT_FALSE(fake_prefs_.GetString(kPrefsOmahaCohortName, &value));
 }
 
+TEST_F(OmahaRequestActionTest, CohortsArePersistedWhenNoUpdate) {
+  OmahaResponse response;
+  OmahaRequestParams params = request_params_;
+  fake_update_response_.include_cohorts = true;
+  fake_update_response_.cohort = "s/154454/8479665";
+  fake_update_response_.cohorthint = "please-put-me-on-beta";
+  fake_update_response_.cohortname = "stable";
+
+  ASSERT_TRUE(TestUpdateCheck(&params,
+                              fake_update_response_.GetNoUpdateResponse(),
+                              -1,
+                              false,  // ping_only
+                              ErrorCode::kSuccess,
+                              metrics::CheckResult::kNoUpdateAvailable,
+                              metrics::CheckReaction::kUnset,
+                              metrics::DownloadErrorCode::kUnset,
+                              &response,
+                              nullptr));
+
+  string value;
+  EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohort, &value));
+  EXPECT_EQ(fake_update_response_.cohort, value);
+
+  EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohortHint, &value));
+  EXPECT_EQ(fake_update_response_.cohorthint, value);
+
+  EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohortName, &value));
+  EXPECT_EQ(fake_update_response_.cohortname, value);
+}
+
 TEST_F(OmahaRequestActionTest, NoOutputPipeTest) {
   const string http_response(fake_update_response_.GetNoUpdateResponse());