update_engine: Make the ChromeOS/AOSP Omaha Client transmit sessionid.

As per Omaha's protocol specification, ChromeOS/AOSP
needs to transmit the 'sessionid` attribute in the request.
The format of the 'sessionid' attribute is sent as GUID version 4.

The sessionid is kept throughout the entirety of the update flow.
1. When the <updatecheck> (pings/download/updates) is done, the pings
   to Omaha will send empty sessionids.
2. If there is a schedule error/issue and a new update is scheduled, a
   new sessionid will be applied.
3. During errors/issues, the same sessionid will be used.
4. All new <updatechecks> will start with a fresh sessionid.

BUG=chromium:940515
TEST=cros_workon_make --board=octopus update_engine --test
TEST=/usr/bin/update_engine_client --check_for_update # after bouncing update-engine + check /var/log/update_engine.log. 'sessionid'
attribute will be in the omaha request.

Change-Id: If4d29b630e3ab1b547606ef1c5fb06cc7a9cd61f
Reviewed-on: https://chromium-review.googlesource.com/1658422
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Ready: Jae Hoon Kim <kimjae@chromium.org>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index f24cd42..40e52f0 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -111,6 +111,7 @@
 constexpr char kXGoogleUpdateInteractivity[] = "X-Goog-Update-Interactivity";
 constexpr char kXGoogleUpdateAppId[] = "X-Goog-Update-AppId";
 constexpr char kXGoogleUpdateUpdater[] = "X-Goog-Update-Updater";
+constexpr char kXGoogleUpdateSessionId[] = "X-Goog-Update-SessionId";
 
 // updatecheck attributes (without the underscore prefix).
 constexpr char kAttrEol[] = "eol";
@@ -285,7 +286,8 @@
     SystemState* system_state,
     OmahaEvent* event,
     std::unique_ptr<HttpFetcher> http_fetcher,
-    bool ping_only)
+    bool ping_only,
+    const string& session_id)
     : system_state_(system_state),
       params_(system_state->request_params()),
       event_(event),
@@ -293,7 +295,8 @@
       policy_provider_(std::make_unique<policy::PolicyProvider>()),
       ping_only_(ping_only),
       ping_active_days_(0),
-      ping_roll_call_days_(0) {
+      ping_roll_call_days_(0),
+      session_id_(session_id) {
   policy_provider_->Reload();
 }
 
@@ -429,7 +432,8 @@
                                        ping_active_days_,
                                        ping_roll_call_days_,
                                        GetInstallDate(system_state_),
-                                       system_state_->prefs());
+                                       system_state_->prefs(),
+                                       session_id_);
   string request_post = omaha_request.GetRequest();
 
   // Set X-Goog-Update headers.
@@ -440,6 +444,7 @@
       kXGoogleUpdateUpdater,
       base::StringPrintf(
           "%s-%s", constants::kOmahaUpdaterID, kOmahaUpdaterVersion));
+  http_fetcher_->SetHeader(kXGoogleUpdateSessionId, session_id_);
 
   http_fetcher_->SetPostData(
       request_post.data(), request_post.size(), kHttpContentTypeTextXml);
diff --git a/omaha_request_action.h b/omaha_request_action.h
index f006d69..8dffb5c 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -104,7 +104,8 @@
   OmahaRequestAction(SystemState* system_state,
                      OmahaEvent* event,
                      std::unique_ptr<HttpFetcher> http_fetcher,
-                     bool ping_only);
+                     bool ping_only,
+                     const std::string& session_id);
   ~OmahaRequestAction() override;
   typedef ActionTraits<OmahaRequestAction>::InputObjectType InputObjectType;
   typedef ActionTraits<OmahaRequestAction>::OutputObjectType OutputObjectType;
@@ -143,6 +144,9 @@
               GetInstallDateWhenOOBECompletedWithValidDate);
   FRIEND_TEST(OmahaRequestActionTest,
               GetInstallDateWhenOOBECompletedDateChanges);
