Refactor the UpdateEngine and its actions in the component updater.

components/update_client/ping_manager_unittest.cc

Index: components/update_client/ping_manager_unittest.cc
diff --git a/components/update_client/ping_manager_unittest.cc b/components/update_client/ping_manager_unittest.cc
index 9795d6762e050c5e49678888183dbc05794bbcb7..2c85259e440d95580d1798d5c83179c7efd7631f 100644
--- a/components/update_client/ping_manager_unittest.cc
+++ b/components/update_client/ping_manager_unittest.cc
@@ -6,13 +6,16 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/version.h"
-#include "components/update_client/crx_update_item.h"
+#include "components/update_client/component.h"
 #include "components/update_client/test_configurator.h"
+#include "components/update_client/update_engine.h"
 #include "components/update_client/url_request_post_interceptor.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -21,13 +24,15 @@ using std::string;
 
 namespace update_client {
 
-class ComponentUpdaterPingManagerTest : public testing::Test {
+class PingManagerTest : public testing::Test {
  public:
-  ComponentUpdaterPingManagerTest();
-  ~ComponentUpdaterPingManagerTest() override {}
+  PingManagerTest();
+  ~PingManagerTest() override {}
 
   void RunThreadsUntilIdle();
 
+  std::unique_ptr<UpdateContext> MakeFakeUpdateContext() const;
+
   // Overrides from testing::Test.
   void SetUp() override;
   void TearDown() override;
@@ -40,25 +45,31 @@ class ComponentUpdaterPingManagerTest : public testing::Test {
   base::MessageLoopForIO loop_;
 };
 
-ComponentUpdaterPingManagerTest::ComponentUpdaterPingManagerTest() {
-}
+PingManagerTest::PingManagerTest() {}
 
-void ComponentUpdaterPingManagerTest::SetUp() {
+void PingManagerTest::SetUp() {
   config_ = new TestConfigurator(base::ThreadTaskRunnerHandle::Get(),
                                  base::ThreadTaskRunnerHandle::Get());
   ping_manager_.reset(new PingManager(config_));
 }
 
-void ComponentUpdaterPingManagerTest::TearDown() {
+void PingManagerTest::TearDown() {
   ping_manager_.reset();
   config_ = nullptr;
 }
 
-void ComponentUpdaterPingManagerTest::RunThreadsUntilIdle() {
+void PingManagerTest::RunThreadsUntilIdle() {
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_F(ComponentUpdaterPingManagerTest, PingManagerTest) {
+std::unique_ptr<UpdateContext> PingManagerTest::MakeFakeUpdateContext() const {
+  return base::MakeUnique<UpdateContext>(
+      config_, false, std::vector<std::string>(),
+      UpdateClient::CrxDataCallback(), UpdateEngine::NotifyObserversCallback(),
+      UpdateEngine::Callback(), nullptr);
+}
+
+TEST_F(PingManagerTest, SendPing) {
   std::unique_ptr<InterceptorFactory> interceptor_factory(
       new InterceptorFactory(base::ThreadTaskRunnerHandle::Get()));
   URLRequestPostInterceptor* interceptor =
@@ -66,115 +77,123 @@ TEST_F(ComponentUpdaterPingManagerTest, PingManagerTest) {
   EXPECT_TRUE(interceptor);
 
   // Test eventresult="1" is sent for successful updates.
-  CrxUpdateItem item;
-  item.id = "abc";
-  item.state = CrxUpdateItem::State::kUpdated;
-  item.previous_version = base::Version("1.0");
-  item.next_version = base::Version("2.0");
+  const auto update_context = MakeFakeUpdateContext();
 
-  ping_manager_->SendPing(&item);
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
-  EXPECT_NE(string::npos,
-            interceptor->GetRequests()[0].find(
-                "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
-                "<event eventtype=\"3\" eventresult=\"1\"/></app>"))
-      << interceptor->GetRequestsAsString();
-  interceptor->Reset();
-
-  // Test eventresult="0" is sent for failed updates.
-  item = CrxUpdateItem();
-  item.id = "abc";
-  item.state = CrxUpdateItem::State::kNoUpdate;
-  item.previous_version = base::Version("1.0");
-  item.next_version = base::Version("2.0");
-
-  ping_manager_->SendPing(&item);
-  base::RunLoop().RunUntilIdle();
+  {
+    Component component(*update_context, "abc");
+
+    component.state_ = base::MakeUnique<Component::StateUpdated>(&component);
+    component.previous_version_ = base::Version("1.0");
+    component.next_version_ = base::Version("2.0");
+
+    ping_manager_->SendPing(component);
+    base::RunLoop().RunUntilIdle();
+
+    EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
+    EXPECT_NE(string::npos,
+              interceptor->GetRequests()[0].find(
+                  "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
+                  "<event eventtype=\"3\" eventresult=\"1\"/></app>"))
+        << interceptor->GetRequestsAsString();
+    interceptor->Reset();
+  }
 
-  EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
-  EXPECT_NE(string::npos,
-            interceptor->GetRequests()[0].find(
-                "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
-                "<event eventtype=\"3\" eventresult=\"0\"/></app>"))
-      << interceptor->GetRequestsAsString();
-  interceptor->Reset();
-
-  // Test the error values and the fingerprints.
-  item = CrxUpdateItem();
-  item.id = "abc";
-  item.state = CrxUpdateItem::State::kNoUpdate;
-  item.previous_version = base::Version("1.0");
-  item.next_version = base::Version("2.0");
-  item.previous_fp = "prev fp";
-  item.next_fp = "next fp";
-  item.error_category = 1;
-  item.error_code = 2;
-  item.extra_code1 = -1;
-  item.diff_error_category = 10;
-  item.diff_error_code = 20;
-  item.diff_extra_code1 = -10;
-  item.diff_update_failed = true;
-  item.crx_diffurls.push_back(GURL("http://host/path"));
-
-  ping_manager_->SendPing(&item);
-  base::RunLoop().RunUntilIdle();
+  {
+    // Test eventresult="0" is sent for failed updates.
+    Component component(*update_context, "abc");
+    component.state_ =
+        base::MakeUnique<Component::StateUpdateError>(&component);
+    component.previous_version_ = base::Version("1.0");
+    component.next_version_ = base::Version("2.0");
+
+    ping_manager_->SendPing(component);
+    base::RunLoop().RunUntilIdle();
+
+    EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
+    EXPECT_NE(string::npos,
+              interceptor->GetRequests()[0].find(
+                  "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
+                  "<event eventtype=\"3\" eventresult=\"0\"/></app>"))
+        << interceptor->GetRequestsAsString();
+    interceptor->Reset();
+  }
 
-  EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
-  EXPECT_NE(string::npos,
-            interceptor->GetRequests()[0].find(
-                "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
-                "<event eventtype=\"3\" eventresult=\"0\" errorcat=\"1\" "
-                "errorcode=\"2\" extracode1=\"-1\" diffresult=\"0\" "
-                "differrorcat=\"10\" "
-                "differrorcode=\"20\" diffextracode1=\"-10\" "
-                "previousfp=\"prev fp\" nextfp=\"next fp\"/></app>"))
-      << interceptor->GetRequestsAsString();
-  interceptor->Reset();
-
-  // Test the download metrics.
-  item = CrxUpdateItem();
-  item.id = "abc";
-  item.state = CrxUpdateItem::State::kUpdated;
-  item.previous_version = base::Version("1.0");
-  item.next_version = base::Version("2.0");
-
-  CrxDownloader::DownloadMetrics download_metrics;
-  download_metrics.url = GURL("http://host1/path1");
-  download_metrics.downloader = CrxDownloader::DownloadMetrics::kUrlFetcher;
-  download_metrics.error = -1;
-  download_metrics.downloaded_bytes = 123;
-  download_metrics.total_bytes = 456;
-  download_metrics.download_time_ms = 987;
-  item.download_metrics.push_back(download_metrics);
-
-  download_metrics = CrxDownloader::DownloadMetrics();
-  download_metrics.url = GURL("http://host2/path2");
-  download_metrics.downloader = CrxDownloader::DownloadMetrics::kBits;
-  download_metrics.error = 0;
-  download_metrics.downloaded_bytes = 1230;
-  download_metrics.total_bytes = 4560;
-  download_metrics.download_time_ms = 9870;
-  item.download_metrics.push_back(download_metrics);
-
-  ping_manager_->SendPing(&item);
-  base::RunLoop().RunUntilIdle();
+  {
+    // Test the error values and the fingerprints.
+    Component component(*update_context, "abc");
+    component.state_ =
+        base::MakeUnique<Component::StateUpdateError>(&component);
+    component.previous_version_ = base::Version("1.0");
+    component.next_version_ = base::Version("2.0");
+    component.previous_fp_ = "prev fp";
+    component.next_fp_ = "next fp";
+    component.error_category_ = 1;
+    component.error_code_ = 2;
+    component.extra_code1_ = -1;
+    component.diff_error_category_ = 10;
+    component.diff_error_code_ = 20;
+    component.diff_extra_code1_ = -10;
+    component.crx_diffurls_.push_back(GURL("http://host/path"));
+
+    ping_manager_->SendPing(component);
+    base::RunLoop().RunUntilIdle();
+
+    EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
+    EXPECT_NE(string::npos,
+              interceptor->GetRequests()[0].find(
+                  "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
+                  "<event eventtype=\"3\" eventresult=\"0\" errorcat=\"1\" "
+                  "errorcode=\"2\" extracode1=\"-1\" diffresult=\"0\" "
+                  "differrorcat=\"10\" "
+                  "differrorcode=\"20\" diffextracode1=\"-10\" "
+                  "previousfp=\"prev fp\" nextfp=\"next fp\"/></app>"))
+        << interceptor->GetRequestsAsString();
+    interceptor->Reset();
+  }
 
-  EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
-  EXPECT_NE(
-      string::npos,
-      interceptor->GetRequests()[0].find(
-          "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
-          "<event eventtype=\"3\" eventresult=\"1\"/>"
-          "<event eventtype=\"14\" eventresult=\"0\" downloader=\"direct\" "
-          "errorcode=\"-1\" url=\"http://host1/path1\" downloaded=\"123\" "
-          "total=\"456\" download_time_ms=\"987\"/>"
-          "<event eventtype=\"14\" eventresult=\"1\" downloader=\"bits\" "
-          "url=\"http://host2/path2\" downloaded=\"1230\" total=\"4560\" "
-          "download_time_ms=\"9870\"/></app>"))
-      << interceptor->GetRequestsAsString();
-  interceptor->Reset();
+  {
+    // Test the download metrics.
+    Component component(*update_context, "abc");
+    component.state_ = base::MakeUnique<Component::StateUpdated>(&component);
+    component.previous_version_ = base::Version("1.0");
+    component.next_version_ = base::Version("2.0");
+
+    CrxDownloader::DownloadMetrics download_metrics;
+    download_metrics.url = GURL("http://host1/path1");
+    download_metrics.downloader = CrxDownloader::DownloadMetrics::kUrlFetcher;
+    download_metrics.error = -1;
+    download_metrics.downloaded_bytes = 123;
+    download_metrics.total_bytes = 456;
+    download_metrics.download_time_ms = 987;
+    component.download_metrics_.push_back(download_metrics);
+
+    download_metrics = CrxDownloader::DownloadMetrics();
+    download_metrics.url = GURL("http://host2/path2");
+    download_metrics.downloader = CrxDownloader::DownloadMetrics::kBits;
+    download_metrics.error = 0;
+    download_metrics.downloaded_bytes = 1230;
+    download_metrics.total_bytes = 4560;
+    download_metrics.download_time_ms = 9870;
+    component.download_metrics_.push_back(download_metrics);
+
+    ping_manager_->SendPing(component);
+    base::RunLoop().RunUntilIdle();
+
+    EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
+    EXPECT_NE(
+        string::npos,
+        interceptor->GetRequests()[0].find(
+            "<app appid=\"abc\" version=\"1.0\" nextversion=\"2.0\">"
+            "<event eventtype=\"3\" eventresult=\"1\"/>"
+            "<event eventtype=\"14\" eventresult=\"0\" downloader=\"direct\" "
+            "errorcode=\"-1\" url=\"http://host1/path1\" downloaded=\"123\" "
+            "total=\"456\" download_time_ms=\"987\"/>"
+            "<event eventtype=\"14\" eventresult=\"1\" downloader=\"bits\" "
+            "url=\"http://host2/path2\" downloaded=\"1230\" total=\"4560\" "
+            "download_time_ms=\"9870\"/></app>"))
+        << interceptor->GetRequestsAsString();
+    interceptor->Reset();
+  }
 
   interceptor_factory.reset();
   base::RunLoop().RunUntilIdle();
@@ -182,20 +201,22 @@ TEST_F(ComponentUpdaterPingManagerTest, PingManagerTest) {
 
 // Tests that sending the ping fails when the component requires encryption but
 // the ping URL is unsecure.
-TEST_F(ComponentUpdaterPingManagerTest, PingManagerRequiresEncryptionTest) {
+TEST_F(PingManagerTest, RequiresEncryption) {
   config_->SetPingUrl(GURL("http:\\foo\bar"));
 
+  const auto update_context = MakeFakeUpdateContext();
+
   {
-    CrxUpdateItem item;
-    item.component.requires_network_encryption = true;
+    Component component(*update_context, "abc");
+    component.crx_component_.requires_network_encryption = true;
 
-    EXPECT_FALSE(ping_manager_->SendPing(&item));
+    EXPECT_FALSE(ping_manager_->SendPing(component));
   }
 
   {
     // Tests that the default for |requires_network_encryption| is true.
-    CrxUpdateItem item;
-    EXPECT_FALSE(ping_manager_->SendPing(&item));
+    Component component(*update_context, "abc");
+    EXPECT_FALSE(ping_manager_->SendPing(component));
   }
 }