Add policy fetch support.

Bug: 1068797
Change-Id: Iaf56825d95fdb1ba502dfb2d8538f288192e3c28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332572
Commit-Queue: Xiaoling Bao <xiaolingbao@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794656}

chrome/updater/BUILD.gn

@@ -262,6 +262,21 @@ if (is_win || is_mac) {
     }
   }
 
+  source_set("updater_tests_support") {
+    testonly = true
+
+    sources = [
+      "unittest_util.cc",
+      "unittest_util.h",
+    ]
+
+    deps = [
+      ":base",
+      ":lib",
+      "//base",
+    ]
+  }
+
   test("updater_tests") {
     testonly = true
 
@@ -277,8 +292,6 @@ if (is_win || is_mac) {
       "tag_unittest.cc",
       "test/integration_tests.cc",
       "test/integration_tests.h",
-      "unittest_util.cc",
-      "unittest_util.h",
       "unittest_util_unittest.cc",
       "updater_unittest.cc",
     ]
@@ -287,6 +300,7 @@ if (is_win || is_mac) {
       ":base",
       ":lib",
       ":updater",
+      ":updater_tests_support",
       ":version_header",
       "//base",
       "//base/test:test_support",

chrome/updater/device_management/BUILD.gn

@@ -69,6 +69,7 @@ source_set("unittest") {
     "//base",
     "//base/test:test_support",
     "//chrome/updater:base",
+    "//chrome/updater:updater_tests_support",
     "//chrome/updater/protos:omaha_proto",
     "//components/update_client:update_client",
     "//net:test_support",

chrome/updater/device_management/dm_client.cc

@@ -12,6 +12,7 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/updater/constants.h"
+#include "chrome/updater/device_management/dm_cached_policy_info.h"
 #include "chrome/updater/device_management/dm_storage.h"
 #include "chrome/updater/updater_version.h"
 #include "chrome/updater/util.h"
@@ -46,6 +47,11 @@ constexpr char kAuthorizationHeader[] = "Authorization";
 constexpr char kRegistrationRequestType[] = "register_policy_agent";
 constexpr char kRegistrationTokenType[] = "GoogleEnrollmentToken";
 
+// Constants for policy fetch requests.
+constexpr char kPolicyFetchRequestType[] = "policy";
+constexpr char kPolicyFetchTokenType[] = "GoogleDMToken";
+constexpr char kGoogleUpdateMachineLevelApps[] = "google/machine-level-apps";
+
 constexpr int kHTTPStatusOK = 200;
 constexpr int kHTTPStatusGone = 410;
 
@@ -154,18 +160,19 @@ void DMClient::PostRegisterRequest(DMRequestCallback request_callback) {
       policy::GetOSVersion());
 
   // Authorization token is the enrollment token for device registration.
-  base::flat_map<std::string, std::string> additional_headers;
-  additional_headers.emplace(
-      kAuthorizationHeader,
-      base::StringPrintf("%s token=%s", kRegistrationTokenType,
-                         enrollment_token.c_str()));
+  const base::flat_map<std::string, std::string> additional_headers = {
+      {kAuthorizationHeader,
+       base::StringPrintf("%s token=%s", kRegistrationTokenType,
+                          enrollment_token.c_str())},
+  };
 
   network_fetcher_->PostRequest(
       BuildURL(kRegistrationRequestType), data, kDMContentType,
       additional_headers,
       base::BindOnce(&DMClient::OnRequestStarted, base::Unretained(this)),
       base::BindRepeating(&DMClient::OnRequestProgress, base::Unretained(this)),
-      base::BindOnce(&DMClient::OnRequestComplete, base::Unretained(this)));
+      base::BindOnce(&DMClient::OnRegisterRequestComplete,
+                     base::Unretained(this)));
 }
 
 void DMClient::OnRequestStarted(int response_code, int64_t content_length) {
@@ -180,11 +187,12 @@ void DMClient::OnRequestProgress(int64_t current) {
   VLOG(1) << "POST request progess made, current bytes: " << current;
 }
 
-void DMClient::OnRequestComplete(std::unique_ptr<std::string> response_body,
-                                 int net_error,
-                                 const std::string& header_etag,
-                                 const std::string& header_x_cup_server_proof,
-                                 int64_t xheader_retry_after_sec) {
+void DMClient::OnRegisterRequestComplete(
+    std::unique_ptr<std::string> response_body,
+    int net_error,
+    const std::string& header_etag,
+    const std::string& header_x_cup_server_proof,
+    int64_t xheader_retry_after_sec) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   RequestResult request_result = RequestResult::kSuccess;
 
@@ -206,7 +214,91 @@ void DMClient::OnRequestComplete(std::unique_ptr<std::string> response_body,
       request_result = RequestResult::kUnexpectedResponse;
     } else {
       VLOG(1) << "Register request completed, got DM token: " << dm_token;
-      storage_->StoreDmToken(dm_token);
+      if (!storage_->StoreDmToken(dm_token))
+        request_result = RequestResult::kSerializationError;
+    }
+  }
+
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(request_callback_), request_result));
+}
+
+void DMClient::PostPolicyFetchRequest(DMRequestCallback request_callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  RequestResult result = RequestResult::kSuccess;
+  const std::string dm_token = storage_->GetDmToken();
+  network_fetcher_ = config_->CreateNetworkFetcher();
+  request_callback_ = std::move(request_callback);
+
+  if (storage_->IsDeviceDeregistered()) {
+    result = RequestResult::kDeregistered;
+  } else if (dm_token.empty()) {
+    result = RequestResult::kNoDMToken;
+  } else if (storage_->GetDeviceID().empty()) {
+    result = RequestResult::kNoDeviceID;
+  } else if (!network_fetcher_) {
+    result = RequestResult::kFetcherError;
+  }
+
+  if (result != RequestResult::kSuccess) {
+    LOG(ERROR) << "Policy fetch skipped with DM error: "
+               << static_cast<int>(result);
+    base::SequencedTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(request_callback_), result));
+    return;
+  }
+
+  cached_info_ = storage_->GetCachedPolicyInfo();
+  const std::string data =
+      GetPolicyFetchRequestData(kGoogleUpdateMachineLevelApps, *cached_info_);
+
+  // Authorization token is the DM token for policy fetch
+  const base::flat_map<std::string, std::string> additional_headers = {
+      {kAuthorizationHeader,
+       base::StringPrintf("%s token=%s", kPolicyFetchTokenType,
+                          dm_token.c_str())},
+  };
+
+  network_fetcher_->PostRequest(
+      BuildURL(kPolicyFetchRequestType), data, kDMContentType,
+      additional_headers,
+      base::BindOnce(&DMClient::OnRequestStarted, base::Unretained(this)),
+      base::BindRepeating(&DMClient::OnRequestProgress, base::Unretained(this)),
+      base::BindOnce(&DMClient::OnPolicyFetchRequestComplete,
+                     base::Unretained(this)));
+}
+
+void DMClient::OnPolicyFetchRequestComplete(
+    std::unique_ptr<std::string> response_body,
+    int net_error,
+    const std::string& header_etag,
+    const std::string& header_x_cup_server_proof,
+    int64_t xheader_retry_after_sec) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  RequestResult request_result = RequestResult::kSuccess;
+
+  if (net_error != 0) {
+    LOG(ERROR) << "DM policy fetch failed due to net error: " << net_error;
+    request_result = RequestResult::kNetworkError;
+  } else if (http_status_code_ == kHTTPStatusGone) {
+    VLOG(1) << "Device is now de-registered.";
+    storage_->DeregisterDevice();
+  } else if (http_status_code_ != kHTTPStatusOK) {
+    LOG(ERROR) << "DM policy fetch failed due to http error: "
+               << http_status_code_;
+    request_result = RequestResult::kHttpError;
+  } else {
+    DMPolicyMap policies = ParsePolicyFetchResponse(
+        *response_body, *cached_info_, storage_->GetDmToken(),
+        storage_->GetDeviceID());
+
+    if (policies.empty()) {
+      request_result = RequestResult::kUnexpectedResponse;
+    } else {
+      VLOG(1) << "Policy fetch request completed, got " << policies.size()
+              << " new policies.";
+      if (!storage_->PersistPolicies(policies))
+        request_result = RequestResult::kSerializationError;
     }
   }
 