+  friend class UpdateAttempterTest;
+  FRIEND_TEST(UpdateAttempterTest, SessionIdTestEnforceEmptyStrPingOmaha);
+  FRIEND_TEST(UpdateAttempterTest, SessionIdTestConsistencyInUpdateFlow);
 
   // Enumeration used in PersistInstallDate().
   enum InstallDateProvisioningSource {
@@ -307,6 +311,8 @@
   int ping_active_days_;
   int ping_roll_call_days_;
 
+  std::string session_id_;
+
   DISALLOW_COPY_AND_ASSIGN(OmahaRequestAction);
 };
 
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 91de9d4..e13d10e 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -84,6 +84,7 @@
 const char kTestAppId[] = "test-app-id";
 const char kTestAppId2[] = "test-app2-id";
 const char kTestAppIdSkipUpdatecheck[] = "test-app-id-skip-updatecheck";
+const char kTestSessionId[] = "12341234-1234-1234-1234-1234123412341234";
 
 // This is a helper struct to allow unit tests build an update response with the
 // values they care about.
@@ -296,6 +297,8 @@
                   fetcher->GetHeader("X-Goog-Update-Interactivity"));
         EXPECT_EQ(kTestAppId, fetcher->GetHeader("X-Goog-Update-AppId"));
         EXPECT_NE("", fetcher->GetHeader("X-Goog-Update-Updater"));
+        EXPECT_EQ(kTestSessionId,
+                  fetcher->GetHeader("X-Goog-Update-SessionId"));
       }
       post_data_ = fetcher->post_data();
     } else if (action->Type() ==
@@ -327,6 +330,7 @@
   metrics::CheckResult expected_check_result;
   metrics::CheckReaction expected_check_reaction;
   metrics::DownloadErrorCode expected_download_error_code;
+  string session_id;
 };
 
 class OmahaRequestActionTest : public ::testing::Test {
@@ -366,6 +370,7 @@
         .expected_check_result = metrics::CheckResult::kUpdateAvailable,
         .expected_check_reaction = metrics::CheckReaction::kUpdating,
         .expected_download_error_code = metrics::DownloadErrorCode::kUnset,
+        .session_id = kTestSessionId,
     };
   }
 
@@ -439,8 +444,12 @@
   // are not using the default request_params_.
   EXPECT_EQ(&request_params_, fake_system_state_.request_params());
 
-  auto omaha_request_action = std::make_unique<OmahaRequestAction>(
-      &fake_system_state_, nullptr, std::move(fetcher), tuc_params_.ping_only);
+  auto omaha_request_action =
+      std::make_unique<OmahaRequestAction>(&fake_system_state_,
+                                           nullptr,
+                                           std::move(fetcher),
+                                           tuc_params_.ping_only,
+                                           tuc_params_.session_id);
 
   auto mock_policy_provider =
       std::make_unique<NiceMock<policy::MockPolicyProvider>>();
@@ -510,7 +519,8 @@
       event,
       std::make_unique<MockHttpFetcher>(
           http_response.data(), http_response.size(), nullptr),
-      false);
+      false,
+      "");
   ActionProcessor processor;
   processor.set_delegate(&delegate_);
   processor.EnqueueAction(std::move(action));
@@ -1311,7 +1321,8 @@
       nullptr,
       std::make_unique<MockHttpFetcher>(
           http_response.data(), http_response.size(), nullptr),
-      false);
+      false,
+      "");
   ActionProcessor processor;
   processor.set_delegate(&delegate_);
   processor.EnqueueAction(std::move(action));
@@ -1445,7 +1456,8 @@
       nullptr,
       std::make_unique<MockHttpFetcher>(
           http_response.data(), http_response.size(), nullptr),
-      false);
+      false,
+      "");
   TerminateEarlyTestProcessorDelegate delegate;
   ActionProcessor processor;
   processor.set_delegate(&delegate);
@@ -1580,7 +1592,8 @@
       nullptr,
       std::make_unique<MockHttpFetcher>(
           http_response.data(), http_response.size(), nullptr),
-      false);
+      false,
+      "");
   EXPECT_FALSE(update_check_action.IsEvent());
 
   OmahaRequestAction event_action(
@@ -1588,7 +1601,8 @@
       new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
       std::make_unique<MockHttpFetcher>(
           http_response.data(), http_response.size(), nullptr),
-      false);
+      false,
+      "");
   EXPECT_TRUE(event_action.IsEvent());
 }
 
