Parse update check run actions for the component updater.

The action will be used in a future changelist, landing soon.

BUG=687231

Review-Url: https://codereview.chromium.org/2847023002
Cr-Commit-Position: refs/heads/master@{#468161}

components/test/data/update_client/updatecheck_reply_1.xml

@@ -10,6 +10,9 @@
           <package name='jebgalgnebhfojomionfpkfelancnnkf.crx'/>
         </packages>
       </manifest>
+      <actions>
+        <action run='this'/>
+      </actions>
     </updatecheck>
   </app>
 </response>

components/test/data/update_client/updatecheck_reply_noupdate.xml

 <?xml version='1.0' encoding='UTF-8'?>
 <response protocol='3.0'>
   <app appid='jebgalgnebhfojomionfpkfelancnnkf'>
-    <updatecheck status='noupdate'/>j
+    <updatecheck status='noupdate'>
+      <actions>
+        <action run='this'/>
+      </actions>
+    </updatecheck>
   </app>
 </response>

components/update_client/BUILD.gn

@@ -108,6 +108,7 @@ bundle_data("unit_tests_bundle_data") {
     "//components/test/data/update_client/jebgalgnebhfojomionfpkfelancnnkf.crx",
     "//components/test/data/update_client/updatecheck_reply_1.xml",
     "//components/test/data/update_client/updatecheck_reply_4.xml",
+    "//components/test/data/update_client/updatecheck_reply_noupdate.xml",
   ]
   outputs = [
     "{{bundle_resources_dir}}/" +

components/update_client/component.cc

@@ -198,6 +198,7 @@ void Component::SetParseResult(const UpdateResponse::Result& result) {
   DCHECK_EQ(0, update_check_error_);
 
   status_ = result.status;
+  action_run_ = result.action_run;
 
   if (result.manifest.packages.empty())
     return;

components/update_client/component.h

@@ -134,6 +134,7 @@ class Component {
 
   FRIEND_TEST_ALL_PREFIXES(PingManagerTest, SendPing);
   FRIEND_TEST_ALL_PREFIXES(PingManagerTest, RequiresEncryption);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, NoUpdateActionRun);
   FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckCupError);
   FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckError);
   FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckInvalidAp);
