Added a diagnostic user message when enumerating printers fails. Also tweaked...

chrome/service/cloud_print/cloud_print_proxy_backend.cc

Index: chrome/service/cloud_print/cloud_print_proxy_backend.cc
===================================================================
--- chrome/service/cloud_print/cloud_print_proxy_backend.cc	(revision 72042)
+++ chrome/service/cloud_print/cloud_print_proxy_backend.cc	(working copy)
@@ -80,6 +80,7 @@
   virtual void OnPrinterJobHandlerShutdown(PrinterJobHandler* job_handler,
                                    const std::string& printer_id);
   virtual void OnAuthError();
+  virtual bool OnPrinterNotFound(const std::string& printer_name);
 
   // notifier::TalkMediator::Delegate implementation.
   virtual void OnNotificationStateChange(
@@ -115,11 +116,17 @@
       DictionaryValue* json_data,
       bool succeeded);
 
-CloudPrintURLFetcher::ResponseAction HandlePrintSystemUnavailableResponse(
+  CloudPrintURLFetcher::ResponseAction HandlePrintSystemUnavailableResponse(
       const URLFetcher* source,
       const GURL& url,
       DictionaryValue* json_data,
       bool succeeded);
+
+  CloudPrintURLFetcher::ResponseAction HandleEnumPrintersFailedResponse(
+      const URLFetcher* source,
+      const GURL& url,
+      DictionaryValue* json_data,
+      bool succeeded);
   // End response handlers
 
   // NotifyXXX is how the Core communicates with the frontend across
@@ -150,9 +157,10 @@
   // handler is responsible for checking for pending print jobs for this
   // printer and print them.
   void InitJobHandlerForPrinter(DictionaryValue* printer_data);
-  // Sends a diagnostic message to the cloud print server that the print
-  // system failed to initialize.
-  void ReportPrintSystemUnavailable(const std::string& failure_message);
+  // Reports a diagnostic message to the server.
+  void ReportUserMessage(const std::string& message_id,
+                         const std::string& failure_message,
+                         ResponseHandler handler);
 
   // Callback method for GetPrinterCapsAndDefaults.
   void OnReceivePrinterCaps(
@@ -181,6 +189,9 @@
   // user a chance to further trim the list. When the frontend gives us the
   // final list we make a copy into this so that we can start registering.
   printing::PrinterList printer_list_;
+  // Indicates whether the printers in printer_list_ is the complete set of
+  // printers to be registered for this proxy.
+  bool complete_list_available_;
   // The CloudPrintURLFetcher instance for the current request.
   scoped_refptr<CloudPrintURLFetcher> request_;
   // The index of the nex printer to be uploaded.
@@ -280,6 +291,7 @@
                                    const DictionaryValue* print_system_settings)
     : backend_(backend),
       cloud_print_server_url_(cloud_print_server_url),
+      complete_list_available_(false),
       next_upload_index_(0),
       next_response_handler_(NULL),
       new_printers_available_(false),
@@ -382,17 +394,30 @@
     StartRegistration();
   } else {
     // We could not initialize the print system. We need to notify the server.
-    ReportPrintSystemUnavailable(result.message());
+    ReportUserMessage(
+        kPrintSystemFailedMessageId,
+        result.message(),
+        &CloudPrintProxyBackend::Core::HandlePrintSystemUnavailableResponse);
   }
 }
 
 void CloudPrintProxyBackend::Core::StartRegistration() {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   printer_list_.clear();
-  print_system_->EnumeratePrinters(&printer_list_);
-  // Now we need to ask the server about printers that were registered on the
-  // server so that we can trim this list.
-  GetRegisteredPrinters();
+  cloud_print::PrintSystem::PrintSystemResult result =
+      print_system_->EnumeratePrinters(&printer_list_);
+  complete_list_available_ = result.succeeded();
+  if (!result.succeeded() && !result.message().empty()) {

[gene1, 2011/01/24 19:46:47]

I think we should not use message here to decide whether to proceed with
registration or not. It should be possible to fail without a valid message.

Since we need a 3-state indicator here, let's return explicit 'partial_results'
flag, or at least check the returning list for the values in case of the error.

[sanjeevr, 2011/01/24 19:59:05]

We can have a fallback message. I don't see why we need a 3-state indicator.

+    // There was a failure enumerating printers. Send a message to the server.
+    ReportUserMessage(
+        kEnumPrintersFailedMessageId,
+        result.message(),
+        &CloudPrintProxyBackend::Core::HandleEnumPrintersFailedResponse);
+  } else {
+    // Now we need to ask the server about printers that were registered on the
+    // server so that we can trim this list.
+    GetRegisteredPrinters();
+  }
 }
 
 void CloudPrintProxyBackend::Core::EndRegistration() {
@@ -478,10 +503,6 @@
     const std::string& printer_name,
     const printing::PrinterCapsAndDefaults& caps_and_defaults) {
   DCHECK(next_upload_index_ < printer_list_.size());
-  std::string mime_boundary;
-  CloudPrintHelpers::CreateMimeBoundaryForUpload(&mime_boundary);
-  std::string post_data;
-  GURL post_url;
   if (succeeded) {
     const printing::PrinterBasicInfo& info =
         printer_list_.at(next_upload_index_);
@@ -489,6 +510,10 @@
     last_uploaded_printer_name_ = info.printer_name;
     last_uploaded_printer_info_ = caps_and_defaults;
 
+    std::string mime_boundary;
+    CloudPrintHelpers::CreateMimeBoundaryForUpload(&mime_boundary);
+    std::string post_data;
+
     CloudPrintHelpers::AddMultipartValueForUpload(kProxyIdValue, proxy_id_,
                                                   mime_boundary,
                                                   std::string(), &post_data);
@@ -522,38 +547,32 @@
         kPrinterCapsHashValue,
         MD5String(last_uploaded_printer_info_.printer_capabilities),
         mime_boundary, std::string(), &post_data);
-    post_url = CloudPrintHelpers::GetUrlForPrinterRegistration(
+    GURL post_url = CloudPrintHelpers::GetUrlForPrinterRegistration(
         cloud_print_server_url_);
 
     next_response_handler_ =
         &CloudPrintProxyBackend::Core::HandleRegisterPrinterResponse;
+    // Terminate the request body
+    post_data.append("--" + mime_boundary + "--\r\n");
+    std::string mime_type("multipart/form-data; boundary=");
+    mime_type += mime_boundary;
+    request_ = new CloudPrintURLFetcher;
+    request_->StartPostRequest(post_url, this, auth_token_,
+                               kCloudPrintAPIMaxRetryCount, mime_type,
+                               post_data);
   } else {
     LOG(ERROR) << "CP_PROXY: Failed to get printer info for: " <<
         printer_name;
     // This printer failed to register, notify the server of this failure.
-    post_url = CloudPrintHelpers::GetUrlForUserMessage(
-        cloud_print_server_url_,
-        kGetPrinterCapsFailedMessageId);
     string16 printer_name_utf16 = UTF8ToUTF16(printer_name);
     std::string status_message = l10n_util::GetStringFUTF8(
         IDS_CLOUD_PRINT_REGISTER_PRINTER_FAILED,
         printer_name_utf16);
-    CloudPrintHelpers::AddMultipartValueForUpload(kMessageTextValue,
-                                                  status_message,
-                                                  mime_boundary,
-                                                  std::string(),
-                                                  &post_data);
-    next_response_handler_ =
-        &CloudPrintProxyBackend::Core::HandleRegisterFailedStatusResponse;
+    ReportUserMessage(
+        kGetPrinterCapsFailedMessageId,
+        status_message,
+        &CloudPrintProxyBackend::Core::HandleRegisterFailedStatusResponse);
   }
-  // Terminate the request body
-  post_data.append("--" + mime_boundary + "--\r\n");
-  std::string mime_type("multipart/form-data; boundary=");
-  mime_type += mime_boundary;
-  request_ = new CloudPrintURLFetcher;
-  request_->StartPostRequest(post_url, this, auth_token_,
-                             kCloudPrintAPIMaxRetryCount, mime_type,
-                             post_data);
 }
 
 void CloudPrintProxyBackend::Core::HandlePrinterNotification(
@@ -725,22 +744,23 @@
   }
 }
 
