Implement basic USB setup in the cups printer detector. Factor out

chrome/browser/chromeos/printer_detector/cups_printer_detector.cc

Index: chrome/browser/chromeos/printer_detector/cups_printer_detector.cc
diff --git a/chrome/browser/chromeos/printer_detector/cups_printer_detector.cc b/chrome/browser/chromeos/printer_detector/cups_printer_detector.cc
index 4532256dc3c09642ac36568617d8fce6372f67d8..1fab4cc31b08a38fda24798e1f2f91e6f36e061c 100644
--- a/chrome/browser/chromeos/printer_detector/cups_printer_detector.cc
+++ b/chrome/browser/chromeos/printer_detector/cups_printer_detector.cc
@@ -15,17 +15,17 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/printer_detector/printer_detector.h"
+#include "chrome/browser/chromeos/printing/ppd_provider_factory.h"
+#include "chrome/browser/chromeos/printing/printer_configurer.h"
+#include "chrome/browser/chromeos/printing/printers_manager_factory.h"
+#include "chrome/browser/chromeos/printing/usb_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/notifications/notification.h"
-#include "chrome/browser/notifications/notification_delegate.h"
-#include "chrome/browser/notifications/notification_ui_manager.h"
-#include "chrome/browser/ui/browser_navigator.h"
-#include "chrome/browser/ui/browser_navigator_params.h"
-#include "chrome/common/url_constants.h"
-#include "chrome/grit/generated_resources.h"
-#include "chrome/grit/theme_resources.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/debug_daemon_client.h"
+#include "chromeos/printing/ppd_provider.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
+#include "content/public/browser/browser_thread.h"
 #include "device/base/device_client.h"
 #include "device/usb/usb_device.h"
 #include "device/usb/usb_device_filter.h"
