Only accept cohort in app that matches product id.
The system app also has cohort data but it's empty, and it overwrites
the one in product app.
We don't really care about the cohort for system app, so ignore them.
Bug: 65414782
Test: unittests
Change-Id: Ie4b6f93563ec5a8d76e22e23a863ba7e04b9f119
(cherry picked from commit 8fd11a1907bb7a33678d3c38443eedfcf3e8e1e6)
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index cce4287..7ac7059 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -380,12 +380,6 @@
string current_path;
// These are the values extracted from the XML.
- string app_cohort;
- string app_cohorthint;
- string app_cohortname;
- bool app_cohort_set = false;
- bool app_cohorthint_set = false;
- bool app_cohortname_set = false;
string updatecheck_poll_interval;
map<string, string> updatecheck_attrs;
string daystart_elapsed_days;
@@ -397,6 +391,12 @@
string manifest_version;
map<string, string> action_postinstall_attrs;
string updatecheck_status;
+ string cohort;
+ string cohorthint;
+ string cohortname;
+ bool cohort_set = false;
+ bool cohorthint_set = false;
+ bool cohortname_set = false;
struct Package {
string name;
@@ -430,22 +430,23 @@
}
if (data->current_path == "/response/app") {
- data->apps.emplace_back();
+ OmahaParserData::App app;
if (attrs.find("appid") != attrs.end()) {
- data->apps.back().id = attrs["appid"];
+ app.id = attrs["appid"];
}
if (attrs.find("cohort") != attrs.end()) {
- data->app_cohort_set = true;
- data->app_cohort = attrs["cohort"];
+ app.cohort_set = true;
+ app.cohort = attrs["cohort"];
}
if (attrs.find("cohorthint") != attrs.end()) {
- data->app_cohorthint_set = true;
- data->app_cohorthint = attrs["cohorthint"];
+ app.cohorthint_set = true;
+ app.cohorthint = attrs["cohorthint"];
}
if (attrs.find("cohortname") != attrs.end()) {
- data->app_cohortname_set = true;
- data->app_cohortname = attrs["cohortname"];
+ app.cohortname_set = true;
+ app.cohortname = attrs["cohortname"];
}
+ data->apps.push_back(std::move(app));
} else if (data->current_path == "/response/app/updatecheck") {
if (!data->apps.empty())
data->apps.back().updatecheck_status = attrs["status"];
@@ -923,12 +924,17 @@
}
// 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);
+ for (const auto& app : parser_data->apps) {
+ if (app.id == params_->GetAppId()) {
+ if (app.cohort_set)
+ PersistCohortData(kPrefsOmahaCohort, app.cohort);
+ if (app.cohorthint_set)
+ PersistCohortData(kPrefsOmahaCohortHint, app.cohorthint);
+ if (app.cohortname_set)
+ PersistCohortData(kPrefsOmahaCohortName, app.cohortname);
+ break;
+ }
+ }
// Parse the updatecheck attributes.
PersistEolStatus(parser_data->updatecheck_attrs);
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 7e38956..409165c 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -144,8 +144,9 @@
(disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
"/></actions></manifest></updatecheck></app>" +
(multi_app
- ? "<app appid=\"" + app_id2 +
- "\"><updatecheck status=\"ok\"><urls><url codebase=\"" +
+ ? "<app appid=\"" + app_id2 + "\"" +
+ (include_cohorts ? " cohort=\"cohort2\"" : "") +
+ "><updatecheck status=\"ok\"><urls><url codebase=\"" +
codebase2 + "\"/></urls><manifest version=\"" + version2 +
"\"><packages>"
"<package name=\"package3\" size=\"333\" "
@@ -1182,6 +1183,37 @@
EXPECT_EQ(fake_update_response_.cohortname, value);
}
+TEST_F(OmahaRequestActionTest, MultiAppCohortTest) {
+ OmahaResponse response;
+ OmahaRequestParams params = request_params_;
+ fake_update_response_.multi_app = true;
+ 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(¶ms,
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ 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());