diff --git a/omaha_request_builder_xml.cc b/omaha_request_builder_xml.cc
index e335c40..3e4a335 100644
--- a/omaha_request_builder_xml.cc
+++ b/omaha_request_builder_xml.cc
@@ -337,10 +337,11 @@
 
   string request_xml = base::StringPrintf(
       "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
-      "<request requestid=\"%s\""
+      "<request requestid=\"%s\" sessionid=\"%s\""
       " protocol=\"3.0\" updater=\"%s\" updaterversion=\"%s\""
       " installsource=\"%s\" ismachine=\"1\">\n%s%s</request>\n",
       base::GenerateGUID().c_str() /* requestid */,
+      session_id_.c_str(),
       constants::kOmahaUpdaterID,
       kOmahaUpdaterVersion,
       params_->interactive() ? "ondemandupdate" : "scheduler",
diff --git a/omaha_request_builder_xml.h b/omaha_request_builder_xml.h
index c390b9e..0ba44b8 100644
--- a/omaha_request_builder_xml.h
+++ b/omaha_request_builder_xml.h
@@ -121,7 +121,8 @@
                          int ping_active_days,
                          int ping_roll_call_days,
                          int install_date_in_days,
-                         PrefsInterface* prefs)
+                         PrefsInterface* prefs,
+                         const std::string& session_id)
       : event_(event),
         params_(params),
         ping_only_(ping_only),
@@ -129,7 +130,8 @@
         ping_active_days_(ping_active_days),
         ping_roll_call_days_(ping_roll_call_days),
         install_date_in_days_(install_date_in_days),
-        prefs_(prefs) {}
+        prefs_(prefs),
+        session_id_(session_id) {}
 
   ~OmahaRequestBuilderXml() override = default;
 
@@ -173,6 +175,7 @@
   int ping_roll_call_days_;
   int install_date_in_days_;
   PrefsInterface* prefs_;
+  std::string session_id_;
 
   DISALLOW_COPY_AND_ASSIGN(OmahaRequestBuilderXml);
 };
diff --git a/omaha_request_builder_xml_unittest.cc b/omaha_request_builder_xml_unittest.cc
index 23abebb..4375bed 100644
--- a/omaha_request_builder_xml_unittest.cc
+++ b/omaha_request_builder_xml_unittest.cc
@@ -90,7 +90,8 @@
                                        0,
                                        0,
                                        0,
-                                       fake_system_state_.prefs()};
+                                       fake_system_state_.prefs(),
+                                       ""};
   const string request_xml = omaha_request.GetRequest();
   const string key = "requestid";
   const string request_id =
@@ -100,4 +101,28 @@
     EXPECT_TRUE(base::IsValidGUID(request_id));
 }
 
+TEST_F(OmahaRequestBuilderXmlTest, GetRequestXmlSessionIdTest) {
+  const string gen_session_id = base::GenerateGUID();
+  OmahaEvent omaha_event;
+  OmahaRequestParams omaha_request_params{&fake_system_state_};
+  OmahaRequestBuilderXml omaha_request{&omaha_event,
+                                       &omaha_request_params,
+                                       false,
+                                       false,
+                                       0,
+                                       0,
+                                       0,
+                                       fake_system_state_.prefs(),
+                                       gen_session_id};
+  const string request_xml = omaha_request.GetRequest();
+  const string key = "sessionid";
+  const string session_id =
+      FindAttributeKeyValueInXml(request_xml, key, kGuidSize);
+  // A valid |session_id| is either a GUID version 4 or empty string.
+  if (!session_id.empty()) {
+    EXPECT_TRUE(base::IsValidGUID(session_id));
+  }
+  EXPECT_EQ(gen_session_id, session_id);
+}
+
 }  // namespace chromeos_update_engine
diff --git a/update_attempter.cc b/update_attempter.cc
index fcafd56..73785bf 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -648,6 +648,10 @@
   CHECK(!processor_->IsRunning());
   processor_->set_delegate(this);
 
+  // The session ID needs to be kept throughout the update flow. The value
+  // of the session ID will reset/update only when it is a new update flow.
+  session_id_ = base::GenerateGUID();
+
   // Actions:
   auto update_check_fetcher = std::make_unique<LibcurlHttpFetcher>(
       GetProxyResolver(), system_state_->hardware());
@@ -656,8 +660,12 @@
   // See comment in libcurl_http_fetcher.cc.
   update_check_fetcher->set_no_network_max_retries(interactive ? 1 : 3);
   update_check_fetcher->set_is_update_check(true);