@@ -395,6 +396,9 @@ class Component {
   std::string previous_fp_;
   std::string next_fp_;
 
+  // Contains the file name of the payload to run.
+  std::string action_run_;
+
   // True if the update check response for this component includes an update.
   bool is_update_available_ = false;
 

components/update_client/request_sender_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
@@ -62,10 +63,11 @@ class RequestSenderTest : public testing::Test {
   std::unique_ptr<RequestSender> request_sender_;
   std::unique_ptr<InterceptorFactory> interceptor_factory_;
 
-  URLRequestPostInterceptor* post_interceptor_1_;  // Owned by the factory.
-  URLRequestPostInterceptor* post_interceptor_2_;  // Owned by the factory.
+  // Owned by the factory.
+  URLRequestPostInterceptor* post_interceptor_1_ = nullptr;
+  URLRequestPostInterceptor* post_interceptor_2_ = nullptr;
 
-  int error_;
+  int error_ = 0;
   std::string response_;
 
  private:
@@ -76,19 +78,15 @@ class RequestSenderTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(RequestSenderTest);
 };
 
-RequestSenderTest::RequestSenderTest()
-    : post_interceptor_1_(nullptr),
-      post_interceptor_2_(nullptr),
-      error_(0),
-      scoped_task_scheduler_(&loop_) {}
+RequestSenderTest::RequestSenderTest() : scoped_task_scheduler_(&loop_) {}
 
 RequestSenderTest::~RequestSenderTest() {}
 
 void RequestSenderTest::SetUp() {
-  config_ = new TestConfigurator(base::ThreadTaskRunnerHandle::Get(),
-                                 base::ThreadTaskRunnerHandle::Get());
-  interceptor_factory_.reset(
-      new InterceptorFactory(base::ThreadTaskRunnerHandle::Get()));
+  config_ = base::MakeShared<TestConfigurator>(
+      base::ThreadTaskRunnerHandle::Get(), base::ThreadTaskRunnerHandle::Get());
+  interceptor_factory_ =
+      base::MakeUnique<InterceptorFactory>(base::ThreadTaskRunnerHandle::Get());
   post_interceptor_1_ =
       interceptor_factory_->CreateInterceptorForPath(kUrlPath1);
   post_interceptor_2_ =
@@ -96,16 +94,16 @@ void RequestSenderTest::SetUp() {
   EXPECT_TRUE(post_interceptor_1_);
   EXPECT_TRUE(post_interceptor_2_);
 
-  request_sender_.reset();
+  request_sender_ = nullptr;
 }
 
 void RequestSenderTest::TearDown() {
-  request_sender_.reset();
+  request_sender_ = nullptr;
 
   post_interceptor_1_ = nullptr;
   post_interceptor_2_ = nullptr;
 
-  interceptor_factory_.reset();
+  interceptor_factory_ = nullptr;
 
   config_ = nullptr;
 
@@ -148,10 +146,8 @@ TEST_F(RequestSenderTest, RequestSendSuccess) {
   EXPECT_TRUE(post_interceptor_1_->ExpectRequest(
       new PartialMatch("test"), test_file("updatecheck_reply_1.xml")));
 
-  std::vector<GURL> urls;
-  urls.push_back(GURL(kUrl1));
-  urls.push_back(GURL(kUrl2));
-  request_sender_.reset(new RequestSender(config_));
+  const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)};
+  request_sender_ = base::MakeUnique<RequestSender>(config_);
   request_sender_->Send(false, "test", urls,
                         base::Bind(&RequestSenderTest::RequestSenderComplete,
                                    base::Unretained(this)));
@@ -175,7 +171,7 @@ TEST_F(RequestSenderTest, RequestSendSuccess) {
   EXPECT_TRUE(base::StartsWith(response_,
                                "<?xml version='1.0' encoding='UTF-8'?>",
                                base::CompareCase::SENSITIVE));
-  EXPECT_EQ(443ul, response_.size());
+  EXPECT_EQ(505ul, response_.size());
 }
 
 // Tests that the request succeeds using the second url after the first url
@@ -185,10 +181,8 @@ TEST_F(RequestSenderTest, RequestSendSuccessWithFallback) {
       post_interceptor_1_->ExpectRequest(new PartialMatch("test"), 403));
   EXPECT_TRUE(post_interceptor_2_->ExpectRequest(new PartialMatch("test")));
 
-  std::vector<GURL> urls;
-  urls.push_back(GURL(kUrl1));
-  urls.push_back(GURL(kUrl2));
-  request_sender_.reset(new RequestSender(config_));
+  const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)};
+  request_sender_ = base::MakeUnique<RequestSender>(config_);
   request_sender_->Send(false, "test", urls,
                         base::Bind(&RequestSenderTest::RequestSenderComplete,
                                    base::Unretained(this)));
@@ -215,10 +209,8 @@ TEST_F(RequestSenderTest, RequestSendFailed) {
   EXPECT_TRUE(
       post_interceptor_2_->ExpectRequest(new PartialMatch("test"), 403));
 
-  std::vector<GURL> urls;
-  urls.push_back(GURL(kUrl1));
-  urls.push_back(GURL(kUrl2));
-  request_sender_.reset(new RequestSender(config_));
+  const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)};
+  request_sender_ = base::MakeUnique<RequestSender>(config_);
   request_sender_->Send(false, "test", urls,
                         base::Bind(&RequestSenderTest::RequestSenderComplete,
                                    base::Unretained(this)));
