Update CupsPrintersManager with CupsProxy printer installs

Now CupsProxyService correctly updates CupsPrintersManager when it
successfully installs a printer into CUPS. Since any print job involves
many IPP requests, each of which could trigger a printer install, this
caching debug daemon calls.

Bug: chromium:945409
Test: updated unit_tests pass
Change-Id: I4d0cba3e5cbe35c5acd2a78fc5a94ff73da5cf1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863729Reviewed-by: Sean Kau <skau@chromium.org>
Commit-Queue: Luum Habtemariam <luum@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706231}

chrome/browser/chromeos/printing/cups_proxy_service_delegate_impl.cc

@@ -45,6 +45,14 @@ bool CupsProxyServiceDelegateImpl::IsPrinterInstalled(const Printer& printer) {
   return printers_manager_->IsPrinterInstalled(printer);
 }
 
+// Expects |printer| is known by the printers_manager_.
+void CupsProxyServiceDelegateImpl::PrinterInstalled(const Printer& printer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(GetPrinter(printer.id()));
+  printers_manager_->PrinterInstalled(
+      printer, false /* unused */, PrinterSetupSource::kMaxValue /* unused */);
+}
+
 scoped_refptr<base::SingleThreadTaskRunner>
 CupsProxyServiceDelegateImpl::GetIOTaskRunner() {
   return base::CreateSingleThreadTaskRunner({content::BrowserThread::IO});

chrome/browser/chromeos/printing/cups_proxy_service_delegate_impl.h

@@ -43,6 +43,9 @@ class CupsProxyServiceDelegateImpl
   // Returns whether |printer| is currently installed in CUPS with this config.
   bool IsPrinterInstalled(const Printer& printer) override;
 
+  // Records that |printer| has been installed into CUPS with this config.
+  void PrinterInstalled(const Printer& printer) override;
+
   // Returns an IO-thread task runner.
   scoped_refptr<base::SingleThreadTaskRunner> GetIOTaskRunner() override;
 

chrome/services/cups_proxy/cups_proxy_service_delegate.h

@@ -36,6 +36,7 @@ class CupsProxyServiceDelegate {
   virtual base::Optional<chromeos::Printer> GetPrinter(
       const std::string& id) = 0;
   virtual bool IsPrinterInstalled(const chromeos::Printer& printer) = 0;
+  virtual void PrinterInstalled(const chromeos::Printer& printer) = 0;
   virtual scoped_refptr<base::SingleThreadTaskRunner> GetIOTaskRunner() = 0;
 
   // |cb| will be run on this delegate's sequenced context.

chrome/services/cups_proxy/fake_cups_proxy_service_delegate.cc

@@ -21,6 +21,9 @@ bool FakeCupsProxyServiceDelegate::IsPrinterInstalled(
   return false;
 }
 
+void FakeCupsProxyServiceDelegate::PrinterInstalled(
+    const chromeos::Printer& printer) {}
+
 scoped_refptr<base::SingleThreadTaskRunner>
 FakeCupsProxyServiceDelegate::GetIOTaskRunner() {
   return nullptr;

chrome/services/cups_proxy/fake_cups_proxy_service_delegate.h

@@ -25,6 +25,7 @@ class FakeCupsProxyServiceDelegate : public CupsProxyServiceDelegate {
       chromeos::PrinterClass printer_class) override;
   base::Optional<chromeos::Printer> GetPrinter(const std::string& id) override;
   bool IsPrinterInstalled(const chromeos::Printer& printer) override;
+  void PrinterInstalled(const chromeos::Printer& printer) override;
   scoped_refptr<base::SingleThreadTaskRunner> GetIOTaskRunner() override;
   void SetupPrinter(const chromeos::Printer& printer,
                     SetupPrinterCallback cb) override;

chrome/services/cups_proxy/printer_installer.cc

@@ -44,16 +44,22 @@ void PrinterInstaller::InstallPrinter(std::string printer_id,
 
   // Install printer.
   delegate_->SetupPrinter(
-      *printer, base::BindOnce(&PrinterInstaller::OnInstallPrinter,
-                               weak_factory_.GetWeakPtr(), std::move(cb)));
+      *printer,
+      base::BindOnce(&PrinterInstaller::OnInstallPrinter,
+                     weak_factory_.GetWeakPtr(), std::move(cb), *printer));
 }
 
 // TODO(crbug.com/945409): Test whether we need to call
 // CupsPrintersManager::PrinterInstalled here.
 void PrinterInstaller::OnInstallPrinter(InstallPrinterCallback cb,
+                                        const chromeos::Printer& printer,
                                         bool success) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  if (success) {
+    delegate_->PrinterInstalled(printer);
+  }
+
   Finish(std::move(cb),
          success ? InstallPrinterResult::kSuccess
                  : InstallPrinterResult::kPrinterInstallationFailure);

chrome/services/cups_proxy/printer_installer.h

@@ -39,7 +39,9 @@ class PrinterInstaller {
   void InstallPrinter(std::string printer_id, InstallPrinterCallback cb);
 
  private:
-  void OnInstallPrinter(InstallPrinterCallback cb, bool success);
+  void OnInstallPrinter(InstallPrinterCallback cb,
+                        const chromeos::Printer& printer,
+                        bool success);
   void Finish(InstallPrinterCallback cb, InstallPrinterResult res);
 
   // Unowned delegate granting access to printing stack dependencies.

chrome/services/cups_proxy/printer_installer_unittest.cc

@@ -47,6 +47,11 @@ class FakeServiceDelegate : public FakeCupsProxyServiceDelegate {
     return installed_printers_.at(printer.id());
   }
 
+  void PrinterInstalled(const Printer& printer) override {
+    DCHECK(base::Contains(installed_printers_, printer.id()));
+    installed_printers_[printer.id()] = true;
+  }
+
   base::Optional<Printer> GetPrinter(const std::string& id) override {
     if (!base::Contains(installed_printers_, id)) {
       return base::nullopt;
@@ -68,7 +73,6 @@ class FakeServiceDelegate : public FakeCupsProxyServiceDelegate {
     }
 
     // Install printer.
-    installed_printers_[printer.id()] = true;
     return std::move(callback).Run(true);
   }
 
@@ -162,6 +166,7 @@ TEST_F(PrinterInstallerTest, SetupPrinterFailure) {
 
   auto ret = RunInstallPrinter(kGenericGUID);
   EXPECT_EQ(ret, InstallPrinterResult::kPrinterInstallationFailure);
+  EXPECT_FALSE(delegate_->IsPrinterInstalled(to_install));
 }
 
 }  // namespace