-  auto update_check_action = std::make_unique<OmahaRequestAction>(
-      system_state_, nullptr, std::move(update_check_fetcher), false);
+  auto update_check_action =
+      std::make_unique<OmahaRequestAction>(system_state_,
+                                           nullptr,
+                                           std::move(update_check_fetcher),
+                                           false,
+                                           session_id_);
   auto response_handler_action =
       std::make_unique<OmahaResponseHandlerAction>(system_state_);
   auto update_boot_flags_action =
@@ -667,7 +675,8 @@
       new OmahaEvent(OmahaEvent::kTypeUpdateDownloadStarted),
       std::make_unique<LibcurlHttpFetcher>(GetProxyResolver(),
                                            system_state_->hardware()),
-      false);
+      false,
+      session_id_);
 
   LibcurlHttpFetcher* download_fetcher =
       new LibcurlHttpFetcher(GetProxyResolver(), system_state_->hardware());
@@ -688,7 +697,8 @@
       new OmahaEvent(OmahaEvent::kTypeUpdateDownloadFinished),
       std::make_unique<LibcurlHttpFetcher>(GetProxyResolver(),
                                            system_state_->hardware()),
-      false);
+      false,
+      session_id_);
   auto filesystem_verifier_action =
       std::make_unique<FilesystemVerifierAction>();
   auto update_complete_action = std::make_unique<OmahaRequestAction>(
@@ -696,7 +706,8 @@
       new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
       std::make_unique<LibcurlHttpFetcher>(GetProxyResolver(),
                                            system_state_->hardware()),
-      false);
+      false,
+      session_id_);
 
   auto postinstall_runner_action = std::make_unique<PostinstallRunnerAction>(
       system_state_->boot_control(), system_state_->hardware());
@@ -1450,7 +1461,8 @@
       error_event_.release(),  // Pass ownership.
       std::make_unique<LibcurlHttpFetcher>(GetProxyResolver(),
                                            system_state_->hardware()),
-      false);
+      false,
+      session_id_);
   processor_->EnqueueAction(std::move(error_event_action));
   SetStatusAndNotify(UpdateStatus::REPORTING_ERROR_EVENT);
   processor_->StartProcessing();
@@ -1493,7 +1505,8 @@
         nullptr,
         std::make_unique<LibcurlHttpFetcher>(GetProxyResolver(),
                                              system_state_->hardware()),
-        true);
+        true,
+        "" /* session_id */);
     processor_->set_delegate(nullptr);
     processor_->EnqueueAction(std::move(ping_action));
     // Call StartProcessing() synchronously here to avoid any race conditions
diff --git a/update_attempter.h b/update_attempter.h
index 82b81ce..59c9686 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -26,6 +26,7 @@
 #include <vector>
 
 #include <base/bind.h>
+#include <base/guid.h>
 #include <base/time/time.h>
 #include <gtest/gtest_prod.h>  // for FRIEND_TEST
 
@@ -268,6 +269,9 @@
   FRIEND_TEST(UpdateAttempterTest, RollbackMetricsRollbackSuccess);
   FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest);
   FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionTest);
+  FRIEND_TEST(UpdateAttempterTest, SessionIdTestOnUpdateCheck);
+  FRIEND_TEST(UpdateAttempterTest, SessionIdTestEnforceEmptyStrPingOmaha);
+  FRIEND_TEST(UpdateAttempterTest, SessionIdTestOnOmahaRequestActions);
   FRIEND_TEST(UpdateAttempterTest, SetRollbackHappenedNotRollback);
   FRIEND_TEST(UpdateAttempterTest, SetRollbackHappenedRollback);
   FRIEND_TEST(UpdateAttempterTest, TargetVersionPrefixSetAndReset);
@@ -523,6 +527,9 @@
   base::TimeDelta staging_wait_time_;
   chromeos_update_manager::StagingSchedule staging_schedule_;
 
+  // This is the session ID used to track update flow to Omaha.
+  std::string session_id_;
+
   DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
 };
 
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index e246e1b..fb33011 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -19,6 +19,7 @@
 #include <stdint.h>
 
 #include <memory>
+#include <unordered_set>
 
 #include <base/files/file_util.h>
 #include <base/message_loop/message_loop.h>
@@ -59,11 +60,13 @@
 using policy::DevicePolicy;
 using std::string;
 using std::unique_ptr;
+using std::unordered_set;
 using std::vector;
 using testing::_;
 using testing::DoAll;
 using testing::Field;
 using testing::InSequence;
