Create support for scaled print servers beyond 16

Bug: b:168650771
Change-Id: Iee395f07b4a2f76bcca092cbec37e036e5669e7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2456313Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Piotr Pawliczek <pawliczek@chromium.org>
Reviewed-by: Brian Malcolm <bmalcolm@chromium.org>
Commit-Queue: Matt Menard <mattme@google.com>
Cr-Commit-Position: refs/heads/master@{#822210}

chrome/browser/chromeos/BUILD.gn

@@ -2503,6 +2503,8 @@ source_set("chromeos") {
     "printing/print_management/printing_manager_factory.h",
     "printing/print_server.cc",
     "printing/print_server.h",
+    "printing/print_servers_policy_provider.cc",
+    "printing/print_servers_policy_provider.h",
     "printing/print_servers_provider.cc",
     "printing/print_servers_provider.h",
     "printing/print_servers_provider_factory.cc",
@@ -2526,8 +2528,6 @@ source_set("chromeos") {
     "printing/server_printers_fetcher.h",
     "printing/server_printers_provider.cc",
     "printing/server_printers_provider.h",
-    "printing/server_printers_provider_factory.cc",
-    "printing/server_printers_provider_factory.h",
     "printing/specifics_translation.cc",
     "printing/specifics_translation.h",
     "printing/synced_printers_manager.cc",

chrome/browser/chromeos/printing/cups_printers_manager.cc

@@ -20,6 +20,7 @@
 #include "chrome/browser/chromeos/printing/enterprise_printers_provider.h"
 #include "chrome/browser/chromeos/printing/ppd_provider_factory.h"
 #include "chrome/browser/chromeos/printing/ppd_resolution_tracker.h"
+#include "chrome/browser/chromeos/printing/print_servers_policy_provider.h"
 #include "chrome/browser/chromeos/printing/print_servers_provider.h"
 #include "chrome/browser/chromeos/printing/printer_configurer.h"
 #include "chrome/browser/chromeos/printing/printer_event_tracker.h"
@@ -27,7 +28,6 @@
 #include "chrome/browser/chromeos/printing/printer_info.h"
 #include "chrome/browser/chromeos/printing/printers_map.h"
 #include "chrome/browser/chromeos/printing/server_printers_provider.h"
-#include "chrome/browser/chromeos/printing/server_printers_provider_factory.h"
 #include "chrome/browser/chromeos/printing/synced_printers_manager.h"
 #include "chrome/browser/chromeos/printing/synced_printers_manager_factory.h"
 #include "chrome/browser/chromeos/printing/usb_printer_detector.h"
@@ -77,7 +77,8 @@ class CupsPrintersManagerImpl
       std::unique_ptr<PrinterConfigurer> printer_configurer,
       std::unique_ptr<UsbPrinterNotificationController>
           usb_notification_controller,
-      ServerPrintersProvider* server_printers_provider,
+      std::unique_ptr<PrintServersPolicyProvider> print_servers_provider,
+      std::unique_ptr<ServerPrintersProvider> server_printers_provider,
       std::unique_ptr<EnterprisePrintersProvider> enterprise_printers_provider,
       PrinterEventTracker* event_tracker,
       PrefService* pref_service)
@@ -90,7 +91,8 @@ class CupsPrintersManagerImpl
         auto_usb_printer_configurer_(std::move(printer_configurer),
                                      this,
                                      usb_notification_controller_.get()),
-        server_printers_provider_(server_printers_provider),
+        print_servers_provider_(std::move(print_servers_provider)),
+        server_printers_provider_(std::move(server_printers_provider)),
         enterprise_printers_provider_(std::move(enterprise_printers_provider)),
         enterprise_printers_provider_observer_(this),
         event_tracker_(event_tracker) {
@@ -125,6 +127,9 @@ class CupsPrintersManagerImpl
                             weak_ptr_factory_.GetWeakPtr(), kZeroconfDetector));
     OnPrintersFound(kZeroconfDetector, zeroconf_detector_->GetPrinters());
 
+    print_servers_provider_->SetListener(
+        base::BindRepeating(&CupsPrintersManagerImpl::OnPrintServersUpdated,
+                            weak_ptr_factory_.GetWeakPtr()));
     server_printers_provider_->RegisterPrintersFoundCallback(
         base::BindRepeating(&CupsPrintersManagerImpl::OnPrintersUpdated,
                             weak_ptr_factory_.GetWeakPtr()));
@@ -463,6 +468,68 @@ class CupsPrintersManagerImpl
     }
   }
 