chrome/updater/device_management/dm_client.h

@@ -22,6 +22,7 @@ class NetworkFetcher;
 
 namespace updater {
 
+class CachedPolicyInfo;
 class DMStorage;
 
 // This class is responsible for everything related to communication with the
@@ -63,6 +64,9 @@ class DMClient {
     // Request is not sent because the device is de-registered.
     kDeregistered,
 
+    // Policy fetch request is not sent because there's no DM token.
+    kNoDMToken,
+
     // Request is not sent because network fetcher fails to create.
     kFetcherError,
 
@@ -72,6 +76,9 @@ class DMClient {
     // Request failed with an HTTP error from server.
     kHttpError,
 
+    // Failed to persist the response into storage.
+    kSerializationError,
+
     // Got an unexpected response for the request.
     kUnexpectedResponse,
   };
@@ -92,6 +99,10 @@ class DMClient {
   // token is saved into the storage before |request_callback| is called.
   void PostRegisterRequest(DMRequestCallback request_callback);
 
+  // Posts a policy fetch request to the server. Upon success, new polices
+  // are saved into the storage before |request_callback| is called.
+  void PostPolicyFetchRequest(DMRequestCallback request_callback);
+
  private:
   // Gets the full request URL to DM server for the given request type.
   // Additional device specific values, such as device ID, platform etc. will
@@ -101,14 +112,21 @@ class DMClient {
   // Callback functions for the URLFetcher.
   void OnRequestStarted(int response_code, int64_t content_length);
   void OnRequestProgress(int64_t current);
-  void OnRequestComplete(std::unique_ptr<std::string> response_body,
-                         int net_error,
-                         const std::string& header_etag,
-                         const std::string& header_x_cup_server_proof,
-                         int64_t xheader_retry_after_sec);
+  void OnRegisterRequestComplete(std::unique_ptr<std::string> response_body,
+                                 int net_error,
+                                 const std::string& header_etag,
+                                 const std::string& header_x_cup_server_proof,
+                                 int64_t xheader_retry_after_sec);
+  void OnPolicyFetchRequestComplete(
+      std::unique_ptr<std::string> response_body,
+      int net_error,
+      const std::string& header_etag,
+      const std::string& header_x_cup_server_proof,
+      int64_t xheader_retry_after_sec);
 
   std::unique_ptr<Configurator> config_;
   scoped_refptr<DMStorage> storage_;
+  std::unique_ptr<CachedPolicyInfo> cached_info_;
 
   std::unique_ptr<update_client::NetworkFetcher> network_fetcher_;
   DMRequestCallback request_callback_;

chrome/updater/device_management/dm_client_unittest.cc

@@ -14,7 +14,12 @@
 #include "base/test/gmock_callback_support.h"
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
+#include "chrome/updater/constants.h"
+#include "chrome/updater/device_management/dm_cached_policy_info.h"
+#include "chrome/updater/device_management/dm_policy_builder_for_testing.h"
 #include "chrome/updater/device_management/dm_storage.h"
+#include "chrome/updater/policy_manager.h"
+#include "chrome/updater/unittest_util.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "components/update_client/network.h"
 #include "net/base/url_util.h"
@@ -44,7 +49,7 @@ class TestTokenService : public TokenServiceInterface {
   ~TestTokenService() override = default;
 
   // Overrides for TokenServiceInterface.
-  std::string GetDeviceID() const override { return "TestDeviceID"; }
+  std::string GetDeviceID() const override { return "test-device-id"; }
 
   bool StoreEnrollmentToken(const std::string& enrollment_token) override {
     enrollment_token_ = enrollment_token;
@@ -91,6 +96,117 @@ TestConfigurator::TestConfigurator(const GURL& url)
     : network_fetcher_factory_(base::MakeRefCounted<NetworkFetcherFactory>()),
       server_url_(url.spec()) {}
 
+class DMRequestCallbackHandler
+    : public base::RefCountedThreadSafe<DMRequestCallbackHandler> {
+ public:
+  DMRequestCallbackHandler() = default;
+  void OnRegisterRequestComplete(DMClient::RequestResult result) {
+    EXPECT_EQ(result, expected_result_);
+    if (result == DMClient::RequestResult::kSuccess ||
+        result == DMClient::RequestResult::kAleadyRegistered) {
+      EXPECT_EQ(storage_->GetDmToken(), "test-dm-token");
+    } else {
+      EXPECT_TRUE(storage_->GetDmToken().empty());
+    }
+    PostRequestCompleted();
+  }
+
+  void OnPolicyFetchRequestComplete(DMClient::RequestResult result) {
+    EXPECT_EQ(result, expected_result_);
+    if (expected_http_status_ == net::HTTP_GONE)
+      EXPECT_TRUE(storage_->IsDeviceDeregistered());
+
+    if (expected_http_status_ != net::HTTP_OK) {
+      PostRequestCompleted();
+      return;
+    }
+
+    std::unique_ptr<CachedPolicyInfo> info = storage_->GetCachedPolicyInfo();
+    EXPECT_FALSE(info->public_key().empty());
+    if (expect_new_public_key_)
+      EXPECT_EQ(info->public_key(), GetTestKey2()->GetPublicKeyString());
+    else
+      EXPECT_EQ(info->public_key(), GetTestKey1()->GetPublicKeyString());
+
+    if (result == DMClient::RequestResult::kSuccess) {
+      std::unique_ptr<PolicyManagerInterface> policy_manager =
+          storage_->GetOmahaPolicyManager();
+      EXPECT_NE(policy_manager, nullptr);
+
+      // Sample some of the policy values and check they are expected.
+      EXPECT_TRUE(policy_manager->IsManaged());
+      std::string proxy_mode;
+      EXPECT_TRUE(policy_manager->GetProxyMode(&proxy_mode));
+      EXPECT_EQ(proxy_mode, "test_proxy_mode");
+      int update_policy = 0;
+      EXPECT_TRUE(policy_manager->GetEffectivePolicyForAppUpdates(
+          kChromeAppId, &update_policy));
+      EXPECT_EQ(update_policy, kPolicyAutomaticUpdatesOnly);
+      std::string target_version_prefix;
+      EXPECT_TRUE(policy_manager->GetTargetVersionPrefix(
+          kChromeAppId, &target_version_prefix));
+      EXPECT_EQ(target_version_prefix, "81.");
+    }
+
+    PostRequestCompleted();
+  }
+
+  void OnDeregisterRequestComplete(DMClient::RequestResult result) {
+    EXPECT_EQ(result, DMClient::RequestResult::kSuccess);
+    EXPECT_TRUE(storage_->IsDeviceDeregistered());
+    PostRequestCompleted();
+  }
+
+  MOCK_METHOD0(PostRequestCompleted, void(void));
+
+  void CreateStorage(bool init_dm_token, bool init_cache_info) {
+    ASSERT_TRUE(storage_dir_.CreateUniqueTempDir());
+    constexpr char kEnrollmentToken[] = "TestEnrollmentToken";
+    constexpr char kDmToken[] = "test-dm-token";
+    storage_ = base::MakeRefCounted<DMStorage>(
+        storage_dir_.GetPath(),
+        std::make_unique<TestTokenService>(kEnrollmentToken,
+                                           init_dm_token ? kDmToken : ""));
+
+    if (init_cache_info) {
+      std::unique_ptr<::enterprise_management::DeviceManagementResponse>
+          dm_response = GetDefaultTestingPolicyFetchDMResponse(
+              /*first_request=*/true, /*rotate_to_new_key=*/false,
+              DMPolicyBuilderForTesting::SigningOption::kSignNormally);
+      std::unique_ptr<CachedPolicyInfo> info = storage_->GetCachedPolicyInfo();
+      DMPolicyMap policies = ParsePolicyFetchResponse(
+          dm_response->SerializeAsString(), *info.get(), storage_->GetDmToken(),
+          storage_->GetDeviceID());
+      ASSERT_FALSE(policies.empty());
+      ASSERT_TRUE(storage_->PersistPolicies(policies));
+    }
+  }
+
+  void SetExpectedHttpStatus(net::HttpStatusCode expected_http_status) {
+    expected_http_status_ = expected_http_status;
+  }
+  void SetExpectedRequestResult(DMClient::RequestResult expected_result) {
+    expected_result_ = expected_result;
+  }
+
+  void SetExpectNewPublicKey(bool expect_new_key) {
+    expect_new_public_key_ = expect_new_key;
+  }
+
+  scoped_refptr<DMStorage> GetStorage() { return storage_; }
+
+ private:
+  friend class base::RefCountedThreadSafe<DMRequestCallbackHandler>;
+  ~DMRequestCallbackHandler() = default;
+
+  base::ScopedTempDir storage_dir_;
+  scoped_refptr<DMStorage> storage_;
+
+  net::HttpStatusCode expected_http_status_ = net::HTTP_OK;
+  DMClient::RequestResult expected_result_ = DMClient::RequestResult::kSuccess;
+  bool expect_new_public_key_ = false;
+};
+
 }  // namespace
 
 class DMClientTest : public ::testing::Test {
@@ -112,26 +228,26 @@ class DMClientTest : public ::testing::Test {
     std::string device_id;
     EXPECT_TRUE(
         net::GetValueForKeyInQuery(request.GetURL(), "deviceid", &device_id));
-    EXPECT_EQ(device_id, "TestDeviceID");
+    EXPECT_EQ(device_id, "test-device-id");
 
     EXPECT_EQ(request.headers.at("Content-Type"), "application/x-protobuf");
 
     std::string request_type;
     EXPECT_TRUE(
         net::GetValueForKeyInQuery(request.GetURL(), "request", &request_type));
-    if (request_type == "register_policy_agent")
-      return HandleRegisterRequest(request);
-
-    auto http_response =
-        std::make_unique<net::test_server::BasicHttpResponse>();
-    http_response->set_code(net::HTTP_BAD_REQUEST);
-    return http_response;
-  }
-
-  std::unique_ptr<net::test_server::HttpResponse> HandleRegisterRequest(
-      const net::test_server::HttpRequest& request) {
-    std::string authorization = request.headers.at("Authorization");
-    EXPECT_EQ(authorization, "GoogleEnrollmentToken token=TestEnrollmentToken");
+    if (request_type == "register_policy_agent") {
+      std::string authorization = request.headers.at("Authorization");
+      EXPECT_EQ(authorization,
+                "GoogleEnrollmentToken token=TestEnrollmentToken");
+    } else if (request_type == "policy") {
+      std::string authorization = request.headers.at("Authorization");
+      EXPECT_EQ(authorization, "GoogleDMToken token=test-dm-token");
+    } else {
+      auto http_response =
+          std::make_unique<net::test_server::BasicHttpResponse>();
+      http_response->set_code(net::HTTP_BAD_REQUEST);
+      return http_response;
+    }
 
     auto http_response =
         std::make_unique<net::test_server::BasicHttpResponse>();
@@ -141,33 +257,6 @@ class DMClientTest : public ::testing::Test {
     return http_response;
   }
 
-  void OnRegisterRequestComplete(DMClient::RequestResult result) {
-    EXPECT_EQ(result, expected_result_);
-    if (result == DMClient::RequestResult::kSuccess ||
-        result == DMClient::RequestResult::kAleadyRegistered) {
-      EXPECT_EQ(storage_->GetDmToken(), "TestDMToken");
-    } else {
-      EXPECT_TRUE(storage_->GetDmToken().empty());
-    }
-    PostRequestCompleted();
-  }
-
-  void OnDeregisterRequestComplete(DMClient::RequestResult result) {
-    EXPECT_EQ(result, DMClient::RequestResult::kSuccess);
-    EXPECT_TRUE(storage_->IsDeviceDeregistered());
-    PostRequestCompleted();
-  }
-
-  MOCK_METHOD0(PostRequestCompleted, void(void));
-
-  void CreateStorage(const base::FilePath& root_path, bool initializeDMToken) {
-    constexpr char kEnrollmentToken[] = "TestEnrollmentToken";
-    constexpr char kDmToken[] = "TestDMToken";
-    storage_ = base::MakeRefCounted<DMStorage>(
-        root_path, std::make_unique<TestTokenService>(
-                       kEnrollmentToken, initializeDMToken ? kDmToken : ""));
-  }
-
   void StartTestServerWithResponse(net::HttpStatusCode http_status,
                                    const std::string& body) {
     response_http_status_ = http_status;
@@ -181,114 +270,307 @@ class DMClientTest : public ::testing::Test {
     auto dm_response =
         std::make_unique<enterprise_management::DeviceManagementResponse>();
     dm_response->mutable_register_response()->set_device_management_token(
-        "TestDMToken");
+        "test-dm-token");
     return dm_response->SerializeAsString();
   }
 
-  std::unique_ptr<DMClient> CreateDMClient() const {
+  void CreateDMClient() {
     const GURL url = test_server_.GetURL("/dm_api");
     auto test_config = std::make_unique<TestConfigurator>(url);
-    return std::make_unique<DMClient>(std::move(test_config), storage_);
-  }
-
-  void SetExpectedRequestResult(DMClient::RequestResult expected_result) {
-    expected_result_ = expected_result;
+    client_ = std::make_unique<DMClient>(std::move(test_config),
+                                         callback_handler_->GetStorage());
   }
 
-  scoped_refptr<DMStorage> storage_;
   net::EmbeddedTestServer test_server_;
   net::HttpStatusCode response_http_status_ = net::HTTP_OK;
   std::string response_body_;
 
-  DMClient::RequestResult expected_result_ = DMClient::RequestResult::kSuccess;
-
+  std::unique_ptr<DMClient> client_;
+  scoped_refptr<DMRequestCallbackHandler> callback_handler_;
   base::test::SingleThreadTaskEnvironment task_environment_;
 };
 
 TEST_F(DMClientTest, PostRegisterRequest_Success) {
-  base::ScopedTempDir cache_root;
-  ASSERT_TRUE(cache_root.CreateUniqueTempDir());
-  CreateStorage(cache_root.GetPath(), false /* init DM token */);
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/false,
+                                   /*init_cache_info=*/false);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kSuccess);
   StartTestServerWithResponse(net::HTTP_OK, GetDefaultDeviceRegisterResponse());
 
-  SetExpectedRequestResult(DMClient::RequestResult::kSuccess);
-
   base::RunLoop run_loop;
   base::RepeatingClosure quit_closure = run_loop.QuitClosure();
-  EXPECT_CALL(*this, PostRequestCompleted()).WillOnce(RunClosure(quit_closure));
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
 
-  std::unique_ptr<DMClient> test_client = CreateDMClient();
-  test_client->PostRegisterRequest(base::BindOnce(
-      &DMClientTest::OnRegisterRequestComplete, base::Unretained(this)));
+  CreateDMClient();
+  client_->PostRegisterRequest(base::BindOnce(
+      &DMRequestCallbackHandler::OnRegisterRequestComplete, callback_handler_));
   run_loop.Run();
 }
 
 TEST_F(DMClientTest, PostRegisterRequest_Deregister) {
-  base::ScopedTempDir cache_root;
-  ASSERT_TRUE(cache_root.CreateUniqueTempDir());
-  CreateStorage(cache_root.GetPath(), false /* init DM token */);
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/false,
+                                   /*init_cache_info=*/false);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kSuccess);
   StartTestServerWithResponse(net::HTTP_GONE, "" /* response body */);
 
-  SetExpectedRequestResult(DMClient::RequestResult::kSuccess);
-
   base::RunLoop run_loop;
   base::RepeatingClosure quit_closure = run_loop.QuitClosure();
-  EXPECT_CALL(*this, PostRequestCompleted()).WillOnce(RunClosure(quit_closure));
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
 
-  std::unique_ptr<DMClient> test_client = CreateDMClient();
-  test_client->PostRegisterRequest(base::BindOnce(
-      &DMClientTest::OnDeregisterRequestComplete, base::Unretained(this)));
+  CreateDMClient();
+  client_->PostRegisterRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnDeregisterRequestComplete,
+                     callback_handler_));
   run_loop.Run();
 }
 
 TEST_F(DMClientTest, PostRegisterRequest_BadRequest) {
-  base::ScopedTempDir cache_root;
-  ASSERT_TRUE(cache_root.CreateUniqueTempDir());
-  CreateStorage(cache_root.GetPath(), false /* init DM token */);
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/false,
+                                   /*init_cache_info=*/false);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kHttpError);
   StartTestServerWithResponse(net::HTTP_BAD_REQUEST, "" /* response body */);
 