+using testing::Invoke;
 using testing::Ne;
 using testing::NiceMock;
 using testing::Pointee;
@@ -109,13 +112,13 @@
     }
     return true;
   }
-  void EnableScheduleUpdates() { do_schedule_updates_ = true; }
+
   void DisableScheduleUpdates() { do_schedule_updates_ = false; }
 
-  // Indicates whether ScheduleUpdates() was called.
+  // Indicates whether |ScheduleUpdates()| was called.
   bool schedule_updates_called() const { return schedule_updates_called_; }
 
-  // Need to expose forced_omaha_url_ so we can test it.
+  // Need to expose |forced_omaha_url_| so we can test it.
   const string& forced_omaha_url() const { return forced_omaha_url_; }
 
  private:
@@ -144,6 +147,7 @@
 
   void SetUp() override {
     EXPECT_NE(nullptr, attempter_.system_state_);
+    EXPECT_NE(nullptr, attempter_.system_state_->update_manager());
     EXPECT_EQ(0, attempter_.http_response_code_);
     EXPECT_EQ(UpdateStatus::IDLE, attempter_.status_);
     EXPECT_EQ(0.0, attempter_.download_progress_);
@@ -154,7 +158,7 @@
     attempter_.processor_.reset(processor_);  // Transfers ownership.
     prefs_ = fake_system_state_.mock_prefs();
 
-    // Set up store/load semantics of P2P properties via the mock PayloadState.
+    // Setup store/load semantics of P2P properties via the mock |PayloadState|.
     actual_using_p2p_for_downloading_ = false;
     EXPECT_CALL(*fake_system_state_.mock_payload_state(),
                 SetUsingP2PForDownloading(_))
@@ -188,6 +192,9 @@
   void P2PEnabledInteractiveStart();
   void P2PEnabledStartingFailsStart();
   void P2PEnabledHousekeepingFailsStart();
+  void SessionIdTestChange();
+  void SessionIdTestEnforceEmptyStrPingOmaha();
+  void SessionIdTestConsistencyInUpdateFlow();
   void UpdateToQuickFixBuildStart(bool set_token);
   void ResetRollbackHappenedStart(bool is_consumer,
                                   bool is_policy_available,
@@ -214,7 +221,8 @@
   MockDlcService mock_dlcservice_;
 
   NiceMock<MockActionProcessor>* processor_;
-  NiceMock<MockPrefs>* prefs_;  // Shortcut to fake_system_state_->mock_prefs().
+  NiceMock<MockPrefs>*
+      prefs_;  // Shortcut to |fake_system_state_->mock_prefs()|.
   NiceMock<MockConnectionManager> mock_connection_manager;
 
   bool actual_using_p2p_for_downloading_;
@@ -228,6 +236,75 @@
                  base::Unretained(&loop_)));
 }
 