+  void OnPrintServersUpdated(bool is_complete,
+                             std::map<GURL, PrintServer> print_servers,
+                             ServerPrintersFetchingMode fetching_mode) {
+    fetching_mode_ = fetching_mode;
+    if (!is_complete) {
+      return;
+    }
+
+    print_servers_ = std::map<std::string, PrintServer>();
+    for (const auto& server_pair : print_servers) {
+      const PrintServer& server = server_pair.second;
+      print_servers_.value().emplace(server.GetId(), server);
+    }
+
+    if (fetching_mode_ == ServerPrintersFetchingMode::kSingleServerOnly) {
+      // If the previously selected print server is unavailable, set to the
+      // first print server in the list, or set to none if there are no print
+      // servers.
+      auto& servers = print_servers_.value();
+      if (!selected_print_server_id_.has_value() ||
+          !ChoosePrintServer(selected_print_server_id_)) {
+        auto first_id =
+            servers.empty()
+                ? base::nullopt
+                : base::make_optional(servers.begin()->second.GetId());
+        ChoosePrintServer(first_id);
+      }
+    } else {
+      selected_print_server_id_ = base::nullopt;
+      server_printers_provider_->OnServersChanged(true, print_servers);
+    }
+  }
+
+  // Public API function.
+  bool ChoosePrintServer(
+      const base::Optional<std::string>& selected_print_server_id) override {
+    if (fetching_mode_ != ServerPrintersFetchingMode::kSingleServerOnly ||
+        !print_servers_.has_value()) {
+      return false;
+    }
+
+    std::map<GURL, PrintServer> selected_print_servers;
+    if (selected_print_server_id.has_value()) {
+      auto iter = print_servers_.value().find(selected_print_server_id.value());
+      if (iter != print_servers_.value().end()) {
+        const PrintServer& server = iter->second;
+        selected_print_servers.emplace(server.GetUrl(), server);
+      } else {
+        // A selected value was given that is not available
+        return false;
+      }
+    }
+    selected_print_server_id_ = selected_print_server_id;
+    server_printers_provider_->OnServersChanged(true, selected_print_servers);
+    return true;
+  }
+
+  // Public API function.
+  ServerPrintersFetchingMode GetServerPrintersFetchingMode() const override {
+    return fetching_mode_;
+  }
+
  private:
   base::Optional<Printer> GetEnterprisePrinter(const std::string& id) const {
     return printers_.Get(PrinterClass::kEnterprise, id);
@@ -729,8 +796,16 @@ class CupsPrintersManagerImpl
 
   AutomaticUsbPrinterConfigurer auto_usb_printer_configurer_;
 
-  // Not owned.
-  ServerPrintersProvider* server_printers_provider_;
+  std::unique_ptr<PrintServersPolicyProvider> print_servers_provider_;
+
+  // The print server ID that is the current selection, if any.
+  base::Optional<std::string> selected_print_server_id_;
+
+  ServerPrintersFetchingMode fetching_mode_;
+
+  base::Optional<std::map<std::string, PrintServer>> print_servers_;
+
+  std::unique_ptr<ServerPrintersProvider> server_printers_provider_;
 
   std::unique_ptr<EnterprisePrintersProvider> enterprise_printers_provider_;
   ScopedObserver<EnterprisePrintersProvider,
@@ -781,8 +856,8 @@ std::unique_ptr<CupsPrintersManager> CupsPrintersManager::Create(
       UsbPrinterDetector::Create(), ZeroconfPrinterDetector::Create(),
       CreatePpdProvider(profile), PrinterConfigurer::Create(profile),
       UsbPrinterNotificationController::Create(profile),
-      ServerPrintersProviderFactory::GetInstance()->GetForBrowserContext(
-          profile),
+      PrintServersPolicyProvider::Create(profile),
+      ServerPrintersProvider::Create(),
       EnterprisePrintersProvider::Create(CrosSettings::Get(), profile),
       PrinterEventTrackerFactory::GetInstance()->GetForBrowserContext(profile),
       profile->GetPrefs());
@@ -797,7 +872,8 @@ std::unique_ptr<CupsPrintersManager> CupsPrintersManager::CreateForTesting(
     std::unique_ptr<PrinterConfigurer> printer_configurer,
     std::unique_ptr<UsbPrinterNotificationController>
         usb_notification_controller,
-    ServerPrintersProvider* server_printers_provider,
+    std::unique_ptr<ServerPrintersProvider> server_printers_provider,
+    std::unique_ptr<PrintServersPolicyProvider> print_servers_provider,
     std::unique_ptr<EnterprisePrintersProvider> enterprise_printers_provider,
     PrinterEventTracker* event_tracker,
     PrefService* pref_service) {
@@ -805,8 +881,8 @@ std::unique_ptr<CupsPrintersManager> CupsPrintersManager::CreateForTesting(
       synced_printers_manager, std::move(usb_detector),
       std::move(zeroconf_detector), std::move(ppd_provider),
       std::move(printer_configurer), std::move(usb_notification_controller),
-      server_printers_provider, std::move(enterprise_printers_provider),
-      event_tracker, pref_service);
+      std::move(print_servers_provider), std::move(server_printers_provider),
+      std::move(enterprise_printers_provider), event_tracker, pref_service);
 }
 
 // static

chrome/browser/chromeos/printing/cups_printers_manager.h

@@ -10,6 +10,7 @@
 
 #include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
+#include "chrome/browser/chromeos/printing/print_servers_policy_provider.h"
 #include "chrome/browser/chromeos/printing/printer_installation_manager.h"
 #include "chromeos/printing/printer_configuration.h"
 #include "chromeos/printing/uri.h"
@@ -72,7 +73,8 @@ class CupsPrintersManager : public PrinterInstallationManager,
       std::unique_ptr<PrinterConfigurer> printer_configurer,
       std::unique_ptr<UsbPrinterNotificationController>
           usb_notification_controller,
-      ServerPrintersProvider* server_printers_provider,
+      std::unique_ptr<ServerPrintersProvider> server_printers_provider,
+      std::unique_ptr<PrintServersPolicyProvider> print_servers_provider,
       std::unique_ptr<EnterprisePrintersProvider> enterprise_printers_provider,
       PrinterEventTracker* event_tracker,
       PrefService* pref_service);
@@ -129,6 +131,14 @@ class CupsPrintersManager : public PrinterInstallationManager,
   // Records the total number of detected network printers and the
   // number of detected network printers that have not been saved.
   virtual void RecordNearbyNetworkPrinterCounts() const = 0;