-void CloudPrintProxyBackend::Core::ReportPrintSystemUnavailable(
-    const std::string& failure_message) {
+void CloudPrintProxyBackend::Core::ReportUserMessage(
+    const std::string& message_id,
+    const std::string& failure_message,
+    ResponseHandler handler) {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   std::string mime_boundary;
   CloudPrintHelpers::CreateMimeBoundaryForUpload(&mime_boundary);
   GURL post_url = CloudPrintHelpers::GetUrlForUserMessage(
       cloud_print_server_url_,
-      kPrintSystemFailedMessageId);
+      message_id);
   std::string post_data;
   CloudPrintHelpers::AddMultipartValueForUpload(kMessageTextValue,
                                                 failure_message,
                                                 mime_boundary,
                                                 std::string(),
                                                 &post_data);
-  next_response_handler_ =
-      &CloudPrintProxyBackend::Core::HandlePrintSystemUnavailableResponse;
+  next_response_handler_ = handler;
   // Terminate the request body
   post_data.append("--" + mime_boundary + "--\r\n");
   std::string mime_type("multipart/form-data; boundary=");
@@ -807,6 +827,19 @@
   return CloudPrintURLFetcher::STOP_PROCESSING;
 }
 
+CloudPrintURLFetcher::ResponseAction
+CloudPrintProxyBackend::Core::HandleEnumPrintersFailedResponse(
+    const URLFetcher* source,
+    const GURL& url,
+    DictionaryValue* json_data,
+    bool succeeded) {
+  DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
+  // Now proceed with printer registration.
+  GetRegisteredPrinters();
+  return CloudPrintURLFetcher::STOP_PROCESSING;
+}
+
+
 bool CloudPrintProxyBackend::Core::RemovePrinterFromList(
     const std::string& printer_name) {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
@@ -883,3 +916,10 @@
   backend_->frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
       &Core::NotifyAuthenticationFailed));
 }
+
+bool CloudPrintProxyBackend::Core::OnPrinterNotFound(

[gene1, 2011/01/24 19:46:47]

I am confused with this function. Usually OnSomething is a notification and does
not return bool. Also, 'not found' looks opposite to 'complete list available'.

[sanjeevr, 2011/01/24 19:59:05]

I can pass an out param instead of returning a bool.

+    const std::string& printer_name) {
+  // If we have a complete list of local printers, then this needs to be deleted
+  // from the server.
+  return complete_list_available_;
+}