Implement DevicePrintServersExternalDataHandler

This external data policy is used to handle the
DeviceExternalPrintServers policy which corresponds with the existing
ExternalPrintServers policy used to manage a list of print servers.

Bug: 1100777
Change-Id: I86028c0451212e506001bbdd8dae8010e88d8608
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290968
Commit-Queue: Matt Menard <mattme@google.com>
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Nikita Podguzov <nikitapodguzov@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796568}

chrome/browser/chromeos/BUILD.gn

@@ -3320,6 +3320,7 @@ source_set("unit_tests") {
     "policy/extension_install_event_log_uploader_unittest.cc",
     "policy/extension_install_event_logger_unittest.cc",
     "policy/external_data_handlers/device_native_printers_external_data_handler_unittest.cc",
+    "policy/external_data_handlers/device_print_servers_external_data_handler_unittest.cc",
     "policy/fake_affiliated_invalidation_service_provider.cc",
     "policy/fake_affiliated_invalidation_service_provider.h",
     "policy/heartbeat_scheduler_unittest.cc",

chrome/browser/chromeos/policy/external_data_handlers/device_print_servers_external_data_handler.cc

@@ -6,8 +6,19 @@
 
 #include <utility>
 
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider_factory.h"
 #include "components/policy/policy_constants.h"
 
+namespace {
+
+base::WeakPtr<chromeos::PrintServersProvider> GetDevicePrintServersProvider() {
+  return chromeos::PrintServersProviderFactory::Get()->GetForDevice();
+}
+
+}  // namespace
+
 namespace policy {
 
 DevicePrintServersExternalDataHandler::DevicePrintServersExternalDataHandler(
@@ -21,16 +32,21 @@ DevicePrintServersExternalDataHandler::DevicePrintServersExternalDataHandler(
 DevicePrintServersExternalDataHandler::
     ~DevicePrintServersExternalDataHandler() = default;
 
+void DevicePrintServersExternalDataHandler::OnDeviceExternalDataSet(
+    const std::string& policy) {
+  GetDevicePrintServersProvider()->ClearData();
+}
+
 void DevicePrintServersExternalDataHandler::OnDeviceExternalDataCleared(
     const std::string& policy) {
-  // TODO(b/123933434): Handle when data is cleared.
+  GetDevicePrintServersProvider()->ClearData();
 }
 
 void DevicePrintServersExternalDataHandler::OnDeviceExternalDataFetched(
     const std::string& policy,
     std::unique_ptr<std::string> data,
     const base::FilePath& file_path) {
-  // TODO(b/123933434): Handle |data|.
+  GetDevicePrintServersProvider()->SetData(std::move(data));
 }
 
 void DevicePrintServersExternalDataHandler::Shutdown() {

chrome/browser/chromeos/policy/external_data_handlers/device_print_servers_external_data_handler.h

@@ -28,6 +28,7 @@ class DevicePrintServersExternalDataHandler
   ~DevicePrintServersExternalDataHandler() override;
 
   // DeviceCloudExternalDataPolicyHandler:
+  void OnDeviceExternalDataSet(const std::string& policy) override;
   void OnDeviceExternalDataCleared(const std::string& policy) override;
   void OnDeviceExternalDataFetched(const std::string& policy,
                                    std::unique_ptr<std::string> data,

chrome/browser/chromeos/policy/external_data_handlers/device_print_servers_external_data_handler_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/policy/external_data_handlers/device_print_servers_external_data_handler.h"
+
+#include <memory>
+#include <string>
+
+#include "chrome/browser/chromeos/printing/print_server.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider_factory.h"
+#include "components/policy/core/common/mock_policy_service.h"
+#include "components/policy/core/common/policy_namespace.h"
+#include "components/policy/policy_constants.h"
+#include "components/policy/proto/chrome_device_policy.pb.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::Pointee;
+using ::testing::UnorderedElementsAre;
+
+namespace policy {
+
+namespace {
+
+// An example device native printers configuration file.
+constexpr char kDeviceExternalPrintServersContentsJson[] = R"json(
+[
+  {
+    "id": "First",
+    "display_name": "LexaPrint",
+    "url": "ipp://192.168.1.5",
+  }, {
+    "id": "Second",
+    "display_name": "Color Laser",
+    "url":"ipps://print-server.intranet.example.com:443/ipp/cl2k4",
+  }
+])json";
+
+constexpr char kAllowlistPrefName[] = "test";
+
+class TestObserver : public chromeos::PrintServersProvider::Observer {
+ public:
+  ~TestObserver() override = default;
+
+  // Callback from PrintServersProvider::Observer.
+  void OnServersChanged(
+      bool complete,
+      const std::vector<chromeos::PrintServer>& servers) override {
+    print_servers_ = servers;
+  }
+
+  const std::vector<chromeos::PrintServer>* GetPrintServers() {
+    return &print_servers_;
+  }
+
+ private:
+  std::vector<chromeos::PrintServer> print_servers_;
+};
+
+}  // namespace
+
+class DevicePrintServersExternalDataHandlerTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    EXPECT_CALL(policy_service_,
+                AddObserver(policy::POLICY_DOMAIN_CHROME, testing::_))
+        .Times(1);
+    EXPECT_CALL(policy_service_,
+                RemoveObserver(policy::POLICY_DOMAIN_CHROME, testing::_))
+        .Times(1);
+
+    print_servers_provider_ =
+        chromeos::PrintServersProviderFactory::Get()->GetForDevice();
+    pref_service_.registry()->RegisterListPref(kAllowlistPrefName);
+    print_servers_provider_->SetAllowlistPref(&pref_service_,
+                                              kAllowlistPrefName);
+    device_print_servers_external_data_handler_ =
+        std::make_unique<DevicePrintServersExternalDataHandler>(
+            &policy_service_);
+  }
+
+  void TearDown() override {
+    chromeos::PrintServersProviderFactory::Get()->Shutdown();
+    device_print_servers_external_data_handler_->Shutdown();
+  }
+
+  // Everything must be called on Chrome_UIThread.
+  content::BrowserTaskEnvironment task_environment_;
+
+  sync_preferences::TestingPrefServiceSyncable pref_service_;
+
+  MockPolicyService policy_service_;
+  std::unique_ptr<DevicePrintServersExternalDataHandler>
+      device_print_servers_external_data_handler_;
+  base::WeakPtr<chromeos::PrintServersProvider> print_servers_provider_;
+};
+
+TEST_F(DevicePrintServersExternalDataHandlerTest, OnDataFetched) {
+  TestObserver obs;
+  print_servers_provider_->AddObserver(&obs);
+  EXPECT_TRUE(obs.GetPrintServers()->empty());
+
+  device_print_servers_external_data_handler_->OnDeviceExternalDataSet(
+      key::kDeviceExternalPrintServers);
+  device_print_servers_external_data_handler_->OnDeviceExternalDataFetched(
+      key::kDeviceExternalPrintServers,
+      std::make_unique<std::string>(kDeviceExternalPrintServersContentsJson),
+      base::FilePath());
+  task_environment_.RunUntilIdle();
+
+  chromeos::PrintServer first("First", GURL("http://192.168.1.5:631"),
+                              "LexaPrint");
+  chromeos::PrintServer second(
+      "Second", GURL("https://print-server.intranet.example.com:443/ipp/cl2k4"),
+      "Color Laser");
+
+  EXPECT_THAT(obs.GetPrintServers(),
+              Pointee(UnorderedElementsAre(first, second)));
+}
+
+TEST_F(DevicePrintServersExternalDataHandlerTest, OnDataCleared) {
+  TestObserver obs;
+  print_servers_provider_->AddObserver(&obs);
+  EXPECT_TRUE(obs.GetPrintServers()->empty());
+
+  device_print_servers_external_data_handler_->OnDeviceExternalDataSet(
+      key::kDeviceExternalPrintServers);
+  device_print_servers_external_data_handler_->OnDeviceExternalDataFetched(
+      key::kDeviceExternalPrintServers,
+      std::make_unique<std::string>(kDeviceExternalPrintServersContentsJson),
+      base::FilePath());
+  device_print_servers_external_data_handler_->OnDeviceExternalDataCleared(
+      key::kDeviceExternalPrintServers);
+  task_environment_.RunUntilIdle();
+
+  EXPECT_TRUE(obs.GetPrintServers()->empty());
+}
+
+}  // namespace policy

chrome/browser/chromeos/printing/print_servers_provider_factory.cc

@@ -43,6 +43,15 @@ base::WeakPtr<PrintServersProvider> PrintServersProviderFactory::GetForProfile(
   return GetForAccountId(user->GetAccountId());
 }
 
+base::WeakPtr<PrintServersProvider>
+PrintServersProviderFactory::GetForDevice() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!device_provider_) {
+    device_provider_ = PrintServersProvider::Create();
+  }
+  return device_provider_->AsWeakPtr();
+}
+
 void PrintServersProviderFactory::RemoveForAccountId(
     const AccountId& account_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -52,6 +61,7 @@ void PrintServersProviderFactory::RemoveForAccountId(
 void PrintServersProviderFactory::Shutdown() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   providers_by_user_.clear();
+  device_provider_ = nullptr;
 }
 
 PrintServersProviderFactory::PrintServersProviderFactory() = default;

chrome/browser/chromeos/printing/print_servers_provider_factory.h

@@ -38,6 +38,12 @@ class PrintServersProviderFactory {
   // returned object remains valid until RemoveForUserId or Shutdown is called.
   base::WeakPtr<PrintServersProvider> GetForProfile(Profile* profile);
 
+  // Returns a WeakPtr to the PrintServersProvider registered for the device.
+  // If requested PrintServersProvider does not exist, the object is
+  // created and registered. The returned object remains valid until Shutdown is
+  // called. Returns nullptr if called after Shutdown or during unit tests.
+  base::WeakPtr<PrintServersProvider> GetForDevice();
+
   // Deletes the PrintServersProvider registered for |account_id|.
   void RemoveForAccountId(const AccountId& account_id);
 
@@ -49,6 +55,8 @@ class PrintServersProviderFactory {
 
   std::map<AccountId, std::unique_ptr<PrintServersProvider>> providers_by_user_;
 
+  std::unique_ptr<PrintServersProvider> device_provider_;
+
   DISALLOW_COPY_AND_ASSIGN(PrintServersProviderFactory);
 };