+
+  // Selects a print server from all the available print servers. Returns true on
+  // successfully selecting the requested print server.
+  virtual bool ChoosePrintServer(
+      const base::Optional<std::string>& selected_print_server_id) = 0;
+
+  // Returns the current fetching mode strategy for print servers.
+  virtual ServerPrintersFetchingMode GetServerPrintersFetchingMode() const = 0;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/printing/cups_printers_manager_factory.cc

@@ -7,7 +7,6 @@
 #include "base/memory/singleton.h"
 #include "chrome/browser/chromeos/printing/cups_printers_manager.h"
 #include "chrome/browser/chromeos/printing/cups_printers_manager_proxy.h"
-#include "chrome/browser/chromeos/printing/server_printers_provider_factory.h"
 #include "chrome/browser/chromeos/printing/synced_printers_manager_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
@@ -33,7 +32,6 @@ CupsPrintersManagerFactory::CupsPrintersManagerFactory()
           "CupsPrintersManagerFactory",
           BrowserContextDependencyManager::GetInstance()),
       proxy_(CupsPrintersManagerProxy::Create()) {
-  DependsOn(chromeos::ServerPrintersProviderFactory::GetInstance());
   DependsOn(chromeos::SyncedPrintersManagerFactory::GetInstance());
 }
 

chrome/browser/chromeos/printing/cups_printers_manager_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/printing/cups_printers_manager.h"
 
 #include <algorithm>
+#include <map>
 #include <memory>
 #include <string>
 #include <unordered_set>
@@ -28,8 +29,11 @@
 #include "chrome/browser/chromeos/printing/usb_printer_detector.h"
 #include "chrome/browser/chromeos/printing/usb_printer_notification_controller.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "components/prefs/pref_service.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"
 
 namespace chromeos {
@@ -333,10 +337,55 @@ class FakeServerPrintersProvider : public ServerPrintersProvider {
 
   void RegisterPrintersFoundCallback(OnPrintersUpdateCallback cb) override {}
 
+  void OnServersChanged(bool servers_are_complete,
+                        const std::map<GURL, PrintServer>& servers) override {
+    std::vector<chromeos::PrintServer> print_servers;
+    for (auto& server_pair : servers) {
+      print_servers.push_back(server_pair.second);
+    }
+    print_servers_ = print_servers;
+  }
+
   std::vector<PrinterDetector::DetectedPrinter> GetPrinters() override {
     std::vector<PrinterDetector::DetectedPrinter> printers;
     return printers;
   }
+
+  std::vector<chromeos::PrintServer> GetPrintServers() {
+    return print_servers_;
+  }
+
+ private:
+  std::vector<chromeos::PrintServer> print_servers_;
+};
+
+class FakePrintServersProvider : public PrintServersProvider {
+ public:
+  FakePrintServersProvider() = default;
+  ~FakePrintServersProvider() override = default;
+
+  void AddObserver(Observer* observer) override { observer_ = observer; }
+  void RemoveObserver(Observer* observer) override {}
+  void SetData(std::unique_ptr<std::string> data) override {}
+  void SetAllowlistPref(PrefService* prefs,
+                        const std::string& allowlist_pref) override {}
+  void ClearData() override {}
+
+  base::Optional<std::vector<PrintServer>> GetPrintServers() override {
+    return print_servers_;
+  }
+
+  void SetPrintServers(base::Optional<std::vector<PrintServer>> print_servers) {
+    print_servers_ = print_servers;
+    if (observer_) {
+      observer_->OnServersChanged(print_servers.has_value(),
+                                  print_servers.value());
+    }
+  }
+
+ private:
+  base::Optional<std::vector<PrintServer>> print_servers_;
+  PrintServersProvider::Observer* observer_;
 };
 
 class CupsPrintersManagerTest : public testing::Test,
@@ -349,6 +398,9 @@ class CupsPrintersManagerTest : public testing::Test,
     zeroconf_detector_ = zeroconf_detector.get();
     auto usb_detector = std::make_unique<FakePrinterDetector>();
     usb_detector_ = usb_detector.get();
+    auto server_printers_provider =
+        std::make_unique<FakeServerPrintersProvider>();
+    server_printers_provider_ = server_printers_provider.get();
     auto printer_configurer = std::make_unique<TestPrinterConfigurer>();
     printer_configurer_ = printer_configurer.get();
     auto usb_notif_controller =
@@ -357,6 +409,10 @@ class CupsPrintersManagerTest : public testing::Test,
     auto enterprise_printers_provider =
         std::make_unique<FakeEnterprisePrintersProvider>();
     enterprise_printers_provider_ = enterprise_printers_provider.get();
+    auto print_servers_policy_provider =
+        PrintServersPolicyProvider::CreateForTesting(
+            user_policy_print_servers_provider_.AsWeakPtr(),
+            device_policy_print_servers_provider_.AsWeakPtr());
 
     // Register the pref |UserPrintersAllowed|
     CupsPrintersManager::RegisterProfilePrefs(pref_service_.registry());
@@ -365,8 +421,10 @@ class CupsPrintersManagerTest : public testing::Test,
         &synced_printers_manager_, std::move(usb_detector),
         std::move(zeroconf_detector), ppd_provider_,
         std::move(printer_configurer), std::move(usb_notif_controller),
-        &server_printers_provider_, std::move(enterprise_printers_provider),
-        &event_tracker_, &pref_service_);
+        std::move(server_printers_provider),
+        std::move(print_servers_policy_provider),
+        std::move(enterprise_printers_provider), &event_tracker_,
+        &pref_service_);
     manager_->AddObserver(this);
   }
 