@@ -241,7 +233,7 @@ TEST_F(RequestSenderTest, RequestSendFailed) {
 // Tests that the request fails when no urls are provided.
 TEST_F(RequestSenderTest, RequestSendFailedNoUrls) {
   std::vector<GURL> urls;
-  request_sender_.reset(new RequestSender(config_));
+  request_sender_ = base::MakeUnique<RequestSender>(config_);
   request_sender_->Send(false, "test", urls,
                         base::Bind(&RequestSenderTest::RequestSenderComplete,
                                    base::Unretained(this)));
@@ -255,9 +247,8 @@ TEST_F(RequestSenderTest, RequestSendCupError) {
   EXPECT_TRUE(post_interceptor_1_->ExpectRequest(
       new PartialMatch("test"), test_file("updatecheck_reply_1.xml")));
 
-  std::vector<GURL> urls;
-  urls.push_back(GURL(kUrl1));
-  request_sender_.reset(new RequestSender(config_));
+  const std::vector<GURL> urls = {GURL(kUrl1)};
+  request_sender_ = base::MakeUnique<RequestSender>(config_);
   request_sender_->Send(true, "test", urls,
                         base::Bind(&RequestSenderTest::RequestSenderComplete,
                                    base::Unretained(this)));

components/update_client/update_checker_unittest.cc

@@ -205,7 +205,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) {
   // Sanity check the request.
   const auto request = post_interceptor_->GetRequests()[0];
   EXPECT_NE(string::npos, post_interceptor_->GetRequests()[0].find(
-                              "request protocol=\"3.0\" extra=\"params\""));
+                              "request protocol=\"3.1\" extra=\"params\""));
   // The request must not contain any "dlpref" in the default case.
   EXPECT_EQ(string::npos, request.find(" dlpref=\""));
   EXPECT_NE(
@@ -233,6 +233,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) {
       GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx"),
       component->crx_urls_.front());
 
+  EXPECT_STREQ("this", component->action_run_.c_str());
+
 #if (OS_WIN)
   EXPECT_NE(string::npos, request.find(" domainjoined="));
 #if defined(GOOGLE_CHROME_BUILD)
@@ -535,4 +537,31 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) {
                               "<updatecheck/>"));
 }
 
+TEST_F(UpdateCheckerTest, NoUpdateActionRun) {
+  EXPECT_TRUE(post_interceptor_->ExpectRequest(
+      new PartialMatch("updatecheck"),
+      test_file("updatecheck_reply_noupdate.xml")));
+
+  update_checker_ = UpdateChecker::Create(config_, metadata_.get());
+
+  IdToComponentPtrMap components;
+  components[kUpdateItemId] = MakeComponent();
+
+  auto& component = components[kUpdateItemId];
+
+  update_checker_->CheckForUpdates(
+      std::vector<std::string>{kUpdateItemId}, components, "", true,
+      base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
+                 base::Unretained(this)));
+  RunThreads();
+
+  EXPECT_EQ(1, post_interceptor_->GetHitCount())
+      << post_interceptor_->GetRequestsAsString();
+  ASSERT_EQ(1, post_interceptor_->GetCount())
+      << post_interceptor_->GetRequestsAsString();
+
+  EXPECT_EQ(0, error_);
+  EXPECT_STREQ("this", component->action_run_.c_str());
+}
+
 }  // namespace update_client

components/update_client/update_response.cc

@@ -25,34 +25,25 @@ const char UpdateResponse::Result::kCohort[] = "cohort";
 const char UpdateResponse::Result::kCohortHint[] = "cohorthint";
 const char UpdateResponse::Result::kCohortName[] = "cohortname";
 
-UpdateResponse::UpdateResponse() {
-}
-UpdateResponse::~UpdateResponse() {
-}
+UpdateResponse::UpdateResponse() = default;
+UpdateResponse::~UpdateResponse() = default;
 
-UpdateResponse::Results::Results() : daystart_elapsed_seconds(kNoDaystart) {
-}
+UpdateResponse::Results::Results() = default;
 UpdateResponse::Results::Results(const Results& other) = default;
-UpdateResponse::Results::~Results() {
-}
+UpdateResponse::Results::~Results() = default;
 
-UpdateResponse::Result::Result() {}
+UpdateResponse::Result::Result() = default;
 UpdateResponse::Result::Result(const Result& other) = default;
-UpdateResponse::Result::~Result() {
-}
+UpdateResponse::Result::~Result() = default;
 