@@ -33,44 +33,33 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/one_shot_event.h"
-#include "ui/base/l10n/l10n_util.h"
-#include "ui/base/resource/resource_bundle.h"
 
 namespace chromeos {
 namespace {
 
-const char kUSBPrinterFoundNotificationID[] =
-    "chrome://settings/cupsPrinters/printer_found";
+using printing::PpdProvider;
 
-// Base class used for printer USB interfaces
-// (https://www.usb.org/developers/defined_class).
-const uint8_t kPrinterInterfaceClass = 7;
+// We have to carry quite a bit of data through to the callbacks for
+// getting a printer set up.  Rather than have a bunch of parameters
+// in a bunch of functions below, this struct aggregates everything
+// we need to keep around during the setup.
+struct SetUpPrinterData {
+  // The configurer running the SetUpPrinter call.
+  std::unique_ptr<PrinterConfigurer> configurer;
 
-// USBPrinterSetupNotificationDelegate takes a pointer to the Impl class, so
-// we have to forward declare it.
-class CupsPrinterDetectorImpl;
+  // The printer being set up.
+  std::unique_ptr<Printer> printer;
 
-class USBPrinterSetupNotificationDelegate : public NotificationDelegate {
- public:
-  explicit USBPrinterSetupNotificationDelegate(
-      CupsPrinterDetectorImpl* printer_detector)
-      : printer_detector_(printer_detector) {}
-
-  // NotificationDelegate override:
-  std::string id() const override { return kUSBPrinterFoundNotificationID; }
-
-  // This is defined out of line because it needs the PrinterDetectorImpl
-  // full class declaration, not just the forward declaration.
-  void ButtonClick(int button_index) override;
-
-  bool HasClickedListener() override { return true; }
-
- private:
-  ~USBPrinterSetupNotificationDelegate() override = default;
+  // The usb device causing this setup flow.
+  scoped_refptr<device::UsbDevice> device;
 
-  CupsPrinterDetectorImpl* printer_detector_;
+  // True if this printer is one that the user doesn't already have a
+  // configuration for.
+  bool is_new;
 
-  DISALLOW_COPY_AND_ASSIGN(USBPrinterSetupNotificationDelegate);
+  // True if this was a printer that was plugged in during the session, false if
+  // it was already plugged in when the session started.
+  bool hotplugged;
 };
 
 // The PrinterDetector that drives the flow for setting up a USB printer to use
@@ -86,149 +75,210 @@ class CupsPrinterDetectorImpl : public PrinterDetector,
   }
   ~CupsPrinterDetectorImpl() override = default;
 
-  void ClickOnNotificationButton(int button_index) {
-    // Remove the old notification first.
-    const ProfileID profile_id = NotificationUIManager::GetProfileID(profile_);
-    g_browser_process->notification_ui_manager()->CancelById(
-        kUSBPrinterFoundNotificationID, profile_id);
-
-    if (command_ == ButtonCommand::SETUP) {
-      OnSetUpUSBPrinterStarted();
-      // TODO(skau/xdai): call the CUPS backend to set up the USB printer and
-      // then call OnSetUpPrinterDone() or OnSetUpPrinterError() depending on
-      // the setup result.
-    } else if (command_ == ButtonCommand::CANCEL_SETUP) {
-      // TODO(skau/xdai): call the CUPS backend to cancel the printer setup.
-    } else if (command_ == ButtonCommand::GET_HELP) {
-      chrome::NavigateParams params(profile_,
-                                    GURL(chrome::kChromeUIMdCupsSettingsURL),
-                                    ui::PAGE_TRANSITION_LINK);
-      params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
-      params.window_action = chrome::NavigateParams::SHOW_WINDOW;
-      chrome::Navigate(&params);
-    }
-  }
-
  private:
-  // Action that should be performed when a notification button is clicked.
-  enum class ButtonCommand {
-    SETUP,
-    CANCEL_SETUP,
-    CLOSE,
-    GET_HELP,
-  };
-
-  // UsbService::observer override:
-  void OnDeviceAdded(scoped_refptr<device::UsbDevice> device) override {
+  // Called when the extension system is "ready", meaning everything has been
+  // created.
+  void Initialize() {
     const user_manager::User* user =
         ProfileHelper::Get()->GetUserByProfile(profile_);
+    // Don't bother to listen for USB events unless we are the active user.
+    //
     // TODO(justincarlson) - See if it's appropriate to relax any of these
     // constraints.
     if (!user || !user->HasGaiaAccount() || !user_manager::UserManager::Get() ||
         user != user_manager::UserManager::Get()->GetActiveUser()) {
       return;
     }
-
-    device::UsbDeviceFilter printer_filter;
-    printer_filter.interface_class = kPrinterInterfaceClass;
-    if (!printer_filter.Matches(device))
+    device::UsbService* usb_service =
+        device::DeviceClient::Get()->GetUsbService();
+    if (!usb_service) {
       return;
+    }
+    observer_.Add(usb_service);
+    usb_service->GetDevices(base::Bind(&CupsPrinterDetectorImpl::OnGetDevices,
+                                       weak_ptr_factory_.GetWeakPtr()));
+  }
 
-    ShowUSBPrinterSetupNotification(device);
+  // Callback for initial enumeration of usb devices.  UsbService invokes
+  // this on the UI thread.
+  void OnGetDevices(
+      const std::vector<scoped_refptr<device::UsbDevice>>& devices) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    for (const auto& device : devices) {
+      if (!UsbDeviceIsPrinter(device)) {
+        continue;
+      }
+      if (!known_printers_.insert(device->guid()).second) {

[skau, 2017/03/11 00:00:34]

Is this a common idiom?  I didn't know that map::insert returned pair<iterator,
bool> until I just looked it up.

[Carlson, 2017/03/11 01:19:17]

I would have guessed yes, but apparently not:

https://cs.chromium.org/search/?q=%5C.insert%5C(.*%5C)%5C.second+package:%5Ec...

Broke it up into multiple lines.  I generally default to *not* breaking up stuff
like this because doing so hits the map and hash function multiple times without
need, but OTOH we don't seem to have the sort of "bool InsertIfNotPresent(...)"
function around that makes that easier to read...

(Note this moved into [Maybe]SetUpDevice())

+        // Already dealt with this, don't need to do so again.
+        continue;
+      }
+      SetUpDevice(device, false);
+    }
   }
 
-  // Initializes the printer detector.
-  void Initialize() {
-    device::UsbService* usb_service =
-        device::DeviceClient::Get()->GetUsbService();
-    if (!usb_service)
+  // UsbService::observer override.  This runs on the UI thread.
+  void OnDeviceAdded(scoped_refptr<device::UsbDevice> device) override {

[skau, 2017/03/11 00:00:34]

nit: DRY

[Carlson, 2017/03/11 01:19:16]

Not sure which part you're referring to.  You mean the stuff before SetUpDevice
that's common between here and above?  The number of times "device" appears in
this line?

Assuming the former, moved common stuff into SetUpDevice.

[skau, 2017/03/13 21:56:25]

Yes.  I was referring to the code above.  Sorry for being unclear.

[Carlson, 2017/03/15 00:31:15]

Done.

+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    if (!UsbDeviceIsPrinter(device)) {
       return;
-    observer_.Add(usb_service);
+    }
+    if (!known_printers_.insert(device->guid()).second) {
+      // Already dealt with this, don't need to do so again.
+      return;
+    }
+    SetUpDevice(device, true);
   }
 
-  void SetNotificationUIManagerForTesting(
-      NotificationUIManager* manager) override {
-    LOG(FATAL) << "Not implemented for CUPS";
+  // UsbService::observer override.  This runs on the UI thread.
+  void OnDeviceRemoved(scoped_refptr<device::UsbDevice> device) override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    if (!UsbDeviceIsPrinter(device)) {
+      return;
+    }
+    known_printers_.erase(device->guid());
+
+    // If this printer appears in the list of printers we've failed to set up,
+    // remove it.
+    failed_setup_printers_.erase(device->guid());
   }
 
-  void ShowUSBPrinterSetupNotification(
-      scoped_refptr<device::UsbDevice> device) {
-    // TODO(justincarlson) - Test this notification across a wide variety of
-    // less-than-sane printers to make sure the notification text stays as
-    // intelligible as possible.
-    base::string16 printer_name = device->manufacturer_string() +
-                                  base::UTF8ToUTF16(" ") +
-                                  device->product_string();
-    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-    message_center::RichNotificationData data;
-    data.buttons.push_back(message_center::ButtonInfo(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_BUTTON)));
-    notification_.reset(new Notification(
-        message_center::NOTIFICATION_TYPE_SIMPLE,
-        l10n_util::GetStringUTF16(
-            IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_TITLE),      // title
-        printer_name,                                             // body
-        bundle.GetImageNamed(IDR_PRINTER_DETECTED_NOTIFICATION),  // icon
-        message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
-                                   kUSBPrinterFoundNotificationID),
-        base::string16(),  // display_source
-        GURL(), kUSBPrinterFoundNotificationID, data,
-        new USBPrinterSetupNotificationDelegate(this)));
-    notification_->SetSystemPriority();
-    command_ = ButtonCommand::SETUP;
-
-    g_browser_process->notification_ui_manager()->Add(*notification_, profile_);
+  // This is called on the UI thread.
+  void SetUpDevice(scoped_refptr<device::UsbDevice> printer_device,
+                   bool hotplugged) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    auto data = base::MakeUnique<SetUpPrinterData>();
+    data->configurer = PrinterConfigurer::Create(profile_);
+    data->printer = base::MakeUnique<Printer>();
+    data->device = printer_device;
+    data->is_new = true;
+    data->hotplugged = hotplugged;
+    if (!UsbDeviceToPrinter(*printer_device, data->printer.get())) {
+      LOG(ERROR)
+          << "Failed to convert usb device to printer, abandoning setup.  "
+          << "Usb device details follow:";
+      LOG(ERROR) << UsbPrinterDeviceDetailsAsString(*printer_device);
+      return;
+    }
+    auto existing_printers =
+        PrintersManagerFactory::GetForBrowserContext(profile_)->GetPrinters();

[skau, 2017/03/11 00:00:34]

TODO: add a GetPrinterByUri to PrintersManagerFactory

[Carlson, 2017/03/11 01:19:16]

Done.

+    for (std::unique_ptr<Printer>& printer : existing_printers) {
+      if (printer->uri() == data->printer->uri()) {
+        // Found a match, so use the existing configuration.
+        *(data->printer) = *printer;
+        data->is_new = false;
+
+        // Copy fields that will be invalidated by std::move.
+        auto* configurer_tmp = data->configurer.get();
+        const Printer& printer_tmp = *(data->printer);
+        // off to the configurer.
+        configurer_tmp->SetUpPrinter(
+            printer_tmp, base::Bind(&CupsPrinterDetectorImpl::SetUpPrinterDone,
+                                    weak_ptr_factory_.GetWeakPtr(),
+                                    base::Passed(std::move(data))));
+        return;
+      }
+    }
+
+    // It's not a device we know about.  First we see if we can get an exact
+    // driver match based on USB ids.
+    //
+    // TODO(justincarlson): Add a notification that we are attempting to set up
+    // this printer at this point.
+    auto ppd_provider = printing::CreateProvider(profile_);

[skau, 2017/03/11 00:00:34]

scoped_refptf<PpdProvider>.  In this case, it's important we know the exact
type.

[Carlson, 2017/03/11 01:19:16]

Done.

+    ppd_provider->ResolveUsbIds(
+        printer_device->vendor_id(), printer_device->product_id(),
+        base::Bind(&CupsPrinterDetectorImpl::ResolveUsbIdsDone,
+                   weak_ptr_factory_.GetWeakPtr(), ppd_provider,
+                   base::Passed(std::move(data))));
   }
 
-  void OnSetUpUSBPrinterStarted() {
-    notification_->set_title(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_IN_PROGRESS_TITLE));
-    notification_->set_type(message_center::NOTIFICATION_TYPE_PROGRESS);
-    notification_->set_progress(-1);
-    std::vector<message_center::ButtonInfo> buttons;
-    buttons.push_back(message_center::ButtonInfo(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_CANCEL_BUTTON)));
-    notification_->set_buttons(buttons);
-    command_ = ButtonCommand::CANCEL_SETUP;
-    g_browser_process->notification_ui_manager()->Add(*notification_, profile_);
+  // Called when the query for a driver based on usb ids completes.
+  void ResolveUsbIdsDone(scoped_refptr<PpdProvider> provider,
+                         std::unique_ptr<SetUpPrinterData> data,
+                         PpdProvider::CallbackResultCode result,
+                         const std::string& effective_make_and_model) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    if (result == PpdProvider::SUCCESS) {
+      // Got something based on usb ids.  Go with it.
+      data->printer->mutable_ppd_reference()->effective_make_and_model =
+          effective_make_and_model;
+    } else {
+      // Couldn't figure this printer out based on usb ids, fall back to
+      // guessing the make/model string from what the USB system reports.
+      //
+      // TODO(justincarlson): Consider adding a mechanism for aggregating data
+      // about which usb devices are in the wild but unsupported?
+      //
+      // TODO(justincarlson): Possibly go deeper and query the IEEE1284 fields
+      // for make and model if we determine those are more likely to contain
+      // what we want.
+      data->printer->mutable_ppd_reference()->effective_make_and_model =
+          base::UTF16ToUTF8(data->device->manufacturer_string()) + " " +
+          base::UTF16ToUTF8(data->device->product_string());

[skau, 2017/03/11 00:00:34]

This function should have a name.

[Carlson, 2017/03/11 01:19:16]

A function had no name.  (Done.)

+    }
+
+    // Have to keep copies of some parameters that will be moved in the
+    // arguments and used in the same statement.
+    auto* configurer = data->configurer.get();
+    auto* printer = data->printer.get();
+
+    configurer->SetUpPrinter(
+        *printer, base::Bind(&CupsPrinterDetectorImpl::SetUpPrinterDone,
+                             weak_ptr_factory_.GetWeakPtr(),
+                             base::Passed(std::move(data))));
   }
 