@@ -393,8 +451,17 @@ class CupsPrintersManagerTest : public testing::Test,
     pref_service_.SetManagedPref(name, std::move(value_ptr));
   }
 
+  static chromeos::PrintServer CreatePrintServer(std::string id,
+                                                 std::string server_url,
+                                                 std::string name) {
+    GURL url(server_url);
+    chromeos::PrintServer print_server(id, url, name);
+    return print_server;
+  }
+
  protected:
-  base::test::TaskEnvironment task_environment_;
+  // Everything from PrintServersProvider must be called on Chrome_UIThread
+  content::BrowserTaskEnvironment task_environment_;
 
   // Captured printer lists from observer callbacks.
   base::flat_map<PrinterClass, std::vector<Printer>> observed_printers_;
@@ -402,11 +469,13 @@ class CupsPrintersManagerTest : public testing::Test,
   // Backend fakes driving the CupsPrintersManager.
   FakeSyncedPrintersManager synced_printers_manager_;
   FakeEnterprisePrintersProvider* enterprise_printers_provider_;  // Not owned.
-  FakePrinterDetector* usb_detector_;          // Not owned.
-  FakePrinterDetector* zeroconf_detector_;     // Not owned.
-  TestPrinterConfigurer* printer_configurer_;  // Not owned.
-  FakeUsbPrinterNotificationController* usb_notif_controller_;  // Not owned.
-  FakeServerPrintersProvider server_printers_provider_;
+  FakePrinterDetector* usb_detector_;                             // Not owned.
+  FakePrinterDetector* zeroconf_detector_;                        // Not owned.
+  TestPrinterConfigurer* printer_configurer_;                     // Not owned.
+  FakeUsbPrinterNotificationController* usb_notif_controller_;    // Not owned.
+  FakeServerPrintersProvider* server_printers_provider_;
+  FakePrintServersProvider user_policy_print_servers_provider_;
+  FakePrintServersProvider device_policy_print_servers_provider_;
   scoped_refptr<FakePpdProvider> ppd_provider_;
 
   // This is unused, it's just here for memory ownership.
@@ -932,5 +1001,54 @@ TEST_F(CupsPrintersManagerTest, RecordNearbyNetworkPrinterCounts) {
                                      2, 1);
 }
 
+TEST_F(CupsPrintersManagerTest, GetServerPrinters_StandardMode) {
+  EXPECT_TRUE(server_printers_provider_->GetPrinters().empty());
+
+  std::vector<chromeos::PrintServer> user_print_servers;
+  auto user_print_server =
+      CreatePrintServer("1", "http://192.168.1.5/user-printer", "LexaPrint");
+  user_print_servers.push_back(user_print_server);
+  user_policy_print_servers_provider_.SetPrintServers(user_print_servers);
+  std::vector<chromeos::PrintServer> device_print_servers;
+  auto device_print_server = CreatePrintServer(
+      "2", "http://192.168.1.5/device-printer", "Color Laser");
+  device_print_servers.push_back(device_print_server);
+  device_policy_print_servers_provider_.SetPrintServers(device_print_servers);
+
+  EXPECT_THAT(
+      server_printers_provider_->GetPrintServers(),
+      testing::UnorderedElementsAre(user_print_server, device_print_server));
+}
+
+TEST_F(CupsPrintersManagerTest, GetServerPrinters_SingleServerOnly) {
+  EXPECT_TRUE(server_printers_provider_->GetPrinters().empty());
+
+  auto selected_print_server =
+      CreatePrintServer("user-1", "http://user-print-1", "User LexaPrint - 1");
+
+  std::vector<chromeos::PrintServer> user_print_servers;
+  for (int i = 1; i <= 10; ++i) {
+    auto id = std::to_string(i);
+    auto print_server = CreatePrintServer(
+        "user-" + id, "http://user-print-" + id, "User LexaPrint - " + id);
+    user_print_servers.push_back(print_server);
+  }
+  user_policy_print_servers_provider_.SetPrintServers(user_print_servers);
+  std::vector<chromeos::PrintServer> device_print_servers;
+  for (int i = 1; i <= 7; ++i) {
+    auto id = std::to_string(i);
+    auto print_server =
+        CreatePrintServer("device-" + id, "http://device-print-" + id,
+                          "Device LexaPrint - " + id);
+    device_print_servers.push_back(print_server);
+  }
+  device_policy_print_servers_provider_.SetPrintServers(device_print_servers);
+
+  manager_->ChoosePrintServer(selected_print_server.GetId());
+
+  EXPECT_THAT(server_printers_provider_->GetPrintServers(),
+              testing::UnorderedElementsAre(selected_print_server));
+}
+
 }  // namespace
 }  // namespace chromeos

