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

chrome/service/cloud_print/cloud_print_proxy_backend.cc

 // Copyright (c) 2010 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/service/cloud_print/cloud_print_proxy_backend.h"
 
 #include <map>
 #include <vector>
 
 #include "app/l10n_util.h"
@@ skipping 62 matching lines @@
       bool succeeded);
 
   virtual void OnRequestAuthError();
 
   // cloud_print::PrintServerWatcherDelegate implementation
   virtual void OnPrinterAdded();
   // PrinterJobHandler::Delegate implementation
   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(
       bool notifications_enabled);
   virtual void OnIncomingNotification(
       const IncomingNotificationData& notification_data);
   virtual void OnOutgoingNotification();
 
  private:
   // Prototype for a response handler.
@@ skipping 15 matching lines @@
       const GURL& url,
       DictionaryValue* json_data,
       bool succeeded);
 
   CloudPrintURLFetcher::ResponseAction HandleRegisterFailedStatusResponse(
       const URLFetcher* source,
       const GURL& url,
       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
   // threads.
   void NotifyPrinterListAvailable(
       const printing::PrinterList& printer_list);
@@ skipping 14 matching lines @@
   // Retrieves the list of registered printers for this user/proxy combination
   // from the cloud print server.
   void GetRegisteredPrinters();
   // Removes the given printer from the list. Returns false if the printer
   // did not exist in the list.
   bool RemovePrinterFromList(const std::string& printer_name);
   // Initializes a job handler object for the specified printer. The job
   // 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(
       bool succeeded,
       const std::string& printer_name,
       const printing::PrinterCapsAndDefaults& caps_and_defaults);
 
   void HandlePrinterNotification(const std::string& printer_id);
   void PollForJobs();
   // Schedules a task to poll for jobs. Does nothing if a task is already
   // scheduled.
   void ScheduleJobPoll();
 
   // Our parent CloudPrintProxyBackend
   CloudPrintProxyBackend* backend_;
 
   GURL cloud_print_server_url_;
   scoped_ptr<DictionaryValue> print_system_settings_;
   // Pointer to current print system.
   scoped_refptr<cloud_print::PrintSystem> print_system_;
   // The list of printers to be registered with the cloud print server.
   // To begin with,this list is initialized with the list of local and network
   // printers available. Then we query the server for the list of printers
   // already registered. We trim this list to remove the printers already
   // registered. We then pass a copy of this list to the frontend to give the
   // 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.
   size_t next_upload_index_;
   // The unique id for this proxy
   std::string proxy_id_;
   // The GAIA auth token
   std::string auth_token_;
   // Cached info about the last printer that we tried to upload. We cache this
   // so we won't have to requery the printer if the upload fails and we need
@@ skipping 79 matching lines @@
           core_.get(),
           &CloudPrintProxyBackend::Core::DoRegisterSelectedPrinters,
           printer_list));
 }
 
 CloudPrintProxyBackend::Core::Core(CloudPrintProxyBackend* backend,
                                    const GURL& cloud_print_server_url,
                                    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),
       notifications_enabled_(false),
       job_poll_scheduled_(false) {
   if (print_system_settings) {
     // It is possible to have no print settings specified.
     print_system_settings_.reset(print_system_settings->DeepCopy());
   }
 }
@@ skipping 82 matching lines @@
     talk_mediator_->Login();
 
     print_server_watcher_ = print_system_->CreatePrintServerWatcher();
     print_server_watcher_->StartWatching(this);
 
     proxy_id_ = proxy_id;
 
     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() {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   request_ = NULL;
   if (new_printers_available_) {
     new_printers_available_ = false;
     StartRegistration();
   }
 }
@@ skipping 65 matching lines @@
   } else {
     EndRegistration();
   }
 }
 
 void CloudPrintProxyBackend::Core::OnReceivePrinterCaps(
     bool succeeded,
     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_);
 
     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);
     CloudPrintHelpers::AddMultipartValueForUpload(kPrinterNameValue,
                                                   info.printer_name,
                                                   mime_boundary,
                                                   std::string(), &post_data);
     CloudPrintHelpers::AddMultipartValueForUpload(kPrinterDescValue,
                                                   info.printer_description,
                                                   mime_boundary,
@@ skipping 13 matching lines @@
     CloudPrintHelpers::AddMultipartValueForUpload(
         kPrinterDefaultsValue, last_uploaded_printer_info_.printer_defaults,
         mime_boundary, last_uploaded_printer_info_.defaults_mime_type,
         &post_data);
     // Send a hash of the printer capabilities to the server. We will use this
     // later to check if the capabilities have changed
     CloudPrintHelpers::AddMultipartValueForUpload(
         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(
     const std::string& printer_id) {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   VLOG(1) << "CP_PROXY: Handle printer notification, id: " << printer_id;
   JobHandlerMap::iterator index = job_handler_map_.find(printer_id);
   if (index != job_handler_map_.end())
     index->second->CheckForJobs(kJobFetchReasonNotified);
 }
@@ skipping 151 matching lines @@
     }
     scoped_refptr<PrinterJobHandler> job_handler;
     job_handler = new PrinterJobHandler(printer_info, printer_info_cloud,
                                         auth_token_, cloud_print_server_url_,
                                         print_system_.get(), this);
     job_handler_map_[printer_info_cloud.printer_id] = job_handler;
     job_handler->Initialize();
   }
 }
 
-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=");
   mime_type += mime_boundary;
   request_ = new CloudPrintURLFetcher;
   request_->StartPostRequest(post_url, this, auth_token_,
                              kCloudPrintAPIMaxRetryCount, mime_type,
                              post_data);
 }
 
@@ skipping 46 matching lines @@
     bool succeeded) {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   // Let the frontend know that we do not have a print system.
   backend_->frontend_loop_->PostTask(
       FROM_HERE,
       NewRunnableMethod(this,
                         &Core::NotifyPrintSystemUnavailable));
   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());
   for (printing::PrinterList::iterator index = printer_list_.begin();
        index != printer_list_.end(); index++) {
     if (0 == base::strcasecmp(index->printer_name.c_str(),
                               printer_name.c_str())) {
       index = printer_list_.erase(index);
       return true;
     }
@@ skipping 56 matching lines @@
   VLOG(1) << "CP_PROXY: Printer job handle shutdown, id " << printer_id;
   job_handler_map_.erase(printer_id);
 }
 
 void CloudPrintProxyBackend::Core::OnAuthError() {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   VLOG(1) << "CP_PROXY: Auth Error";
   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_;
+}