-  void OnSetUpUSBPrinterDone() {
-    notification_->set_title(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_SUCCESS_TITLE));
-    notification_->set_type(message_center::NOTIFICATION_TYPE_SIMPLE);
-    std::vector<message_center::ButtonInfo> buttons;
-    buttons.push_back(message_center::ButtonInfo(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_CLOSE_BUTTON)));
-    notification_->set_buttons(buttons);
-    command_ = ButtonCommand::CLOSE;
-    g_browser_process->notification_ui_manager()->Add(*notification_, profile_);
+  // Called on the UI thread with the result of asking to have a printer
+  // configured for cups.  If |printer_to_register| is non-null and we

[skau, 2017/03/11 00:00:34]

nit: s/cups/CUPS/

+  // successfully configured, then the printer is registered with the printers
+  // manager.
+  void SetUpPrinterDone(std::unique_ptr<SetUpPrinterData> data,
+                        PrinterSetupResult result) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    if (result == PrinterSetupResult::SUCCESS) {
+      if (data->is_new) {
+        PrintersManagerFactory::GetForBrowserContext(profile_)->RegisterPrinter(
+            std::move(data->printer));
+      }
+      if (data->hotplugged) {
+        // TODO(justincarlson): Pop a timed notification that says the printer
+        // is now available for printing.
+      }
+    } else {
+      // TODO(justincarlson): Pop a notification that tells the user automatic
+      // setup failed and offers to open the CUPS printer configuration
+      // settings.
+      DCHECK(!base::ContainsKey(failed_setup_printers_, data->device->guid()));
+      failed_setup_printers_.insert({data->device->guid(), *(data->printer)});
+    }
   }
 
