Register and select printer on click.

chrome/browser/ui/webui/print_preview/print_preview_handler.cc

Index: chrome/browser/ui/webui/print_preview/print_preview_handler.cc
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
index e07f23435b60f7ed7eb5b3f1dbf6e32e65df4eb3..2d3de08a10d01ea6d7923cbbeae5bd38d013f8e4 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
@@ -50,6 +50,7 @@
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/webui/print_preview/print_preview_ui.h"
 #include "chrome/browser/ui/webui/print_preview/printer_backend_proxy.h"
+#include "chrome/browser/ui/webui/print_preview/printer_capabilities.h"
 #include "chrome/browser/ui/webui/print_preview/printer_handler.h"
 #include "chrome/browser/ui/webui/print_preview/sticky_settings.h"
 #include "chrome/common/chrome_switches.h"
@@ -86,9 +87,12 @@
 #include "third_party/icu/source/i18n/unicode/ulocdata.h"
 
 #if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/printing/printer_pref_manager.h"
+#include "chrome/browser/chromeos/printing/printer_pref_manager_factory.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h"
 #include "chrome/common/url_constants.h"
+#include "chromeos/printing/printer_configuration.h"
 #endif
 
 #if BUILDFLAG(ENABLE_SERVICE_DISCOVERY)
@@ -174,10 +178,6 @@ const char kDefaultDestinationSelectionRules[] =
 // Id of the predefined PDF printer.
 const char kLocalPdfPrinterId[] = "Save as PDF";
 