+void UpdateAttempterTest::SessionIdTestChange() {
+  EXPECT_NE(UpdateStatus::UPDATED_NEED_REBOOT, attempter_.status());
+  const auto old_session_id = attempter_.session_id_;
+  attempter_.Update("", "", "", "", false, false, 0, false, false);
+  EXPECT_NE(old_session_id, attempter_.session_id_);
+  ScheduleQuitMainLoop();
+}
+
+TEST_F(UpdateAttempterTest, SessionIdTestChange) {
+  loop_.PostTask(FROM_HERE,
+                 base::Bind(&UpdateAttempterTest::SessionIdTestChange,
+                            base::Unretained(this)));
+  loop_.Run();
+}
+
+void UpdateAttempterTest::SessionIdTestEnforceEmptyStrPingOmaha() {
+  // The |session_id_| should not be changed and should remain as an empty
+  // string when |status_| is |UPDATED_NEED_REBOOT| (only for consistency)
+  // and |PingOmaha()| is called.
+  attempter_.DisableScheduleUpdates();
+  attempter_.status_ = UpdateStatus::UPDATED_NEED_REBOOT;
+  const auto old_session_id = attempter_.session_id_;
+  auto CheckIfEmptySessionId = [](AbstractAction* aa) {
+    if (aa->Type() == OmahaRequestAction::StaticType()) {
+      EXPECT_TRUE(static_cast<OmahaRequestAction*>(aa)->session_id_.empty());
+    }
+  };
+  EXPECT_CALL(*processor_, EnqueueAction(Pointee(_)))
+      .WillRepeatedly(Invoke(CheckIfEmptySessionId));
+  EXPECT_CALL(*processor_, StartProcessing());
+  attempter_.PingOmaha();
+  EXPECT_EQ(old_session_id, attempter_.session_id_);
+  EXPECT_EQ(UpdateStatus::UPDATED_NEED_REBOOT, attempter_.status_);
+  ScheduleQuitMainLoop();
+}
+
+TEST_F(UpdateAttempterTest, SessionIdTestEnforceEmptyStrPingOmaha) {
+  loop_.PostTask(
+      FROM_HERE,
+      base::Bind(&UpdateAttempterTest::SessionIdTestEnforceEmptyStrPingOmaha,
+                 base::Unretained(this)));
+  loop_.Run();
+}
+
+void UpdateAttempterTest::SessionIdTestConsistencyInUpdateFlow() {
+  // All session IDs passed into |OmahaRequestActions| should be enforced to
+  // have the same value in |BuildUpdateActions()|.
+  unordered_set<string> session_ids;
+  // Gather all the session IDs being passed to |OmahaRequestActions|.
+  auto CheckSessionId = [&session_ids](AbstractAction* aa) {
+    if (aa->Type() == OmahaRequestAction::StaticType())
+      session_ids.insert(static_cast<OmahaRequestAction*>(aa)->session_id_);
+  };
+  EXPECT_CALL(*processor_, EnqueueAction(Pointee(_)))
+      .WillRepeatedly(Invoke(CheckSessionId));
+  attempter_.BuildUpdateActions(false);
+  // Validate that all the session IDs are the same.
+  EXPECT_EQ(1, session_ids.size());
+  ScheduleQuitMainLoop();
+}
+
+TEST_F(UpdateAttempterTest, SessionIdTestConsistencyInUpdateFlow) {
+  loop_.PostTask(
+      FROM_HERE,
+      base::Bind(&UpdateAttempterTest::SessionIdTestConsistencyInUpdateFlow,
+                 base::Unretained(this)));
+  loop_.Run();
+}
+
 TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) {
   unique_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, nullptr));
   fetcher->FailTransfer(503);  // Sets the HTTP response code.
@@ -269,7 +346,7 @@
 
   EXPECT_EQ(0.0, attempter_.download_progress_);
   // This is set via inspecting the InstallPlan payloads when the
-  // OmahaResponseAction is completed
+  // |OmahaResponseAction| is completed.
   attempter_.new_payload_size_ = bytes_total;
   NiceMock<MockServiceObserver> observer;
   EXPECT_CALL(observer,
@@ -293,14 +370,14 @@
 }
 
 TEST_F(UpdateAttempterTest, ChangeToDownloadingOnReceivedBytesTest) {
-  // The transition into UpdateStatus::DOWNLOADING happens when the
+  // The transition into |UpdateStatus::DOWNLOADING| happens when the
   // first bytes are received.
   uint64_t bytes_progressed = 1024 * 1024;    // 1MB
   uint64_t bytes_received = 2 * 1024 * 1024;  // 2MB
   uint64_t bytes_total = 20 * 1024 * 1024;    // 300MB
   attempter_.status_ = UpdateStatus::CHECKING_FOR_UPDATE;
   // This is set via inspecting the InstallPlan payloads when the
-  // OmahaResponseAction is completed
+  // |OmahaResponseAction| is completed.
   attempter_.new_payload_size_ = bytes_total;
   EXPECT_EQ(0.0, attempter_.download_progress_);
   NiceMock<MockServiceObserver> observer;
@@ -315,8 +392,7 @@
 
 TEST_F(UpdateAttempterTest, BroadcastCompleteDownloadTest) {
   // There is a special case to ensure that at 100% downloaded,
-  // download_progress_ is updated and that value broadcast. This test confirms
-  // that.
+  // |download_progress_| is updated and broadcastest.
   uint64_t bytes_progressed = 0;              // ignored
   uint64_t bytes_received = 5 * 1024 * 1024;  // ignored
   uint64_t bytes_total = 5 * 1024 * 1024;     // 300MB
@@ -338,7 +414,7 @@
   unique_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, nullptr));
   fetcher->FailTransfer(500);  // Sets the HTTP response code.
   OmahaRequestAction action(
-      &fake_system_state_, nullptr, std::move(fetcher), false);
+      &fake_system_state_, nullptr, std::move(fetcher), false, "");
   ObjectCollectorAction<OmahaResponse> collector_action;
   BondActions(&action, &collector_action);
   OmahaResponse response;
