CupsProxyService startup flow

DDoc: go/cups-plugin

The service should lazily start once it knows the daemon has started;
this CL adds a WaitForServiceToBeAvailable method to the CupsProxyClient
for that purpose.

This CL also adds the CupsProxyServiceManager, which is started with the
browser, waits for daemon startup, then starts the service by
connecting to it.
Note: the service maintains its own lifetime and connection with the
daemon; this manager simply starts it.

Bug: chromium:945409
Test: TBD
Change-Id: Idf08a1153493c8e7abfcd5ad291203ea12a692b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1586540Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Luum Habtemariam <luum@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664577}

chrome/app/BUILD.gn

@@ -460,6 +460,7 @@ source_set("chrome_content_browser_overlay_manifest") {
       "//chrome/browser/chromeos/supervision/mojom",
       "//chrome/browser/ui/webui/chromeos/add_supervision:mojo_bindings",
       "//chrome/browser/ui/webui/chromeos/machine_learning:mojo_bindings",
+      "//chrome/services/cups_proxy/public/mojom",
       "//chromeos/assistant:buildflags",
       "//chromeos/services/cellular_setup/public/mojom",
       "//chromeos/services/device_sync/public/cpp:manifest",

chrome/app/DEPS

@@ -45,6 +45,7 @@ include_rules = [
 specific_include_rules = {
   "chrome_content_browser_overlay_manifest\.cc": [
     "+chrome/services/app_service",
+    "+chrome/services/cups_proxy/public",
     "+chromeos/assistant",
     "+chromeos/services/assistant",
     "+chromeos/services/cellular_setup",

chrome/app/chrome_content_browser_overlay_manifest.cc

@@ -53,6 +53,7 @@
 #include "chrome/browser/chromeos/supervision/mojom/onboarding_controller.mojom.h"
 #include "chrome/browser/ui/webui/chromeos/add_supervision/add_supervision.mojom.h"
 #include "chrome/browser/ui/webui/chromeos/machine_learning/machine_learning_internals_page_handler.mojom.h"
+#include "chrome/services/cups_proxy/public/mojom/constants.mojom.h"
 #include "chromeos/assistant/buildflags.h"  // nogncheck
 #include "chromeos/services/cellular_setup/public/mojom/cellular_setup.mojom.h"
 #include "chromeos/services/device_sync/public/cpp/manifest.h"
@@ -177,6 +178,9 @@ const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
         .RequireCapability(
             chromeos::network_config::mojom::kServiceName,
             chromeos::network_config::mojom::kNetworkConfigCapability)
+        .RequireCapability(
+            chromeos::printing::mojom::kCupsProxyServiceName,
+            chromeos::printing::mojom::kStartCupsProxyServiceCapability)
         .ExposeInterfaceFilterCapability_Deprecated(
             "navigation:frame", "cellular_setup",
             service_manager::Manifest::InterfaceList<

chrome/browser/chromeos/BUILD.gn

@@ -75,6 +75,7 @@ source_set("chromeos") {
     "//chrome/services/app_service:lib",
     "//chrome/services/app_service/public/cpp:app_service_proxy",
     "//chrome/services/app_service/public/cpp:app_update",
+    "//chrome/services/cups_proxy/public/mojom",
     "//chrome/services/file_util/public/cpp",
     "//chrome/services/wilco_dtc_supportd/public/mojom",
     "//chromeos",
@@ -1827,6 +1828,8 @@ source_set("chromeos") {
     "printing/cups_printers_manager.h",
     "printing/cups_printers_manager_factory.cc",
     "printing/cups_printers_manager_factory.h",
+    "printing/cups_proxy_service_manager.cc",
+    "printing/cups_proxy_service_manager.h",
     "printing/enterprise_printers_provider.cc",
     "printing/enterprise_printers_provider.h",
     "printing/ppd_provider_factory.cc",

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -97,6 +97,7 @@
 #include "chrome/browser/chromeos/power/power_metrics_reporter.h"
 #include "chrome/browser/chromeos/power/process_data_collector.h"
 #include "chrome/browser/chromeos/power/renderer_freezer.h"
+#include "chrome/browser/chromeos/printing/cups_proxy_service_manager.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/resource_reporter/resource_reporter.h"
 #include "chrome/browser/chromeos/scheduler_configuration_manager.h"
@@ -688,6 +689,8 @@ void ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() {
   lock_to_single_user_manager_ =
       std::make_unique<policy::LockToSingleUserManager>();
 
+  cups_proxy_service_manager_ = std::make_unique<CupsProxyServiceManager>();
+
   ChromeBrowserMainPartsLinux::PreMainMessageLoopRun();
 }
 

chrome/browser/chromeos/chrome_browser_main_chromeos.h

@@ -39,6 +39,7 @@ namespace chromeos {
 
 class ArcKioskAppManager;
 class CrosUsbDetector;
+class CupsProxyServiceManager;
 class DemoModeResourcesRemover;
 class DiscoverManager;
 class EventRewriterDelegateImpl;
@@ -177,6 +178,8 @@ class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux {
   std::unique_ptr<KerberosCredentialsManager> kerberos_credentials_manager_;
 
   std::unique_ptr<GnubbyNotification> gnubby_notification_;
+  std::unique_ptr<chromeos::CupsProxyServiceManager>
+      cups_proxy_service_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsChromeos);
 };

chrome/browser/chromeos/printing/cups_proxy_service_manager.cc

+// Copyright 2019 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/cups_proxy_service_manager.h"
+
+#include "chrome/services/cups_proxy/public/mojom/constants.mojom.h"
+#include "chromeos/dbus/cups_proxy/cups_proxy_client.h"
+#include "content/public/common/service_manager_connection.h"
+#include "services/service_manager/public/cpp/connector.h"
+
+namespace chromeos {
+
+CupsProxyServiceManager::CupsProxyServiceManager() : weak_factory_(this) {
+  CupsProxyClient::Get()->WaitForServiceToBeAvailable(base::BindOnce(
+      &CupsProxyServiceManager::OnDaemonAvailable, weak_factory_.GetWeakPtr()));
+}
+
+CupsProxyServiceManager::~CupsProxyServiceManager() = default;
+
+void CupsProxyServiceManager::OnDaemonAvailable(bool daemon_available) {
+  if (!daemon_available) {
+    DVLOG(1) << "CupsProxyDaemon startup error";
+    return;
+  }
+
+  // Attempt to start the service, which will then bootstrap a connection
+  // with the daemon.
+  // Note: The service does not support BindInterface calls, so we
+  // intentionally leave out a connection_error_handler, since it would
+  // called immediately.
+  content::ServiceManagerConnection::GetForProcess()->GetConnector()->Connect(
+      printing::mojom::kCupsProxyServiceName,
+      service_handle_.BindNewPipeAndPassReceiver());
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/printing/cups_proxy_service_manager.h

+// Copyright 2019 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_CUPS_PROXY_SERVICE_MANAGER_H_
+#define CHROME_BROWSER_CHROMEOS_PRINTING_CUPS_PROXY_SERVICE_MANAGER_H_
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/services/cups_proxy/public/mojom/proxy.mojom.h"
+#include "mojo/public/cpp/bindings/remote.h"
+
+namespace chromeos {
+
+// Responsible for lazily starting the CupsProxyService once notified that the
+// CupsProxyDaemon has started.
+//
+// Note: This manager is not currently fault-tolerant, i.e. should the
+// service/daemon fail, we do not try to restart.
+class CupsProxyServiceManager {
+ public:
+  CupsProxyServiceManager();
+  ~CupsProxyServiceManager();
+
+ private:
+  void OnDaemonAvailable(bool daemon_available);
+
+  mojo::Remote<printing::mojom::StartCupsProxyService> service_handle_;
+  base::WeakPtrFactory<CupsProxyServiceManager> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CupsProxyServiceManager);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_PRINTING_CUPS_PROXY_SERVICE_MANAGER_H_

chrome/services/cups_proxy/public/cpp/manifest.cc

@@ -6,6 +6,7 @@
 
 #include "base/no_destructor.h"
 #include "chrome/services/cups_proxy/public/mojom/constants.mojom.h"
+#include "chrome/services/cups_proxy/public/mojom/proxy.mojom.h"
 #include "services/service_manager/public/cpp/manifest_builder.h"
 
 namespace chromeos {
@@ -22,8 +23,9 @@ const service_manager::Manifest& GetCupsProxyManifest() {
                                service_manager::Manifest::
                                    InstanceSharingPolicy::kSingleton)
                            .Build())
-          .ExposeCapability(mojom::kCupsProxierCapability,
-                            service_manager::Manifest::InterfaceList<>())
+          .ExposeCapability(mojom::kStartCupsProxyServiceCapability,
+                            service_manager::Manifest::InterfaceList<
+                                mojom::StartCupsProxyService>())
           .Build()};
   return *manifest;
 }

chrome/services/cups_proxy/public/mojom/BUILD.gn

@@ -8,6 +8,7 @@ import("//mojo/public/tools/bindings/mojom.gni")
 mojom("mojom") {
   sources = [
     "constants.mojom",
+    "proxy.mojom",
   ]
 
   public_deps = [

chrome/services/cups_proxy/public/mojom/constants.mojom

@@ -7,5 +7,5 @@ module chromeos.printing.mojom;
 // Service id.
 const string kCupsProxyServiceName = "cups_proxy";
 
-// IppProxier capability id.
-const string kCupsProxierCapability = "cups_proxier";
+// StartCupsProxyService capability id.
+const string kStartCupsProxyServiceCapability = "start_cups_proxy_service";

chrome/services/cups_proxy/public/mojom/proxy.mojom

+// Copyright 2019 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.
+
+// NOTE: This mojom should be kept in sync with the copy in Chromium OS's repo
+// in TODO(crbug.com/945409): Update this with CrOS side mojom copy.
+
+module chromeos.printing.mojom;
+
+// The CupsProxyService's should lazily start once the CupsProxyDaemon has
+// started; since the ChromeOS daemon cannot directly connect to the service,
+// the browser must start it. Currently the only way to start a service is by
+// connecting to its interface, so we explicitly define an interface for this
+// purpose.
+// Note: This service both manages its own lifetime and bootstraps its own
+// connection with the daemon, i.e. it rejects any interface requests besides
+// this one.
+interface StartCupsProxyService {
+};

chromeos/dbus/cups_proxy/cups_proxy_client.cc

@@ -13,7 +13,6 @@
 #include "chromeos/dbus/cups_proxy/fake_cups_proxy_client.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
-#include "dbus/object_proxy.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
@@ -27,7 +26,7 @@ class CupsProxyClientImpl : public CupsProxyClient {
   CupsProxyClientImpl() = default;
   ~CupsProxyClientImpl() override = default;
 
-  // CupsProxyClient override.
+  // CupsProxyClient overrides.
   void BootstrapMojoConnection(
       base::ScopedFD fd,
       base::OnceCallback<void(bool success)> result_callback) override {
@@ -42,6 +41,12 @@ class CupsProxyClientImpl : public CupsProxyClient {
                        std::move(result_callback)));
   }
 
+  void WaitForServiceToBeAvailable(
+      dbus::ObjectProxy::WaitForServiceToBeAvailableCallback callback)
+      override {
+    daemon_proxy_->WaitForServiceToBeAvailable(std::move(callback));
+  }
+
   void Init(dbus::Bus* const bus) {
     daemon_proxy_ =
         bus->GetObjectProxy(::printing::kCupsProxyDaemonName,

chromeos/dbus/cups_proxy/cups_proxy_client.h

@@ -10,6 +10,7 @@
 #include "base/callback_forward.h"
 #include "base/component_export.h"
 #include "base/files/scoped_file.h"
+#include "dbus/object_proxy.h"
 
 namespace dbus {
 class Bus;
@@ -35,6 +36,14 @@ class COMPONENT_EXPORT(CUPS_PROXY) CupsProxyClient {
   // Returns the global instance which may be null if not initialized.
   static CupsProxyClient* Get();
 
+  // Registers |callback| to run when the CupsProxyDaemon becomes available.
+  // If the daemon is already available, or if connecting to the name-owner-
+  // changed signal fails, |callback| will be run once asynchronously.
+  // Otherwise, |callback| will be run once in the future after the service
+  // becomes available.
+  virtual void WaitForServiceToBeAvailable(
+      dbus::ObjectProxy::WaitForServiceToBeAvailableCallback callback) = 0;
+
   // Passes the file descriptor |fd| over D-Bus to the CupsProxyDaemon.
   // * The daemon expects a Mojo invitation in |fd| with an attached Mojo pipe.
   // * The daemon will bind the Mojo pipe to an

chromeos/dbus/cups_proxy/fake_cups_proxy_client.cc

@@ -6,13 +6,21 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/callback.h"
+#include "base/threading/thread_task_runner_handle.h"
 
 namespace chromeos {
 
 FakeCupsProxyClient::FakeCupsProxyClient() = default;
 FakeCupsProxyClient::~FakeCupsProxyClient() = default;
 
+void FakeCupsProxyClient::WaitForServiceToBeAvailable(
+    dbus::ObjectProxy::WaitForServiceToBeAvailableCallback callback) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), true));
+}
+
 void FakeCupsProxyClient::BootstrapMojoConnection(
     base::ScopedFD fd,
     base::OnceCallback<void(bool success)> result_callback) {

chromeos/dbus/cups_proxy/fake_cups_proxy_client.h

@@ -9,6 +9,7 @@
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
 #include "chromeos/dbus/cups_proxy/cups_proxy_client.h"
+#include "dbus/object_proxy.h"
 
 namespace chromeos {
 
@@ -18,7 +19,9 @@ class FakeCupsProxyClient : public CupsProxyClient {
   FakeCupsProxyClient();
   ~FakeCupsProxyClient() override;
 
-  // CupsProxyClient override.
+  // CupsProxyClient:
+  void WaitForServiceToBeAvailable(
+      dbus::ObjectProxy::WaitForServiceToBeAvailableCallback callback) override;
   void BootstrapMojoConnection(
       base::ScopedFD fd,
       base::OnceCallback<void(bool success)> result_callback) override;