chrome/browser/chromeos/printing/print_servers_policy_provider.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/printing/print_servers_policy_provider.h"
+
+#include "base/bind.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+
+namespace chromeos {
+
+namespace {
+
+constexpr int kMaxRecords = 16;
+
+}  // namespace
+
+PrintServersPolicyProvider::PrintServersPolicyProvider(
+    base::WeakPtr<PrintServersProvider> user_policy_provider,
+    base::WeakPtr<PrintServersProvider> device_policy_provider)
+    : user_policy_provider_(user_policy_provider),
+      device_policy_provider_(device_policy_provider) {
+  user_policy_provider_->AddObserver(this);
+  device_policy_provider_->AddObserver(this);
+}
+
+PrintServersPolicyProvider::~PrintServersPolicyProvider() = default;
+
+// static
+std::unique_ptr<PrintServersPolicyProvider> PrintServersPolicyProvider::Create(
+    Profile* profile) {
+  base::WeakPtr<PrintServersProvider> user_policy_provider =
+      PrintServersProviderFactory::Get()->GetForProfile(profile);
+  user_policy_provider->SetAllowlistPref(profile->GetPrefs(),
+                                         prefs::kExternalPrintServersAllowlist);
+  base::WeakPtr<PrintServersProvider> device_policy_provider =
+      PrintServersProviderFactory::Get()->GetForDevice();
+  device_policy_provider->SetAllowlistPref(
+      g_browser_process->local_state(),
+      prefs::kDeviceExternalPrintServersAllowlist);
+  return std::make_unique<PrintServersPolicyProvider>(user_policy_provider,
+                                                      device_policy_provider);
+}
+
+// static
+std::unique_ptr<PrintServersPolicyProvider>
+PrintServersPolicyProvider::CreateForTesting(
+    base::WeakPtr<PrintServersProvider> user_policy_provider,
+    base::WeakPtr<PrintServersProvider> device_policy_provider) {
+  return std::make_unique<PrintServersPolicyProvider>(user_policy_provider,
+                                                      device_policy_provider);
+}
+
+void PrintServersPolicyProvider::SetListener(
+    const OnPrintServersChanged& callback) {
+  callback_ = std::make_unique<OnPrintServersChanged>(callback);
+}
+
+void PrintServersPolicyProvider::OnServersChanged(
+    bool unused_complete,
+    const std::vector<PrintServer>& unused_servers) {
+  if (callback_) {
+    std::map<GURL, PrintServer> all_servers;
+    auto device_servers = device_policy_provider_->GetPrintServers();
+    if (device_servers.has_value()) {
+      for (const auto& server : device_servers.value()) {
+        all_servers.emplace(server.GetUrl(), server);
+      }
+    }
+    auto user_servers = user_policy_provider_->GetPrintServers();
+    if (user_servers.has_value()) {
+      for (const auto& server : user_servers.value()) {
+        all_servers.emplace(server.GetUrl(), server);
+      }
+    }
+    bool is_complete = user_servers.has_value() || device_servers.has_value();
+    ServerPrintersFetchingMode fetching_mode = GetFetchingMode(all_servers);
+    callback_->Run(is_complete, all_servers, fetching_mode);
+  }
+}
+
+ServerPrintersFetchingMode PrintServersPolicyProvider::GetFetchingMode(
+    const std::map<GURL, PrintServer>& all_servers) {
+  return all_servers.size() <= kMaxRecords
+             ? ServerPrintersFetchingMode::kStandard
+             : ServerPrintersFetchingMode::kSingleServerOnly;
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/printing/print_servers_policy_provider.h

+// 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.
+#ifndef CHROME_BROWSER_CHROMEOS_PRINTING_PRINT_SERVERS_POLICY_PROVIDER_H_
+#define CHROME_BROWSER_CHROMEOS_PRINTING_PRINT_SERVERS_POLICY_PROVIDER_H_
+
+#include <map>
+#include <memory>
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/printing/print_server.h"
+#include "chrome/browser/chromeos/printing/print_servers_provider.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+class Profile;
+
+namespace chromeos {
+
+enum ServerPrintersFetchingMode {
+  kStandard,
+  kSingleServerOnly,
+};
+
+// This class observes values provided by the DeviceExternalPrintServers and
+// ExternalPrintServers policies and calculates resultant list of available
+// print servers. This list is propagated to the provided callback.
+class PrintServersPolicyProvider : public KeyedService,
+                                   public PrintServersProvider::Observer {
+ public:
+  PrintServersPolicyProvider(
+      base::WeakPtr<PrintServersProvider> user_policy_provider,
+      base::WeakPtr<PrintServersProvider> device_policy_provider);
+
+  ~PrintServersPolicyProvider() override;
+
+  using OnPrintServersChanged = typename base::RepeatingCallback<
+      void(bool, std::map<GURL, PrintServer>, ServerPrintersFetchingMode)>;
+
+  static std::unique_ptr<PrintServersPolicyProvider> Create(Profile* profile);
+
+  static std::unique_ptr<PrintServersPolicyProvider> CreateForTesting(
+      base::WeakPtr<PrintServersProvider> user_policy_provider,
+      base::WeakPtr<PrintServersProvider> device_policy_provider);
+
+  // Set the callback when print servers has been updated via policy.
+  void SetListener(const OnPrintServersChanged& callback);
+
+  // PrintServersProvider::Observer
+  void OnServersChanged(
+      bool unused_complete,
+      const std::vector<PrintServer>& unused_servers) override;
+
+ private:
+  ServerPrintersFetchingMode GetFetchingMode(
+      const std::map<GURL, PrintServer>& all_servers);
+
+  base::WeakPtr<PrintServersProvider> user_policy_provider_;
+  base::WeakPtr<PrintServersProvider> device_policy_provider_;
+
+  std::map<GURL, PrintServer> all_servers_;
+
+  std::unique_ptr<OnPrintServersChanged> callback_;
+
+  base::WeakPtrFactory<PrintServersPolicyProvider> weak_ptr_factory_{this};
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_PRINTING_PRINT_SERVERS_POLICY_PROVIDER_H_

chrome/browser/chromeos/printing/print_servers_provider.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -30,8 +31,6 @@ namespace chromeos {
 
 namespace {
 
-constexpr int kMaxRecords = 16;
-
 struct TaskResults {
   int task_id;
   std::vector<PrintServer> servers;
@@ -155,6 +154,7 @@ class PrintServersProviderImpl : public PrintServersProvider {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   }
 
+  // This method sets the allowlist to calculate resultant list of servers.
   void SetAllowlistPref(PrefService* prefs,
                         const std::string& allowlist_pref) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -173,6 +173,17 @@ class PrintServersProviderImpl : public PrintServersProvider {
     UpdateAllowlist();
   }
 
+  void NotifyObservers(bool servers_are_complete,
+                       const std::vector<PrintServer>& servers) {
+    for (auto& observer : observers_) {
+      observer.OnServersChanged(servers_are_complete, servers);
+    }
+  }
+
+  base::Optional<std::vector<PrintServer>> GetPrintServers() override {
+    return IsCompleted() ? base::make_optional(result_servers_) : base::nullopt;
+  }
+
   void AddObserver(PrintServersProvider::Observer* observer) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     observers_.AddObserver(observer);
@@ -193,8 +204,7 @@ class PrintServersProviderImpl : public PrintServersProvider {
     result_servers_.clear();
     if (!(previously_completed && previously_empty)) {
       // Notify observers.
-      for (auto& observer : observers_)
-        observer.OnServersChanged(true, result_servers_);
+      NotifyObservers(true, result_servers_);
     }
   }
 
@@ -208,8 +218,7 @@ class PrintServersProviderImpl : public PrintServersProvider {
                        weak_ptr_factory_.GetWeakPtr()));
     if (previously_completed) {
       // Notify observers.
-      for (auto& observer : observers_)
-        observer.OnServersChanged(false, result_servers_);
+      NotifyObservers(false, result_servers_);
     }
   }
 
@@ -247,8 +256,7 @@ class PrintServersProviderImpl : public PrintServersProvider {
     const bool has_changes = CalculateResultantList();
     if (has_changes) {
       const bool is_completed = IsCompleted();
-      for (auto& observer : observers_)
-        observer.OnServersChanged(is_completed, result_servers_);
+      NotifyObservers(is_completed, result_servers_);
     }
   }
 
@@ -265,13 +273,6 @@ class PrintServersProviderImpl : public PrintServersProvider {
     } else {
       for (auto& print_server : servers_) {
         if (allowlist_.value().count(print_server.GetId())) {
-          if (new_servers.size() == kMaxRecords) {
-            LOG(WARNING) << "The list of resultant print servers read from "
-                         << "policies is too long. Only the first "
-                         << kMaxRecords << " print servers will be taken into "
-                         << "account";
-            break;
-          }
           new_servers.push_back(print_server);
         }
       }
@@ -302,8 +303,7 @@ class PrintServersProviderImpl : public PrintServersProvider {
     const bool has_changes = CalculateResultantList();
     // Notify observers if something changed.
     if (is_complete || has_changes) {
-      for (auto& observer : observers_)
-        observer.OnServersChanged(is_complete, result_servers_);
+      NotifyObservers(is_complete, result_servers_);
     }
   }
 
@@ -325,6 +325,8 @@ class PrintServersProviderImpl : public PrintServersProvider {
   PrefChangeRegistrar pref_change_registrar_;
   std::string allowlist_pref_;
 
+  std::unique_ptr<base::RepeatingCallback<void()>> policy_callback_;
+
   base::ObserverList<PrintServersProvider::Observer>::Unchecked observers_;
   base::WeakPtrFactory<PrintServersProviderImpl> weak_ptr_factory_{this};
 

chrome/browser/chromeos/printing/print_servers_provider.h

@@ -9,6 +9,7 @@
 #include <set>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/printing/print_server.h"
@@ -62,6 +63,10 @@ class PrintServersProvider
   // Clears the content of the policy.
   virtual void ClearData() = 0;
 
+  // Returns the list of all print servers given from the data provided in
+  // SetData(...) and limited by the allowlist.
+  virtual base::Optional<std::vector<PrintServer>> GetPrintServers() = 0;
+
  protected:
   PrintServersProvider() = default;
 

chrome/browser/chromeos/printing/printing_stubs.cc

@@ -20,4 +20,14 @@ base::Optional<Printer> StubCupsPrintersManager::GetPrinter(
   return {};
 }
 
+bool StubCupsPrintersManager::ChoosePrintServer(
+    const base::Optional<std::string>& selected_print_server_id) {
+  return true;
+}
+
+ServerPrintersFetchingMode StubCupsPrintersManager::GetServerPrintersFetchingMode()
+    const {
+  return ServerPrintersFetchingMode::kStandard;
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/printing/printing_stubs.h

@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/optional.h"
 #include "chrome/browser/chromeos/printing/cups_printers_manager.h"
 #include "chrome/browser/chromeos/printing/printer_configurer.h"
 #include "chromeos/printing/ppd_provider.h"
@@ -32,6 +33,9 @@ class StubCupsPrintersManager : public CupsPrintersManager {
   void FetchPrinterStatus(const std::string& printer_id,
                           PrinterStatusCallback cb) override {}
   void RecordNearbyNetworkPrinterCounts() const override {}
+  bool ChoosePrintServer(
+      const base::Optional<std::string>& selected_print_server_id) override;
+  ServerPrintersFetchingMode GetServerPrintersFetchingMode() const override;
 };
 
 class StubPrinterConfigurer : public PrinterConfigurer {

chrome/browser/chromeos/printing/server_printers_provider.cc

@@ -16,8 +16,6 @@
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/task/post_task.h"
-#include "chrome/browser/browser_process.h"
-#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 "chrome/browser/chromeos/printing/server_printers_fetcher.h"
@@ -40,68 +38,12 @@ struct PrintServerWithPrinters {
   std::vector<PrinterDetector::DetectedPrinter> printers;  // queried printers
 };
 
-class PrintServersPolicyProvider : public PrintServersProvider::Observer {
- public:
-  PrintServersPolicyProvider(base::WeakPtr<PrintServersProvider> provider,
-                             PrefService* prefs,
-                             const std::string& pref_name)
-      : provider_(provider) {
-    provider_->SetAllowlistPref(prefs, pref_name);
-    provider->AddObserver(this);
-  }
-
-  ~PrintServersPolicyProvider() override {
-    if (provider_) {
-      provider_->RemoveObserver(this);
-    }
-  }
-
-  base::Optional<std::vector<PrintServer>>& GetPrinterServers() {
-    return servers_;
-  }
-
-  void SetListener(const base::RepeatingCallback<void()>& callback) {
-    callback_ = std::make_unique<base::RepeatingCallback<void()>>(callback);
-    callback_->Run();
-  }
-
-  // PrintServersProvider::Observer implementation.
-  void OnServersChanged(bool servers_are_complete,
-                        const std::vector<PrintServer>& servers) override {
-    servers_ =
-        servers_are_complete ? base::make_optional(servers) : base::nullopt;
-    if (callback_) {
-      callback_->Run();
-    }
-  }
-
- private:
-  base::WeakPtr<PrintServersProvider> provider_;
-  base::Optional<std::vector<PrintServer>> servers_;
-  std::unique_ptr<base::RepeatingCallback<void()>> callback_;
-};
-
 class ServerPrintersProviderImpl
     : public ServerPrintersProvider,
       public base::SupportsWeakPtr<ServerPrintersProviderImpl> {
  public:
-  explicit ServerPrintersProviderImpl(Profile* profile)
-      : user_policy_provider_(std::make_unique<PrintServersPolicyProvider>(
-            PrintServersProviderFactory::Get()->GetForProfile(profile),
-            profile->GetPrefs(),
-            prefs::kExternalPrintServersAllowlist)),
-        device_policy_provider_(std::make_unique<PrintServersPolicyProvider>(
-            PrintServersProviderFactory::Get()->GetForDevice(),
-            g_browser_process->local_state(),
-            prefs::kDeviceExternalPrintServersAllowlist)) {
+  ServerPrintersProviderImpl() {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-    user_policy_provider_->SetListener(
-        base::BindRepeating(&ServerPrintersProviderImpl::NotifyPolicyChanged,
-                            weak_ptr_factory_.GetWeakPtr()));
-    device_policy_provider_->SetListener(
-        base::BindRepeating(&ServerPrintersProviderImpl::NotifyPolicyChanged,
-                            weak_ptr_factory_.GetWeakPtr()));
   }
 
   ~ServerPrintersProviderImpl() override = default;
@@ -122,7 +64,7 @@ class ServerPrintersProviderImpl
   }
 
   void OnServersChanged(bool servers_are_complete,
-                        const std::map<GURL, PrintServer>& servers) {
+                        const std::map<GURL, PrintServer>& servers) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     // Create an entry in the device log.
     if (servers_are_complete) {
@@ -210,25 +152,6 @@ class ServerPrintersProviderImpl
   }
 
  private:
-  void NotifyPolicyChanged() {
-    std::map<GURL, PrintServer> all_servers;
-    auto& device_servers = device_policy_provider_->GetPrinterServers();
-    if (device_servers.has_value()) {
-      for (const auto& server : device_servers.value()) {
-        all_servers.emplace(server.GetUrl(), server);
-      }
-    }
-    auto& user_servers = user_policy_provider_->GetPrinterServers();
-    if (user_servers.has_value()) {
-      for (const auto& server : user_servers.value()) {
-        all_servers.emplace(server.GetUrl(), server);
-      }
-    }
-
-    bool is_complete = user_servers.has_value() || device_servers.has_value();
-    OnServersChanged(is_complete, all_servers);
-  }
-
   // Returns true <=> all policies have been parsed and applied and all servers
   // have been queried (even when some errors occurred).
   bool IsComplete() const {
@@ -247,9 +170,6 @@ class ServerPrintersProviderImpl
   // URLs that are being queried now with corresponding fetcher objects.
   std::map<GURL, std::unique_ptr<ServerPrintersFetcher>> fetchers_;
 
-  std::unique_ptr<PrintServersPolicyProvider> user_policy_provider_;
-  std::unique_ptr<PrintServersPolicyProvider> device_policy_provider_;
-
   SEQUENCE_CHECKER(sequence_checker_);
   base::WeakPtrFactory<ServerPrintersProviderImpl> weak_ptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(ServerPrintersProviderImpl);
@@ -258,9 +178,8 @@ class ServerPrintersProviderImpl
 }  // namespace
 
 // static
-std::unique_ptr<ServerPrintersProvider> ServerPrintersProvider::Create(
-    Profile* profile) {
-  return std::make_unique<ServerPrintersProviderImpl>(profile);
+std::unique_ptr<ServerPrintersProvider> ServerPrintersProvider::Create() {
+  return std::make_unique<ServerPrintersProviderImpl>();
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/printing/server_printers_provider.h

@@ -5,28 +5,27 @@
 #ifndef CHROME_BROWSER_CHROMEOS_PRINTING_SERVER_PRINTERS_PROVIDER_H_
 #define CHROME_BROWSER_CHROMEOS_PRINTING_SERVER_PRINTERS_PROVIDER_H_
 
+#include <map>
 #include <memory>
 #include <vector>
 
 #include "base/macros.h"
+#include "chrome/browser/chromeos/printing/print_server.h"
 #include "chrome/browser/chromeos/printing/printer_detector.h"
 #include "components/keyed_service/core/keyed_service.h"
 
-class Profile;
-
 namespace chromeos {
 
-// Uses classes PrintServersProvider and ServerPrintersFetcher to track list of
-// external print servers and printers exposed by them. These printers are
-// called server printers. All changes in the final list of available server
-// printers are signaled through a callback registered with the method
+// Given a list of external print servers, uses ServerPrintersFetcher to track
+// list of printers exposed by them. These printers are called server printers.
+// All changes in the final list of available server printers are signaled
+// through a callback registered with the method
 // RegisterPrintersFoundCallback(...). All methods must be called from the UI
 // sequence, the callback is also called from this sequence.
-class ServerPrintersProvider : public KeyedService {
+class ServerPrintersProvider {
  public:
-  // |profile| is a user profile, it cannot be nullptr.
-  static std::unique_ptr<ServerPrintersProvider> Create(Profile* profile);
-  ~ServerPrintersProvider() override = default;
+  static std::unique_ptr<ServerPrintersProvider> Create();
+  virtual ~ServerPrintersProvider() = default;
 
   using OnPrintersUpdateCallback = base::RepeatingCallback<void(bool complete)>;
 
@@ -35,6 +34,12 @@ class ServerPrintersProvider : public KeyedService {
   // unregister the current one.
   virtual void RegisterPrintersFoundCallback(OnPrintersUpdateCallback cb) = 0;
 
+  // |servers_are_complete| is true if all policies have been parsed and
+  // applied. |servers| contains the current list of print servers to query for
+  // printers.
+  virtual void OnServersChanged(bool servers_are_complete,
+                                const std::map<GURL, PrintServer>& servers) = 0;
+
   virtual std::vector<PrinterDetector::DetectedPrinter> GetPrinters() = 0;
 
  protected:

chrome/browser/chromeos/printing/server_printers_provider_factory.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/printing/server_printers_provider_factory.h"
-
-#include "base/no_destructor.h"
-#include "chrome/browser/chromeos/printing/server_printers_provider.h"
-#include "chrome/browser/profiles/incognito_helpers.h"
-#include "chrome/browser/profiles/profile.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
-
-namespace chromeos {
-
-// static
-ServerPrintersProviderFactory* ServerPrintersProviderFactory::GetInstance() {
-  static base::NoDestructor<ServerPrintersProviderFactory> instance;
-  return instance.get();
-}
-
-// static
-ServerPrintersProvider* ServerPrintersProviderFactory::GetForBrowserContext(
-    content::BrowserContext* context) {
-  return static_cast<ServerPrintersProvider*>(
-      GetInstance()->GetServiceForBrowserContext(context, true));
-}
-
-ServerPrintersProviderFactory::ServerPrintersProviderFactory()
-    : BrowserContextKeyedServiceFactory(
-          "ServerPrintersProviderFactory",
-          BrowserContextDependencyManager::GetInstance()) {}
-
-ServerPrintersProviderFactory::~ServerPrintersProviderFactory() = default;
-
-KeyedService* ServerPrintersProviderFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  auto* profile = Profile::FromBrowserContext(context);
-  return ServerPrintersProvider::Create(profile).release();
-}
-
-content::BrowserContext* ServerPrintersProviderFactory::GetBrowserContextToUse(
-    content::BrowserContext* context) const {
-  return chrome::GetBrowserContextRedirectedInIncognito(context);
-}
-
-bool ServerPrintersProviderFactory::ServiceIsNULLWhileTesting() const {
-  return true;
-}
-
-}  // namespace chromeos

chrome/browser/chromeos/printing/server_printers_provider_factory.h

-// 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_PRINTING_SERVER_PRINTERS_PROVIDER_FACTORY_H_
-#define CHROME_BROWSER_CHROMEOS_PRINTING_SERVER_PRINTERS_PROVIDER_FACTORY_H_
-
-#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
-
-namespace content {
-class BrowserContext;
-}
-
-namespace base {
-template <typename T>
-class NoDestructor;
-}
-
-namespace chromeos {
-
-class ServerPrintersProvider;
-
-class ServerPrintersProviderFactory : public BrowserContextKeyedServiceFactory {
- public:
-  static ServerPrintersProviderFactory* GetInstance();
-  static ServerPrintersProvider* GetForBrowserContext(
-      content::BrowserContext* context);
-
- private:
-  friend class base::NoDestructor<ServerPrintersProviderFactory>;
-
-  ServerPrintersProviderFactory();
-  ~ServerPrintersProviderFactory() override;
-
-  // BrowserContextKeyedServiceFactory overrides:
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-  content::BrowserContext* GetBrowserContextToUse(
-      content::BrowserContext* context) const override;
-  bool ServiceIsNULLWhileTesting() const override;
-
-  DISALLOW_COPY_AND_ASSIGN(ServerPrintersProviderFactory);
-};
-
-}  // namespace chromeos
-
-#endif  // CHROME_BROWSER_CHROMEOS_PRINTING_SERVER_PRINTERS_PROVIDER_FACTORY_H_