-  SetExpectedRequestResult(DMClient::RequestResult::kHttpError);
   base::RunLoop run_loop;
   base::RepeatingClosure quit_closure = run_loop.QuitClosure();
-  EXPECT_CALL(*this, PostRequestCompleted()).WillOnce(RunClosure(quit_closure));
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
 
-  std::unique_ptr<DMClient> test_client = CreateDMClient();
-  test_client->PostRegisterRequest(base::BindOnce(
-      &DMClientTest::OnRegisterRequestComplete, base::Unretained(this)));
+  CreateDMClient();
+  client_->PostRegisterRequest(base::BindOnce(
+      &DMRequestCallbackHandler::OnRegisterRequestComplete, callback_handler_));
   run_loop.Run();
 }
 
 TEST_F(DMClientTest, PostRegisterRequest_AlreadyRegistered) {
-  base::ScopedTempDir cache_root;
-  ASSERT_TRUE(cache_root.CreateUniqueTempDir());
-  CreateStorage(cache_root.GetPath(), true /* init DM token */);
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/false);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kAleadyRegistered);
   StartTestServerWithResponse(net::HTTP_OK, GetDefaultDeviceRegisterResponse());
 
-  SetExpectedRequestResult(DMClient::RequestResult::kAleadyRegistered);
   base::RunLoop run_loop;
   base::RepeatingClosure quit_closure = run_loop.QuitClosure();