-  void OnSetUpUSBPrinterError() {
-    notification_->set_title(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_FAILED_TITLE));
-    notification_->set_type(message_center::NOTIFICATION_TYPE_SIMPLE);
-    std::vector<message_center::ButtonInfo> buttons;
-    buttons.push_back(message_center::ButtonInfo(l10n_util::GetStringUTF16(
-        IDS_PRINTER_DETECTED_NOTIFICATION_SET_UP_GET_HELP_BUTTON)));
-    notification_->set_buttons(buttons);
-    command_ = ButtonCommand::GET_HELP;
-    g_browser_process->notification_ui_manager()->Add(*notification_, profile_);
+  void SetNotificationUIManagerForTesting(
+      NotificationUIManager* manager) override {
+    LOG(FATAL) << "Not implemented for CUPS";
   }
 
-  std::unique_ptr<Notification> notification_;
-  ButtonCommand command_ = ButtonCommand::SETUP;
+  // USB GUIDs of printers we've already dealt with.  There's an inherent race
+  // between initially querying all usb devices and receiving a notification
+  // about a new device, so this set lets us guarantee that we handle a given
+  // printer exactly once.
+  std::set<std::string> known_printers_;
+
+  // Printers that this class failed to set up automatically.  This is provided
+  // to the PrinterDiscoverer used in the settings flow to determine which
+  // printers are available to be set up.  This is a map from the USB GUID
+  // to Printer structure.
+  std::unordered_map<std::string, Printer> failed_setup_printers_;
+
+  // The active ppd_provider, if any.  References to ppd_provider_ are managed
+  // on the UI thread.
+  scoped_refptr<printing::PpdProvider> ppd_provider_;

[skau, 2017/03/11 00:00:34]

This member appears unused.

[Carlson, 2017/03/11 01:19:16]

Thanks, was doing this differently before.

 
   Profile* profile_;
   ScopedObserver<device::UsbService, device::UsbService::Observer> observer_;
   base::WeakPtrFactory<CupsPrinterDetectorImpl> weak_ptr_factory_;
 };
 
-void USBPrinterSetupNotificationDelegate::ButtonClick(int button_index) {
-  printer_detector_->ClickOnNotificationButton(button_index);
-}
-
 }  // namespace
 
 // static