commit ebea33916754d5522ce6489a910b990b119b7174
author Jae Hoon Kim <kimjae@chromium.org> Mon Apr 27 13:27:35 2020 -0700
committer Commit Bot <commit-bot@chromium.org> Wed Apr 29 16:31:50 2020 +0000
tree 1f05ccf621e301a8daced871b1aa14944f282014
parent fab175a3db9ddd9898cbe89c24d2796a2985363b

update_engine: Utilize Optional for OmahaParserData

|OmahaParserData|'s cohort* fields utilize the |base::Optional<T>|
class to indicate value assigned to the fields. This atomically hold
the set value for cohort related fields.

Also, the |HTTPReponseCode| when converted to sane value is logged so
previous error code value can be tracked.

BUG=chromium:928805
TEST=FEATURES=test emerge-$B update_engine update_engine-client # filter

Change-Id: I5dbf59965538dc6c1eab052677a8d607423a34db
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2168611
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

omaha_request_action.cc

diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 85699d8..81abb3e 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -28,6 +28,7 @@
 #include <base/bind.h>
 #include <base/files/file_util.h>
 #include <base/logging.h>
+#include <base/optional.h>
 #include <base/rand_util.h>
 #include <base/strings/string_number_conversions.h>
 #include <base/strings/string_split.h>
@@ -55,6 +56,7 @@
 #include "update_engine/p2p_manager.h"
 #include "update_engine/payload_state_interface.h"
 
+using base::Optional;
 using base::Time;
 using base::TimeDelta;
 using chromeos_update_manager::kRollforwardInfinity;
@@ -139,12 +141,9 @@
     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;
+    Optional<string> cohort;
+    Optional<string> cohorthint;
+    Optional<string> cohortname;
 
     struct Package {
       string name;
@@ -180,21 +179,14 @@
 
   if (data->current_path == "/response/app") {
     OmahaParserData::App app;
-    if (attrs.find(kAttrAppId) != attrs.end()) {
+    if (attrs.find(kAttrAppId) != attrs.end())
       app.id = attrs[kAttrAppId];
-    }
-    if (attrs.find(kAttrCohort) != attrs.end()) {
-      app.cohort_set = true;
+    if (attrs.find(kAttrCohort) != attrs.end())
       app.cohort = attrs[kAttrCohort];
-    }
-    if (attrs.find(kAttrCohortHint) != attrs.end()) {
-      app.cohorthint_set = true;
+    if (attrs.find(kAttrCohortHint) != attrs.end())
       app.cohorthint = attrs[kAttrCohortHint];
-    }
-    if (attrs.find(kAttrCohortName) != attrs.end()) {
-      app.cohortname_set = true;
+    if (attrs.find(kAttrCohortName) != attrs.end())
       app.cohortname = attrs[kAttrCohortName];
-    }
     data->apps.push_back(std::move(app));
   } else if (data->current_path == "/response/app/updatecheck") {
     if (!data->apps.empty())
@@ -733,12 +725,12 @@
   // We persist the cohorts sent by omaha even if the status is "noupdate".
   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);
+      if (app.cohort)
+        PersistCohortData(kPrefsOmahaCohort, app.cohort.value());
+      if (app.cohorthint)
+        PersistCohortData(kPrefsOmahaCohortHint, app.cohorthint.value());
+      if (app.cohortname)
+        PersistCohortData(kPrefsOmahaCohortName, app.cohortname.value());
       break;
     }
   }
@@ -916,11 +908,13 @@
   }
 
   if (!successful) {
-    LOG(ERROR) << "Omaha request network transfer failed.";
     int code = GetHTTPResponseCode();
+    LOG(ERROR) << "Omaha request network transfer failed with HTTPResponseCode="
+               << code;
     // Makes sure we send sane error values.
     if (code < 0 || code >= 1000) {
       code = 999;
+      LOG(WARNING) << "Converting to sane HTTPResponseCode=" << code;
     }
     completer.set_code(static_cast<ErrorCode>(
         static_cast<int>(ErrorCode::kOmahaRequestHTTPResponseBase) + code));