-  EXPECT_CALL(*this, PostRequestCompleted()).WillOnce(RunClosure(quit_closure));
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
 
-  std::unique_ptr<DMClient> test_client = CreateDMClient();
-  test_client->PostRegisterRequest(base::BindOnce(
-      &DMClientTest::OnRegisterRequestComplete, base::Unretained(this)));
+  CreateDMClient();
+  client_->PostRegisterRequest(base::BindOnce(
+      &DMRequestCallbackHandler::OnRegisterRequestComplete, callback_handler_));
   run_loop.Run();
 }
 
 TEST_F(DMClientTest, PostRegisterRequest_BadResponseData) {
-  base::ScopedTempDir cache_root;
-  ASSERT_TRUE(cache_root.CreateUniqueTempDir());
-  CreateStorage(cache_root.GetPath(), false /* init DM token */);
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/false,
+                                   /*init_cache_info=*/false);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kUnexpectedResponse);
   StartTestServerWithResponse(net::HTTP_OK, "BadResponseData");
 
-  SetExpectedRequestResult(DMClient::RequestResult::kUnexpectedResponse);
   base::RunLoop run_loop;
   base::RepeatingClosure quit_closure = run_loop.QuitClosure();
-  EXPECT_CALL(*this, PostRequestCompleted()).WillOnce(RunClosure(quit_closure));
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostRegisterRequest(base::BindOnce(
+      &DMRequestCallbackHandler::OnRegisterRequestComplete, callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch_FirstRequest) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/false);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kSuccess);
+
+  std::unique_ptr<::enterprise_management::DeviceManagementResponse>
+      dm_response = GetDefaultTestingPolicyFetchDMResponse(
+          /*first_request=*/true, /*rotate_to_new_key=*/false,
+          DMPolicyBuilderForTesting::SigningOption::kSignNormally);
+  StartTestServerWithResponse(net::HTTP_OK, dm_response->SerializeAsString());
+
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch_NoRotateKey) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kSuccess);
+
+  std::unique_ptr<::enterprise_management::DeviceManagementResponse>
+      dm_response = GetDefaultTestingPolicyFetchDMResponse(
+          /*first_request=*/false, /*rotate_to_new_key=*/false,
+          DMPolicyBuilderForTesting::SigningOption::kSignNormally);
+  StartTestServerWithResponse(net::HTTP_OK, dm_response->SerializeAsString());
+
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch_RotateKey) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kSuccess);
+  callback_handler_->SetExpectNewPublicKey(true);
+
+  std::unique_ptr<::enterprise_management::DeviceManagementResponse>
+      dm_response = GetDefaultTestingPolicyFetchDMResponse(
+          /*first_request=*/false, /*rotate_to_new_key=*/true,
+          DMPolicyBuilderForTesting::SigningOption::kSignNormally);
+  StartTestServerWithResponse(net::HTTP_OK, dm_response->SerializeAsString());
+
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch_RejectKeyWithBadSignature) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kUnexpectedResponse);
+  callback_handler_->SetExpectNewPublicKey(false);
+
+  std::unique_ptr<::enterprise_management::DeviceManagementResponse>
+      dm_response = GetDefaultTestingPolicyFetchDMResponse(
+          /*first_request=*/false, /*rotate_to_new_key=*/true,
+          DMPolicyBuilderForTesting::SigningOption::kTamperKeySignature);
+  StartTestServerWithResponse(net::HTTP_OK, dm_response->SerializeAsString());
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch_RejectDataWithBadSignature) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kUnexpectedResponse);
+  callback_handler_->SetExpectNewPublicKey(false);
+
+  std::unique_ptr<::enterprise_management::DeviceManagementResponse>
+      dm_response = GetDefaultTestingPolicyFetchDMResponse(
+          /*first_request=*/false, /*rotate_to_new_key=*/false,
+          DMPolicyBuilderForTesting::SigningOption::kTamperDataSignature);
+  StartTestServerWithResponse(net::HTTP_OK, dm_response->SerializeAsString());
+
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch__Deregister) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kSuccess);
+  callback_handler_->SetExpectedHttpStatus(net::HTTP_GONE);
+
+  StartTestServerWithResponse(net::HTTP_GONE, "" /* response body */);
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch__BadResponse) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kUnexpectedResponse);
+  StartTestServerWithResponse(net::HTTP_OK, "Unexpected response data");
+
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
+
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
+  run_loop.Run();
+}
+
+TEST_F(DMClientTest, PostPolicyFetch__BadRequest) {
+  callback_handler_ = base::MakeRefCounted<DMRequestCallbackHandler>();
+  callback_handler_->CreateStorage(/*init_dm_token=*/true,
+                                   /*init_cache_info=*/true);
+  callback_handler_->SetExpectedRequestResult(
+      DMClient::RequestResult::kHttpError);
+  StartTestServerWithResponse(net::HTTP_BAD_REQUEST, "" /* response body */);
+
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+  EXPECT_CALL(*callback_handler_, PostRequestCompleted())
+      .WillOnce(RunClosure(quit_closure));
 
-  std::unique_ptr<DMClient> test_client = CreateDMClient();
-  test_client->PostRegisterRequest(base::BindOnce(
-      &DMClientTest::OnRegisterRequestComplete, base::Unretained(this)));
+  CreateDMClient();
+  client_->PostPolicyFetchRequest(
+      base::BindOnce(&DMRequestCallbackHandler::OnPolicyFetchRequestComplete,
+                     callback_handler_));
   run_loop.Run();
 }
 