-// Additional printer capability setting keys.
-const char kPrinterId[] = "printerId";
-const char kPrinterCapabilities[] = "capabilities";
-
 // Get the print job settings dictionary from |args|. The caller takes
 // ownership of the returned DictionaryValue. Returns NULL on failure.
 std::unique_ptr<base::DictionaryValue> GetSettingsDictionary(
@@ -372,27 +372,6 @@ std::unique_ptr<base::DictionaryValue> GetPdfCapabilities(
   return std::unique_ptr<base::DictionaryValue>(description.root().DeepCopy());
 }
 
-std::pair<std::string, std::string> GetPrinterNameAndDescription(
-    const printing::PrinterBasicInfo& printer) {
-#if defined(OS_MACOSX)
-  // On Mac, |printer.printer_description| specifies the printer name and
-  // |printer.printer_name| specifies the device name / printer queue name.
-  const std::string& real_name = printer.printer_description;
-  std::string real_description;
-  const auto it = printer.options.find(kDriverNameTagName);
-  if (it != printer.options.end())
-    real_description = it->second;
-  return std::make_pair(real_name, real_description);
-#elif defined(OS_CHROMEOS)
-  // For Chrome OS |printer.printer_display_name| is the human readable name and
-  // |printer.printer_name| is the name used in CUPS.
-  return std::make_pair(printer.printer_display_name,
-                        printer.printer_description);
-#else
-  return std::make_pair(printer.printer_name, printer.printer_description);
-#endif
-}
-
 void PrintersToValues(const printing::PrinterList& printer_list,
                       base::ListValue* printers) {
   for (const printing::PrinterBasicInfo& printer : printer_list) {
@@ -400,7 +379,8 @@ void PrintersToValues(const printing::PrinterList& printer_list,
         base::MakeUnique<base::DictionaryValue>();
     printer_info->SetString(printing::kSettingDeviceName, printer.printer_name);
 
-    const auto printer_name_description = GetPrinterNameAndDescription(printer);
+    const auto printer_name_description =
+        printing::GetPrinterNameAndDescription(printer);
     const std::string& printer_name = printer_name_description.first;
     const std::string& printer_description = printer_name_description.second;
     printer_info->SetString(printing::kSettingPrinterName, printer_name);
@@ -408,10 +388,11 @@ void PrintersToValues(const printing::PrinterList& printer_list,
                             printer_description);
 
     auto options = base::MakeUnique<base::DictionaryValue>();
-    printer_info->Set(printing::kSettingPrinterOptions, std::move(options));
     for (const auto opt_it : printer.options)
       options->SetString(opt_it.first, opt_it.second);
 
+    printer_info->Set(printing::kSettingPrinterOptions, std::move(options));
+
     printers->Append(std::move(printer_info));
 
     VLOG(1) << "Found printer " << printer_name << " with device name "
@@ -430,55 +411,6 @@ void EnumeratePrinters(Profile* profile, base::ListValue* printers) {
           << " printers";
 }
 
-std::unique_ptr<base::DictionaryValue>
-GetPrinterCapabilitiesOnBlockingPoolThread(const std::string& device_name) {
-  DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
-  DCHECK(!device_name.empty());
-
-  scoped_refptr<printing::PrintBackend> print_backend(
-      printing::PrintBackend::CreateInstance(nullptr));
-
-  VLOG(1) << "Get printer capabilities start for " << device_name;
-  crash_keys::ScopedPrinterInfo crash_key(
-      print_backend->GetPrinterDriverInfo(device_name));
-
-  std::unique_ptr<base::DictionaryValue> printer_info;
-  if (!print_backend->IsValidPrinter(device_name)) {
-    LOG(WARNING) << "Invalid printer " << device_name;
-    return printer_info;
-  }
-
-  printing::PrinterSemanticCapsAndDefaults info;
-  if (!print_backend->GetPrinterSemanticCapsAndDefaults(device_name, &info)) {
-    LOG(WARNING) << "Failed to get capabilities for " << device_name;
-    return printer_info;
-  }
-
-  std::unique_ptr<base::DictionaryValue> printer_capabilities =
-      cloud_print::PrinterSemanticCapsAndDefaultsToCdd(info);
-  if (!printer_capabilities) {
-    LOG(WARNING) << "Failed to convert capabilities for " << device_name;
-    return printer_info;
-  }
-
-  printing::PrinterBasicInfo basic_info;
-  if (!print_backend->GetPrinterBasicInfo(device_name, &basic_info))
-    return printer_info;
-
-  const auto printer_name_description =
-      GetPrinterNameAndDescription(basic_info);
-  const std::string& printer_name = printer_name_description.first;
-  const std::string& printer_description = printer_name_description.second;
-
-  printer_info.reset(new base::DictionaryValue);
-  printer_info->SetString(kPrinterId, device_name);
-  printer_info->SetString(printing::kSettingPrinterName, printer_name);
-  printer_info->SetString(printing::kSettingPrinterDescription,
-                          printer_description);
-  printer_info->Set(kPrinterCapabilities, printer_capabilities.release());
-  return printer_info;
-}
-
 base::LazyInstance<printing::StickySettings> g_sticky_settings =
     LAZY_INSTANCE_INITIALIZER;
 
@@ -612,8 +544,9 @@ void PrintPreviewHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback("print",
       base::Bind(&PrintPreviewHandler::HandlePrint,
                  base::Unretained(this)));
-  web_ui()->RegisterMessageCallback("getPrinterCapabilities",
-      base::Bind(&PrintPreviewHandler::HandleGetPrinterCapabilities,
+  web_ui()->RegisterMessageCallback(
+      "getPrinterCapabilities",
+      base::Bind(&PrintPreviewHandler::HandlePrinterSelected,

[Lei Zhang, 2016/11/02 23:09:22]

This is weird in that the handler name no longer matches the JS call name.

[skau, 2016/11/03 22:38:08]

It is weird.  I've changed the name back since the end result is still the
retrieval of printer capabilities.  Would you find it surprising that a method
named 'HandleGetPrinterCapabilities' also sets up printers?

[Lei Zhang, 2016/11/03 23:41:46]

Well, from the JS perspective, it wants to get the printer capabilities, so if
you do that as promised, the JS code is happy. If there are logging statements
as a side effect, the JS code doesn't care. If some UMA stats get generated as a
side effect, the JS code doesn't care. If there's some printer configuration
that happens...

[skau, 2016/11/04 19:28:58]

Thanks. When the flow to resolve a misconfigured printer is ready, I'll add a
new event to the JS.  The "is it configured" question should be largely
transparent to Chrome and users.

                  base::Unretained(this)));
 #if BUILDFLAG(ENABLE_BASIC_PRINT_DIALOG)
   web_ui()->RegisterMessageCallback("showSystemDialog",
@@ -1074,8 +1007,7 @@ void PrintPreviewHandler::HandleSaveAppState(const base::ListValue* args) {
       preview_web_contents()->GetBrowserContext())->GetPrefs());
 }
 
-void PrintPreviewHandler::HandleGetPrinterCapabilities(
-    const base::ListValue* args) {
+void PrintPreviewHandler::HandlePrinterSelected(const base::ListValue* args) {
   std::string printer_name;
   bool ret = args->GetString(0, &printer_name);
   if (!ret || printer_name.empty())
@@ -1084,19 +1016,22 @@ void PrintPreviewHandler::HandleGetPrinterCapabilities(
   if (printer_name == kLocalPdfPrinterId) {
     std::unique_ptr<base::DictionaryValue> printer_info(
         new base::DictionaryValue);
-    printer_info->SetString(kPrinterId, printer_name);
+    printer_info->SetString(printing::kPrinterId, printer_name);
     printer_info->Set(
-        kPrinterCapabilities,
+        printing::kPrinterCapabilities,
         GetPdfCapabilities(g_browser_process->GetApplicationLocale()));
     SendPrinterCapabilities(printer_name, std::move(printer_info));
     return;
   }
 
-  base::PostTaskAndReplyWithResult(
-      BrowserThread::GetBlockingPool(), FROM_HERE,
-      base::Bind(&GetPrinterCapabilitiesOnBlockingPoolThread, printer_name),
+  printing::PrinterSetupCallback cb =
       base::Bind(&PrintPreviewHandler::SendPrinterCapabilities,
-                 weak_factory_.GetWeakPtr(), printer_name));
+                 weak_factory_.GetWeakPtr(), printer_name);
+
+  BrowserThread::PostBlockingPoolTask(

[Lei Zhang, 2016/11/02 23:09:22]

Why can't we keep this as base::PostTaskAndReplyWithResult() ? Is it because the
callback has to travel a bit further in the ChromeOS workflow?

[skau, 2016/11/03 22:38:08]

I would like to keep it as PostTaskAndReplyWithResult however the callback has
to be passed to a call to dbus which doesn't have a blocking API (it assumes it
has a message loop).  I don't think I can do it without the callback but I'm
open to suggestions.

+      FROM_HERE, base::Bind(&printing::ConfigurePrinterAndFetchCapabilities,
+                            base::Unretained(Profile::FromWebUI(web_ui())),
+                            printer_name, cb));
 }
 
 void PrintPreviewHandler::OnSigninComplete() {