-UpdateResponse::Result::Manifest::Manifest() {
-}
+UpdateResponse::Result::Manifest::Manifest() = default;
 UpdateResponse::Result::Manifest::Manifest(const Manifest& other) = default;
-UpdateResponse::Result::Manifest::~Manifest() {
-}
+UpdateResponse::Result::Manifest::~Manifest() = default;
 
-UpdateResponse::Result::Manifest::Package::Package() : size(0), sizediff(0) {
-}
+UpdateResponse::Result::Manifest::Package::Package() = default;
 UpdateResponse::Result::Manifest::Package::Package(const Package& other) =
     default;
-UpdateResponse::Result::Manifest::Package::~Package() {
-}
+UpdateResponse::Result::Manifest::Package::~Package() = default;
 
 void UpdateResponse::ParseError(const char* details, ...) {
   va_list args;
@@ -254,6 +245,20 @@ bool ParseUrlsTag(xmlNode* urls,
   return true;
 }
 
+// Parses the <actions> tag. It picks up the "run" attribute of the first
+// "action" element in "actions".
+void ParseActionsTag(xmlNode* updatecheck, UpdateResponse::Result* result) {
+  std::vector<xmlNode*> actions = GetChildren(updatecheck, "actions");
+  if (actions.empty())
+    return;
+
+  std::vector<xmlNode*> action = GetChildren(actions.front(), "action");
+  if (action.empty())
+    return;
+
+  result->action_run = GetAttribute(action.front(), "run");
+}
+
 // Parses the <updatecheck> tag.
 bool ParseUpdateCheckTag(xmlNode* updatecheck,
                          UpdateResponse::Result* result,
@@ -265,8 +270,10 @@ bool ParseUpdateCheckTag(xmlNode* updatecheck,
     return false;
   }
 
-  if (result->status == "noupdate")
+  if (result->status == "noupdate") {
+    ParseActionsTag(updatecheck, result);
     return true;
+  }
 
   if (result->status == "ok") {
     std::vector<xmlNode*> urls = GetChildren(updatecheck, "urls");
@@ -285,6 +292,7 @@ bool ParseUpdateCheckTag(xmlNode* updatecheck,
       return false;
     }
 
+    ParseActionsTag(updatecheck, result);
     return ParseManifestTag(manifests[0], result, error);
   }
 

components/update_client/update_response.h

@@ -72,12 +72,12 @@ class UpdateResponse {
         // Attributes for the full update.
         std::string name;
         std::string hash_sha256;
-        int size;
+        int size = 0;
 
         // Attributes for the differential update.
         std::string namediff;
         std::string hashdiff_sha256;
-        int sizediff;
+        int sizediff = 0;
       };
 
       Manifest();
@@ -113,6 +113,10 @@ class UpdateResponse {
     static const char kCohort[];
     static const char kCohortHint[];
     static const char kCohortName[];
+
+    // Contains the run action returned by the server as part of an update
+    // check response.
+    std::string action_run;
   };
 
   static const int kNoDaystart = -1;
@@ -122,9 +126,10 @@ class UpdateResponse {
     ~Results();
 
     // This will be >= 0, or kNoDaystart if the <daystart> tag was not present.
-    int daystart_elapsed_seconds;
+    int daystart_elapsed_seconds = kNoDaystart;
+
     // This will be >= 0, or kNoDaystart if the <daystart> tag was not present.
-    int daystart_elapsed_days;
+    int daystart_elapsed_days = kNoDaystart;
     std::vector<Result> list;
   };
 

components/update_client/update_response_unittest.cc

@@ -241,6 +241,54 @@ const char* kTwoAppsSetCohort =
     " </app>"
     "</response>";
 
+// Includes a run action for an update check with status='ok'.
+const char* kUpdateCheckStatusOkWithRunAction =
+    "<?xml version='1.0' encoding='UTF-8'?>"
+    "<response protocol='3.0'>"
+    " <app appid='12345'>"
+    "   <updatecheck status='ok'>"
+    "     <urls>"
+    "       <url codebase='http://example.com/'/>"
+    "       <url codebasediff='http://diff.example.com/'/>"
+    "     </urls>"
+    "     <manifest version='1.2.3.4' prodversionmin='2.0.143.0'>"
+    "       <packages>"
+    "         <package name='extension_1_2_3_4.crx'/>"
+    "       </packages>"
+    "     </manifest>"
+    "     <actions>"
+    "       <action run='this'/>"
+    "     </actions>"
+    "   </updatecheck>"
+    " </app>"
+    "</response>";
+
+// Includes a run action for an update check with status='noupdate'.
+const char* kUpdateCheckStatusNoUpdateWithRunAction =
+    "<?xml version='1.0' encoding='UTF-8'?>"
+    "<response protocol='3.0'>"
+    " <app appid='12345'>"
+    "   <updatecheck status='noupdate'>"
+    "     <actions>"
+    "       <action run='this'/>"
+    "     </actions>"
+    "   </updatecheck>"
+    " </app>"
+    "</response>";
+
+// Includes a run action for an update check with status='error'.
+const char* kUpdateCheckStatusErrorWithRunAction =
+    "<?xml version='1.0' encoding='UTF-8'?>"
+    "<response protocol='3.0'>"
+    " <app appid='12345' status='ok'>"
+    "  <updatecheck status='error-osnotsupported'>"
+    "     <actions>"
+    "       <action run='this'/>"
+    "     </actions>"
+    "   </updatecheck>"
+    " </app>"
+    "</response>";
+
 TEST(ComponentUpdaterUpdateResponseTest, TestParser) {
   UpdateResponse parser;
 
@@ -361,6 +409,26 @@ TEST(ComponentUpdaterUpdateResponseTest, TestParser) {
   EXPECT_EQ(secondResult->cohort_attrs.find("cohortname")->second, "cname");
   EXPECT_EQ(secondResult->cohort_attrs.find("cohorthint"),
             secondResult->cohort_attrs.end());
+
+  EXPECT_TRUE(parser.Parse(kUpdateCheckStatusOkWithRunAction));
+  EXPECT_TRUE(parser.errors().empty());
+  EXPECT_FALSE(parser.results().list.empty());
+  firstResult = &parser.results().list[0];
+  EXPECT_STREQ("ok", firstResult->status.c_str());
+  EXPECT_EQ(firstResult->extension_id, "12345");
+  EXPECT_STREQ("this", firstResult->action_run.c_str());
+
+  EXPECT_TRUE(parser.Parse(kUpdateCheckStatusNoUpdateWithRunAction));
+  EXPECT_TRUE(parser.errors().empty());
+  EXPECT_FALSE(parser.results().list.empty());
+  firstResult = &parser.results().list[0];
+  EXPECT_STREQ("noupdate", firstResult->status.c_str());
+  EXPECT_EQ(firstResult->extension_id, "12345");
+  EXPECT_STREQ("this", firstResult->action_run.c_str());
+
+  EXPECT_TRUE(parser.Parse(kUpdateCheckStatusErrorWithRunAction));
+  EXPECT_FALSE(parser.errors().empty());
+  EXPECT_TRUE(parser.results().list.empty());
 }
 
 }  // namespace update_client

components/update_client/utils.cc

@@ -95,6 +95,9 @@ std::string GetServicePack() {
 
 }  // namespace
 
+// Builds a protocol message. The protocol versions so far are:
+// * Version 3.1: it changes how the run actions are serialized.
+// * Version 3.0: it is the version implemented by the desktop updaters.
 std::string BuildProtocolRequest(
     const std::string& prod_id,
     const std::string& browser_version,
@@ -107,7 +110,7 @@ std::string BuildProtocolRequest(
     const std::unique_ptr<UpdaterState::Attributes>& updater_state_attributes) {
   std::string request(
       "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-      "<request protocol=\"3.0\" ");
+      "<request protocol=\"3.1\" ");
 
   if (!additional_attributes.empty())
     base::StringAppendF(&request, "%s ", additional_attributes.c_str());