@@ -368,7 +444,7 @@
 
   FakeSystemState fake_system_state;
   OmahaRequestAction omaha_request_action(
-      &fake_system_state, nullptr, nullptr, false);
+      &fake_system_state, nullptr, nullptr, false, "");
   EXPECT_EQ(ErrorCode::kOmahaRequestError,
             GetErrorCodeForAction(&omaha_request_action, ErrorCode::kError));
   OmahaResponseHandlerAction omaha_response_handler_action(&fake_system_state_);
@@ -488,8 +564,8 @@
 void UpdateAttempterTest::UpdateTestStart() {
   attempter_.set_http_response_code(200);
 
-  // Expect that the device policy is loaded by the UpdateAttempter at some
-  // point by calling RefreshDevicePolicy.
+  // Expect that the device policy is loaded by the |UpdateAttempter| at some
+  // point by calling |RefreshDevicePolicy()|.
   auto device_policy = std::make_unique<policy::MockDevicePolicy>();
   EXPECT_CALL(*device_policy, LoadPolicy())
       .Times(testing::AtLeast(1))
@@ -628,7 +704,7 @@
 TEST_F(UpdateAttempterTest, PingOmahaTest) {
   EXPECT_FALSE(attempter_.waiting_for_scheduled_check_);
   EXPECT_FALSE(attempter_.schedule_updates_called());
-  // Disable scheduling of subsequnet checks; we're using the DefaultPolicy in
+  // Disable scheduling of subsequnet checks; we're using the |DefaultPolicy| in
   // testing, which is more permissive than we want to handle here.
   attempter_.DisableScheduleUpdates();
   loop_.PostTask(FROM_HERE,
@@ -702,7 +778,7 @@
 
 void UpdateAttempterTest::P2PNotEnabledStart() {
   // If P2P is not enabled, check that we do not attempt housekeeping
-  // and do not convey that p2p is to be used.
+  // and do not convey that P2P is to be used.
   MockP2PManager mock_p2p_manager;
   fake_system_state_.set_p2p_manager(&mock_p2p_manager);
   mock_p2p_manager.fake().SetP2PEnabled(false);
@@ -721,8 +797,8 @@
 }
 
 void UpdateAttempterTest::P2PEnabledStartingFailsStart() {
-  // If p2p is enabled, but starting it fails ensure we don't do
-  // any housekeeping and do not convey that p2p should be used.
+  // If P2P is enabled, but starting it fails ensure we don't do
+  // any housekeeping and do not convey that P2P should be used.
   MockP2PManager mock_p2p_manager;
   fake_system_state_.set_p2p_manager(&mock_p2p_manager);
   mock_p2p_manager.fake().SetP2PEnabled(true);
@@ -744,8 +820,8 @@
 }
 
 void UpdateAttempterTest::P2PEnabledHousekeepingFailsStart() {
-  // If p2p is enabled, starting it works but housekeeping fails, ensure
-  // we do not convey p2p is to be used.
+  // If P2P is enabled, starting it works but housekeeping fails, ensure
+  // we do not convey P2P is to be used.
   MockP2PManager mock_p2p_manager;
   fake_system_state_.set_p2p_manager(&mock_p2p_manager);
   mock_p2p_manager.fake().SetP2PEnabled(true);
@@ -769,7 +845,7 @@
   MockP2PManager mock_p2p_manager;
   fake_system_state_.set_p2p_manager(&mock_p2p_manager);
   // If P2P is enabled and starting it works, check that we performed
-  // housekeeping and that we convey p2p should be used.
+  // housekeeping and that we convey P2P should be used.
   mock_p2p_manager.fake().SetP2PEnabled(true);
   mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
   mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
@@ -792,7 +868,7 @@
   fake_system_state_.set_p2p_manager(&mock_p2p_manager);
   // For an interactive check, if P2P is enabled and starting it
   // works, check that we performed housekeeping and that we convey
-  // p2p should be used for sharing but NOT for downloading.
+  // P2P should be used for sharing but NOT for downloading.
   mock_p2p_manager.fake().SetP2PEnabled(true);
   mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
   mock_p2p_manager.fake().SetPerformHousekeepingResult(true);