chrome/updater/device_management/dm_policy_builder_for_testing.cc

@@ -10,6 +10,7 @@
 #include "base/strings/string_util.h"
 #include "chrome/updater/device_management/dm_cached_policy_info.h"
 #include "chrome/updater/protos/omaha_settings.pb.h"
+#include "chrome/updater/unittest_util.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "crypto/rsa_private_key.h"
 #include "crypto/signature_creator.h"
@@ -136,6 +137,8 @@ const uint8_t kSigningKey2Signature[] = {
     0xDD, 0x6F, 0x80, 0xC3,
 };
 
+}  // namespace
+
 std::unique_ptr<DMSigningKeyForTesting> GetTestKey1() {
   constexpr int kFakeKeyVersion = 5;
   return std::make_unique<DMSigningKeyForTesting>(
@@ -150,8 +153,6 @@ std::unique_ptr<DMSigningKeyForTesting> GetTestKey2() {
       sizeof(kSigningKey2Signature), kFakeKeyVersion, "example.com");
 }
 
-}  // namespace
-
 std::unique_ptr<
     ::wireless_android_enterprise_devicemanagement::OmahaSettingsClientProto>
 GetDefaultTestingOmahaPolicyProto() {
@@ -172,7 +173,7 @@ GetDefaultTestingOmahaPolicyProto() {
       ::wireless_android_enterprise_devicemanagement::MANUAL_UPDATES_ONLY);
 
   ::wireless_android_enterprise_devicemanagement::ApplicationSettings app;
-  app.set_app_guid("{8A69D345-D564-463C-AFF1-A69D9E530F96}");
+  app.set_app_guid(kChromeAppId);
   app.set_install(
       ::wireless_android_enterprise_devicemanagement::INSTALL_DISABLED);
   app.set_update(

chrome/updater/device_management/dm_policy_builder_for_testing.h

@@ -61,6 +61,9 @@ class DMSigningKeyForTesting {
   std::string key_signature_domain_;
 };
 
+std::unique_ptr<DMSigningKeyForTesting> GetTestKey1();
+std::unique_ptr<DMSigningKeyForTesting> GetTestKey2();
+
 // Builds DM policy response.
 class DMPolicyBuilderForTesting {
  public:

chrome/updater/device_management/dm_policy_manager_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "build/build_config.h"
 #include "chrome/updater/constants.h"
+#include "chrome/updater/unittest_util.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -138,19 +139,18 @@ TEST(DMPolicyManager, PolicyManagerFromEmptyProto) {
   EXPECT_FALSE(policy_manager->GetPackageCacheExpirationTimeDays(&time_limit));
 
   // Verify app-specific polices.
-  const std::string chrome_guid = "{8A69D345-D564-463C-AFF1-A69D9E530F96}";
   int install_policy = -1;
   EXPECT_FALSE(policy_manager->GetEffectivePolicyForAppInstalls(
-      chrome_guid, &install_policy));
+      kChromeAppId, &install_policy));
   int update_policy = -1;
-  EXPECT_FALSE(policy_manager->GetEffectivePolicyForAppUpdates(chrome_guid,
+  EXPECT_FALSE(policy_manager->GetEffectivePolicyForAppUpdates(kChromeAppId,
                                                                &update_policy));
   bool rollback_allowed = false;
   EXPECT_FALSE(policy_manager->IsRollbackToTargetVersionAllowed(
-      chrome_guid, &rollback_allowed));
+      kChromeAppId, &rollback_allowed));
 
   std::string target_version_prefix;
-  EXPECT_FALSE(policy_manager->GetTargetVersionPrefix(chrome_guid,
+  EXPECT_FALSE(policy_manager->GetTargetVersionPrefix(kChromeAppId,
                                                       &target_version_prefix));
 }
 
@@ -171,7 +171,7 @@ TEST(DMPolicyManager, PolicyManagerFromProto) {
       ::wireless_android_enterprise_devicemanagement::MANUAL_UPDATES_ONLY);
 
   ::wireless_android_enterprise_devicemanagement::ApplicationSettings app;
-  app.set_app_guid("{8A69D345-D564-463C-AFF1-A69D9E530F96}");
+  app.set_app_guid(kChromeAppId);
   app.set_install(
       ::wireless_android_enterprise_devicemanagement::INSTALL_DISABLED);
   app.set_update(
@@ -223,22 +223,21 @@ TEST(DMPolicyManager, PolicyManagerFromProto) {
   EXPECT_FALSE(policy_manager->GetPackageCacheExpirationTimeDays(&time_limit));
 
   // Verify app-specific polices.
-  const std::string chrome_guid = "{8A69D345-D564-463C-AFF1-A69D9E530F96}";
   int install_policy = -1;
   EXPECT_TRUE(policy_manager->GetEffectivePolicyForAppInstalls(
-      chrome_guid, &install_policy));
+      kChromeAppId, &install_policy));
   EXPECT_EQ(install_policy, kPolicyDisabled);
   int update_policy = -1;
-  EXPECT_TRUE(policy_manager->GetEffectivePolicyForAppUpdates(chrome_guid,
+  EXPECT_TRUE(policy_manager->GetEffectivePolicyForAppUpdates(kChromeAppId,
                                                               &update_policy));
   EXPECT_EQ(update_policy, kPolicyAutomaticUpdatesOnly);
   bool rollback_allowed = false;
   EXPECT_TRUE(policy_manager->IsRollbackToTargetVersionAllowed(
-      chrome_guid, &rollback_allowed));
+      kChromeAppId, &rollback_allowed));
   EXPECT_TRUE(rollback_allowed);
 
   std::string target_version_prefix;
-  EXPECT_TRUE(policy_manager->GetTargetVersionPrefix(chrome_guid,
+  EXPECT_TRUE(policy_manager->GetTargetVersionPrefix(kChromeAppId,
                                                      &target_version_prefix));
   EXPECT_EQ(target_version_prefix, "81.");
 

chrome/updater/device_management/dm_storage.cc

@@ -95,16 +95,25 @@ bool DMStorage::IsDeviceDeregistered() const {
   return GetDmToken() == kInvalidTokenValue;
 }
 
-bool DMStorage::PersistPolicies(const std::string& policy_info_data,
-                                const DMPolicyMap& policy_map) const {
+bool DMStorage::PersistPolicies(const DMPolicyMap& policy_map) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  // Persists policy cached info
-  base::FilePath policy_info_file =
-      policy_cache_root_.AppendASCII(kPolicyInfoFileName);
-  if (!base::ImportantFileWriter::WriteFileAtomically(policy_info_file,
-                                                      policy_info_data)) {
-    return false;
+  if (policy_map.empty())
+    return true;
+
+  // Each policy in the map should be signed in the same way. If a policy
+  // in the map contains a public key, normally it means the server rotates the
+  // key. In this case, we persists the policy into the cached policy info file
+  // for future policy fetch.
+  const std::string policy_info_data = policy_map.cbegin()->second;
+  CachedPolicyInfo cached_info;
+  if (cached_info.Populate(policy_info_data) &&
+      !cached_info.public_key().empty()) {
+    base::FilePath policy_info_file =
+        policy_cache_root_.AppendASCII(kPolicyInfoFileName);
+    if (!base::ImportantFileWriter::WriteFileAtomically(policy_info_file,
+                                                        policy_info_data)) {
+      return false;
+    }
   }
 
   // Persists individual policies.
@@ -135,18 +144,18 @@ bool DMStorage::PersistPolicies(const std::string& policy_info_data,
 
 std::unique_ptr<CachedPolicyInfo> DMStorage::GetCachedPolicyInfo() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  auto cached_info = std::make_unique<CachedPolicyInfo>();
 
   if (!IsValidDMToken())
-    return nullptr;
+    return cached_info;
 
   base::FilePath policy_info_file =
       policy_cache_root_.AppendASCII(kPolicyInfoFileName);
   std::string policy_info_data;
-  auto cached_info = std::make_unique<CachedPolicyInfo>();
   if (!base::PathExists(policy_info_file) ||
       !base::ReadFileToString(policy_info_file, &policy_info_data) ||
       !cached_info->Populate(policy_info_data)) {
-    return nullptr;
+    return cached_info;
   }
 
   return cached_info;

chrome/updater/device_management/dm_storage.h

@@ -88,11 +88,11 @@ class DMStorage : public base::RefCountedThreadSafe<DMStorage> {
 
   // Persists DM policies.
   //
-  // |policy_info_data| is the serialized data of a PolicyFetchResponse. It will
-  // be saved into a fixed file named "CachedPolicyInfo" in cache root. The
-  // file content will be used to construct an updater::CachedPolicyInfo object
-  // to get public key, its version, and signing timestamp. The values will
-  // be used in subsequent policy fetches.
+  // If the first policy in the map contains a valid public key, its serialized
+  // data will be saved into a fixed file named "CachedPolicyInfo" in the cache
+  // root. The file content will be used to construct an
+  // updater::CachedPolicyInfo object to get public key, its version, and
+  // signing timestamp. The values will be used in subsequent policy fetches.
   //
   // Each entry in |policy_map| will be stored within a sub-directory named
   // {Base64Encoded{policy_type}}, with a fixed file name of
@@ -113,8 +113,7 @@ class DMStorage : public base::RefCountedThreadSafe<DMStorage> {
   //  ('Z29vZ2xlL21hY2hpbmUtbGV2ZWwtb21haGE=' is base64 encoding of
   //  "google/machine-level-omaha").
   //
-  bool PersistPolicies(const std::string& policy_info_data,
-                       const DMPolicyMap& policy_map) const;
+  bool PersistPolicies(const DMPolicyMap& policy_map) const;
 
   // Creates a CachedPolicyInfo object and populates it with the public key
   // information loaded from file |policy_cache_root_|\CachedPolicyInfo.

chrome/updater/device_management/dm_storage_unittest.cc

@@ -12,6 +12,7 @@
 #include "chrome/updater/device_management/dm_storage.h"
 #include "chrome/updater/policy_manager.h"
 #include "chrome/updater/protos/omaha_settings.pb.h"
+#include "chrome/updater/unittest_util.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -63,7 +64,7 @@ std::string CannedOmahaPolicyFetchResponse() {
       ::wireless_android_enterprise_devicemanagement::MANUAL_UPDATES_ONLY);
 
   ::wireless_android_enterprise_devicemanagement::ApplicationSettings app;
-  app.set_app_guid("{8A69D345-D564-463C-AFF1-A69D9E530F96}");
+  app.set_app_guid(kChromeAppId);
 
   app.set_install(
       ::wireless_android_enterprise_devicemanagement::INSTALL_DISABLED);
@@ -111,13 +112,10 @@ TEST(DMStorage, PersistPolicies) {
   EXPECT_TRUE(base::DirectoryExists(stale_poliy));
 
   auto storage = base::MakeRefCounted<DMStorage>(cache_root.GetPath());
-  EXPECT_TRUE(storage->PersistPolicies("policy-meta-data", policies));
+  EXPECT_TRUE(storage->PersistPolicies(policies));
   base::FilePath policy_info_file =
       cache_root.GetPath().AppendASCII("CachedPolicyInfo");
-  EXPECT_TRUE(base::PathExists(policy_info_file));
-  std::string policy_info_data;
-  EXPECT_TRUE(base::ReadFileToString(policy_info_file, &policy_info_data));
-  EXPECT_EQ(policy_info_data, "policy-meta-data");
+  EXPECT_FALSE(base::PathExists(policy_info_file));
 
   base::FilePath omaha_policy_file =
       cache_root.GetPath()
@@ -164,8 +162,9 @@ TEST(DMStorage, GetCachedPolicyInfo) {
   ASSERT_TRUE(cache_root.CreateUniqueTempDir());
   auto storage = base::MakeRefCounted<DMStorage>(
       cache_root.GetPath(), std::make_unique<TestTokenService>());
-  EXPECT_TRUE(storage->PersistPolicies(response.SerializeAsString(),
-                                       /* policies map */ {}));
+  EXPECT_TRUE(storage->PersistPolicies({
+      {"sample-policy-type", response.SerializeAsString()},
+  }));
 
   auto policy_info = storage->GetCachedPolicyInfo();
   ASSERT_NE(policy_info, nullptr);
@@ -176,17 +175,14 @@ TEST(DMStorage, GetCachedPolicyInfo) {
 }
 
 TEST(DMStorage, ReadCachedOmahaPolicy) {
-  // Persist the default testing omaha policy.
-  std::string omaha_policy_data = CannedOmahaPolicyFetchResponse();
-
   DMPolicyMap policies({
-      {"google/machine-level-omaha", omaha_policy_data},
+      {"google/machine-level-omaha", CannedOmahaPolicyFetchResponse()},
   });
   base::ScopedTempDir cache_root;
   ASSERT_TRUE(cache_root.CreateUniqueTempDir());
   auto storage = base::MakeRefCounted<DMStorage>(
       cache_root.GetPath(), std::make_unique<TestTokenService>());
-  EXPECT_TRUE(storage->PersistPolicies(omaha_policy_data, policies));
+  EXPECT_TRUE(storage->PersistPolicies(policies));
 
   auto policy_manager = storage->GetOmahaPolicyManager();
   ASSERT_NE(policy_manager, nullptr);
@@ -228,22 +224,21 @@ TEST(DMStorage, ReadCachedOmahaPolicy) {
   EXPECT_FALSE(policy_manager->GetPackageCacheExpirationTimeDays(&cache_life));
 
   // Chrome policies.
-  const std::string chrome_appid = "{8A69D345-D564-463C-AFF1-A69D9E530F96}";
   int chrome_install_policy = -1;
   EXPECT_TRUE(policy_manager->GetEffectivePolicyForAppInstalls(
-      chrome_appid, &chrome_install_policy));
+      kChromeAppId, &chrome_install_policy));
   EXPECT_EQ(chrome_install_policy, kPolicyDisabled);
   int chrome_update_policy = -1;
   EXPECT_TRUE(policy_manager->GetEffectivePolicyForAppUpdates(
-      chrome_appid, &chrome_update_policy));
+      kChromeAppId, &chrome_update_policy));
   EXPECT_EQ(chrome_update_policy, kPolicyAutomaticUpdatesOnly);
   std::string target_version_prefix;
-  EXPECT_TRUE(policy_manager->GetTargetVersionPrefix(chrome_appid,
+  EXPECT_TRUE(policy_manager->GetTargetVersionPrefix(kChromeAppId,
                                                      &target_version_prefix));
   EXPECT_EQ(target_version_prefix, "3.6.55");
   bool rollback_allowed = false;
   EXPECT_TRUE(policy_manager->IsRollbackToTargetVersionAllowed(
-      chrome_appid, &rollback_allowed));
+      kChromeAppId, &rollback_allowed));
   EXPECT_TRUE(rollback_allowed);
 
   // No app-specific policy should fallback to global.

chrome/updater/unittest_util.cc

@@ -8,6 +8,9 @@
 #include "chrome/updater/tag.h"
 
 namespace updater {
+
+const char kChromeAppId[] = "{8A69D345-D564-463C-AFF1-A69D9E530F96}";
+
 namespace tagging {
 
 std::ostream& operator<<(std::ostream& os, const ErrorCode& error_code) {

chrome/updater/unittest_util.h

@@ -29,6 +29,8 @@ std::ostream& operator<<(std::ostream& os, const base::Optional<T>& opt) {
 // Externally-defined printers for chrome/updater-related types.
 namespace updater {
 
+extern const char kChromeAppId[];
+
 namespace tagging {
 std::ostream& operator<<(std::ostream&, const ErrorCode&);
 std::ostream& operator<<(std::ostream&, const AppArgs::NeedsAdmin&);