Remove success field from PrinterSetupResponse

The success field is redundant - failure is indicated by calling
RejectJavascriptCallback() instead of ResolveJavascriptCallback().

Bug: none
Change-Id: Ic300a2ae8c4521cdf71ab9505437b6551e2e9122
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3059816
Commit-Queue: Pranav Batra <batrapranav@chromium.org>
Auto-Submit: Pranav Batra <batrapranav@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#907325}

chrome/browser/resources/print_preview/native_layer_cros.js

@@ -13,7 +13,6 @@ import {PrinterStatus, PrinterStatusReason} from './data/printer_status_cros.js'
 /**
  * @typedef {{
  *   printerId: string,
- *   success: boolean,
  *   capabilities: !Cdd,
  *   policies: (DestinationPolicies | undefined),
  * }}

chrome/browser/resources/print_preview/ui/destination_dialog_cros.js

@@ -298,17 +298,15 @@ Polymer({
         .then(
             response => {
               this.destinationInConfiguring_ = null;
-              listItem.onConfigureComplete(response.success);
-              if (response.success) {
-                destination.capabilities = response.capabilities;
-                if (response.policies) {
-                  destination.policies = response.policies;
-                }
-                this.selectDestination_(destination);
-                // After destination is selected, start fetching for the EULA
-                // URL.
-                this.destinationStore.fetchEulaUrl(destination.id);
+              listItem.onConfigureComplete(true);
+              destination.capabilities = response.capabilities;
+              if (response.policies) {
+                destination.policies = response.policies;
               }
+              this.selectDestination_(destination);
+              // After destination is selected, start fetching for the EULA
+              // URL.
+              this.destinationStore.fetchEulaUrl(destination.id);
             },
             () => {
               this.destinationInConfiguring_ = null;

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

@@ -259,37 +259,36 @@ void PrintPreviewHandlerChromeOS::SendEulaUrl(const std::string& callback_id,
   ResolveJavascriptCallback(base::Value(callback_id), base::Value(eula_url));
 }
 
-// Resolves the callback (via ResolveJavascriptCallback) with a
-// PrinterSetupResponse object (defined in
-// chrome/browser/resources/print_preview/native_layer_cros.js).
-// destination_info is a CapabilitiesResponse object (defined in
+// Resolves the callback with a PrinterSetupResponse object (defined in
+// chrome/browser/resources/print_preview/native_layer_cros.js) or rejects it
+// if `destination_info` does not contain a capabilities dictionary.
+// `destination_info` is a CapabilitiesResponse object (defined in
 // chrome/browser/resources/print_preview/native_layer.js).
 void PrintPreviewHandlerChromeOS::SendPrinterSetup(
     const std::string& callback_id,
     const std::string& printer_name,
     base::Value destination_info) {
-  base::Value response(base::Value::Type::DICTIONARY);
   base::Value* caps_value =
       destination_info.is_dict()
           ? destination_info.FindKeyOfType(kSettingCapabilities,
                                            base::Value::Type::DICTIONARY)
           : nullptr;
+  if (!caps_value) {
+    VLOG(1) << "Printer setup failed";
+    RejectJavascriptCallback(base::Value(callback_id), base::Value());
+    return;
+  }
+
+  base::Value response(base::Value::Type::DICTIONARY);
   response.SetKey("printerId", base::Value(printer_name));
-  response.SetKey("success", base::Value(!!caps_value));
-  response.SetKey("capabilities",
-                  caps_value ? std::move(*caps_value)
-                             : base::Value(base::Value::Type::DICTIONARY));
-  if (caps_value) {
-    base::Value* printer =
-        destination_info.FindKeyOfType(kPrinter, base::Value::Type::DICTIONARY);
-    if (printer) {
-      base::Value* policies_value = printer->FindKeyOfType(
-          kSettingPolicies, base::Value::Type::DICTIONARY);
-      if (policies_value)
-        response.SetKey("policies", std::move(*policies_value));
-    }
-  } else {
-    LOG(WARNING) << "Printer setup failed";
+  response.SetKey("capabilities", std::move(*caps_value));
+  base::Value* printer =
+      destination_info.FindKeyOfType(kPrinter, base::Value::Type::DICTIONARY);
+  if (printer) {
+    base::Value* policies_value =
+        printer->FindKeyOfType(kSettingPolicies, base::Value::Type::DICTIONARY);
+    if (policies_value)
+      response.SetKey("policies", std::move(*policies_value));
   }
   ResolveJavascriptCallback(base::Value(callback_id), response);
 }

chrome/test/data/webui/print_preview/destination_search_test.js

@@ -100,11 +100,6 @@ suite(destination_search_test.suiteName, function() {
       assert(destination_search_test.TestNames.GetCapabilitiesSucceeds),
       function() {
         const destId = '00112233DEADBEEF';
-        const response = {
-          printerId: destId,
-          capabilities: getCddTemplate(destId).capabilities,
-          success: true,
-        };
         nativeLayer.setLocalDestinationCapabilities(getCddTemplate(destId));
 
         const waiter = eventToPromise(

chrome/test/data/webui/print_preview/destination_search_test_chromeos.js

@@ -22,7 +22,6 @@ destination_search_test_chromeos.suiteName = 'DestinationSearchTest';
 destination_search_test_chromeos.TestNames = {
   ReceiveSuccessfulSetup: 'receive successful setup',
   ResolutionFails: 'resolution fails',
-  ReceiveFailedSetup: 'receive failed setup',
   CloudKioskPrinter: 'cloud kiosk printer',
   ReceiveSuccessfulSetupWithPolicies: 'receive successful setup with policies',
 };
@@ -113,7 +112,6 @@ suite(destination_search_test_chromeos.suiteName, function() {
         const response = {
           printerId: destId,
           capabilities: getCddTemplate(destId).capabilities,
-          success: true,
         };
         nativeLayerCros.setSetupPrinterResponse(response);
 
@@ -156,30 +154,6 @@ suite(destination_search_test_chromeos.suiteName, function() {
             });
       });
 
-  // Test what happens when the setupPrinter request is resolved with a
-  // failed status. Chrome OS only.
-  test(
-      assert(destination_search_test_chromeos.TestNames.ReceiveFailedSetup),
-      function() {
-        const originalDestination = destinationStore.selectedDestination;
-        const destId = '00112233DEADBEEF';
-        const response = {
-          printerId: destId,
-          capabilities: getCddTemplate(destId).capabilities,
-          success: false,
-        };
-        nativeLayerCros.setSetupPrinterResponse(response);
-        requestSetup(destId);
-        return nativeLayerCros.whenCalled('setupPrinter')
-            .then(function(actualDestId) {
-              assertEquals(destId, actualDestId);
-              // The selected printer should not have changed, since a printer
-              // cannot be selected until setup succeeds.
-              assertEquals(
-                  originalDestination, destinationStore.selectedDestination);
-            });
-      });
-
   // Test what happens when a simulated cloud kiosk printer is selected.
   test(
       assert(destination_search_test_chromeos.TestNames.CloudKioskPrinter),
@@ -218,7 +192,6 @@ suite(destination_search_test_chromeos.suiteName, function() {
             defaultDuplexMode: null,
             defaultPinMode: null,
           },
-          success: true,
         };
         nativeLayerCros.setSetupPrinterResponse(response);
         requestSetup(destId);

chrome/test/data/webui/print_preview/print_preview_ui_browsertest.js

@@ -986,13 +986,6 @@ TEST_F(
           destination_search_test_chromeos.TestNames.ResolutionFails);
     });
 
-TEST_F(
-    'PrintPreviewDestinationSearchTestChromeOS', 'ReceiveFailedSetup',
-    function() {
-      this.runMochaTest(
-          destination_search_test_chromeos.TestNames.ReceiveFailedSetup);
-    });
-
 TEST_F(
     'PrintPreviewDestinationSearchTestChromeOS',
     'ReceiveSuccessfultSetupWithPolicies', function() {