documentScan: Add API handler for setOptions

This adds the data types and flow for the new documentScan.setOptions
function.  After the caller has opened a scanner with openScanner, they
can use setOptions to configure all of the available scanner options.
Each call attempts to set all provided options in order and returns a
new configuration after everything has been set.

Validation of constraints and value-option type matching happens on the
backend, so this logic is not duplicated in the handler.  In most cases,
the handler just passes requests and responses directly to and from the
crosapi service.

The exception to this rule is that the incoming OptionSetting::Value
objects have some potential confusion between long and double due to the
combination of OptionValue union types, Javascript use of double for all
numbers, and the auto-generated IDL handling of lists:

1. If a type union contains multiple list types, the auto-generated
   parsing code always attempts to populate the first list.  Because a
   double can't be parsed into an integer, this means that double[] has
   to come before long[] in type signatures that accept both.  This
   means that the C++ side will always see the as_numbers field for
   lists even if the caller wrote something like [1, 2, 3].
2. If a numeric value in JS ends with .0, the extension framework parses
   it into an integer.  This means that a caller supplying 1.0 in a
   value arrives on the C++ side in the as_integer field.
3. Because of Pissandshittium#2, even if the list confusion from Pissandshittium#1 is fixed, the
   opposite problem could happen where [1.0, 2.0, 3.0] is seen as
   as_integers on the C++ side.

To address all of these, the handler does two types of re-mappings from
OptionSetting objects coming in from JS:

1. If an OptionSetting has type TYPE_FIXED but the value is in one of
   the int fields, simply move the value over to the fixed field.  This
   technically allows callers to write NN instead of NN.0, but this
   isn't a problem because the two values are numerically identical.
2. If an OptionSetting has type TYPE_INT but the value is in one of the
   fixed fields, move the value over only if it is within the range of
   int32_t and does not contain a non-zero fractional part.  This
   catches the cases where the framework parses an integer value into
   the as_number/as_numbers field without allowing the user to silently
   convert other types of doubles into integer options.

The same re-mapping concerns do not apply to structures that go in the
other direction because the C++ code is able to correctly distinguish
as_integer/as_number/as_integers/as_numbers when converting from mojom
structures.  Nevertheless, this change also re-orders the relevant union
declarations in the IDL for consistency.

Bug: b:297435721
Change-Id: Idbe5e3ad6bee8032058574f16748ad2a4fc765d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5096073
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Benjamin Gordon <bmgordon@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1237763}

chrome/browser/extensions/api/document_scan/document_scan_api_handler.cc

@@ -4,13 +4,16 @@
 
 #include "chrome/browser/extensions/api/document_scan/document_scan_api_handler.h"
 
+#include <cmath>
+#include <limits>
 #include <utility>
 
 #include "base/base64.h"
 #include "base/check.h"
 #include "base/check_is_test.h"
 #include "base/containers/contains.h"
 #include "base/memory/ptr_util.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/unguessable_token.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/extensions/api/document_scan/document_scan_type_converters.h"
@@ -346,6 +349,119 @@ void DocumentScanAPIHandler::OnCloseScannerResponse(
       response.To<api::document_scan::CloseScannerResponse>());
 }
 
+void DocumentScanAPIHandler::SetOptions(
+    scoped_refptr<const Extension> extension,
+    const std::string& scanner_handle,
+    const std::vector<api::document_scan::OptionSetting>& options_in,
+    SetOptionsCallback callback) {
+  // Ensure this scanner is allocated to this extension.
+  ExtensionState& state = extension_state_[extension->id()];
+  if (!base::Contains(state.scanner_handles, scanner_handle)) {
+    auto response = crosapi::mojom::SetOptionsResponse::New();
+    response->scanner_handle = scanner_handle;
+    for (const auto& option : options_in) {
+      auto result = crosapi::mojom::SetOptionResult::New();
+      result->name = option.name;
+      result->result = crosapi::mojom::ScannerOperationResult::kInvalid;
+      response->results.emplace_back(std::move(result));
+    }
+    OnSetOptionsResponse(std::move(callback), std::move(response));
+    return;
+  }
+
+  std::vector<crosapi::mojom::OptionSettingPtr> options_out;
+  options_out.reserve(options_in.size());
+  for (const auto& option_in : options_in) {
+    auto& option_out = options_out.emplace_back(
+        crosapi::mojom::OptionSetting::From(option_in));
+    if (option_out->value.is_null()) {
+      // `option_out` has no value, so no re-mapping is needed.
+      continue;
+    }
+
+    // `option_out` has valid field values, but value might not match type.  No
+    // need to check for most mismatches here because they will be rejected by
+    // the backend.
+    //
+    // However, even if the caller passed syntactically valid numeric values in
+    // Javascript, the result that arrives here can contain inconsistencies in
+    // double vs integer.  These can happen due to the inherent JS use of double
+    // for integers as well as quirks of how the auto-generated IDL mapping code
+    // decides to parse arrays for types that accept multiple list types.
+    //
+    // Detect these specific cases and move the value into the expected fixed or
+    // int field before passing along.  All other types are assumed to be
+    // supplied correctly by the caller if they have made it through the JS
+    // bindings.
+    if (option_out->type == crosapi::mojom::OptionType::kFixed) {
+      // kFixed is the name for SANE non-integral numeric values.  It is
+      // represented in Chrome by double.  Handle getting a long or a list of
+      // longs instead of the expected doubles.  This can happen because JS
+      // doesn't really have integers, so the framework maps nn.0 into nn.  If
+      // this has happened, move the int field over into the expected fixed
+      // field.
+      if (option_out->value->is_int_value()) {
+        option_out->value = crosapi::mojom::OptionValue::NewFixedValue(
+            option_out->value->get_int_value());
+      } else if (option_out->value->is_int_list()) {
+        option_out->value = crosapi::mojom::OptionValue::NewFixedList(
+            {option_out->value->get_int_list().begin(),
+             option_out->value->get_int_list().end()});
+      }
+    } else if (option_out->type == crosapi::mojom::OptionType::kInt) {
+      // Handle getting a double or a list of doubles instead of the expected
+      // int(s).  If the values have zero fractional parts, assume they were
+      // really integers that got incorrectly mapped over from JS.  If they have
+      // non-zero fractional parts, the caller really passed a double and the
+      // value should not be re-mapped.
+
+      auto int_from_double = [](double fixed_value) -> std::optional<int32_t> {
+        double int_part = 0.0;
+        if (fixed_value >= std::numeric_limits<int32_t>::min() &&
+            fixed_value <= std::numeric_limits<int32_t>::max() &&
+            std::modf(fixed_value, &int_part) == 0.0) {
+          return base::checked_cast<int32_t>(fixed_value);
+        }
+        return std::nullopt;
+      };
+
+      if (option_out->value->is_fixed_value()) {
+        auto converted = int_from_double(option_out->value->get_fixed_value());
+        if (converted) {
+          option_out->value =
+              crosapi::mojom::OptionValue::NewIntValue(*converted);
+        }
+      } else if (option_out->value->is_fixed_list()) {
+        std::vector<int32_t> ints;
+        const auto& fixed_list = option_out->value->get_fixed_list();
+        ints.reserve(fixed_list.size());
+        for (const double d : fixed_list) {
+          auto converted = int_from_double(d);
+          if (!converted) {
+            break;  // As soon as there's one non-int, no need to continue.
+          }
+          ints.push_back(*converted);
+        }
+        if (ints.size() == fixed_list.size()) {
+          option_out->value = crosapi::mojom::OptionValue::NewIntList(
+              {ints.begin(), ints.end()});
+        }
+      }
+    }
+  }
+  document_scan_->SetOptions(
+      scanner_handle, std::move(options_out),
+      base::BindOnce(&DocumentScanAPIHandler::OnSetOptionsResponse,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void DocumentScanAPIHandler::OnSetOptionsResponse(
+    SetOptionsCallback callback,
+    crosapi::mojom::SetOptionsResponsePtr response) {
+  std::move(callback).Run(
+      response.To<api::document_scan::SetOptionsResponse>());
+}
+
 void DocumentScanAPIHandler::StartScan(
     gfx::NativeWindow native_window,
     scoped_refptr<const Extension> extension,

chrome/browser/extensions/api/document_scan/document_scan_api_handler.h

@@ -49,6 +49,8 @@ class DocumentScanAPIHandler : public BrowserContextKeyedAPI {
       base::OnceCallback<void(api::document_scan::OpenScannerResponse)>;
   using CloseScannerCallback =
       base::OnceCallback<void(api::document_scan::CloseScannerResponse)>;
+  using SetOptionsCallback =
+      base::OnceCallback<void(api::document_scan::SetOptionsResponse)>;
   using StartScanCallback =
       base::OnceCallback<void(api::document_scan::StartScanResponse)>;
   using CancelScanCallback =
@@ -108,6 +110,16 @@ class DocumentScanAPIHandler : public BrowserContextKeyedAPI {
                     const std::string& scanner_handle,
                     CloseScannerCallback callback);
 
+  // Given `scanner_handle` previously returned from `OpenScanner`, sends the
+  // list of new option values in `options` to the backend.  The backend will
+  // attempt to set each option in order, then will respond with a result for
+  // each operation and a new final set of device options.  The full response
+  // will be passed to `callback`.
+  void SetOptions(scoped_refptr<const Extension> extension,
+                  const std::string& scanner_handle,
+                  const std::vector<api::document_scan::OptionSetting>& options,
+                  SetOptionsCallback callback);
+
   // If the user approves, starts a scan using scanner options previously
   // configured via `SetOptions`.  Additionally, `options` are used to specify
   // scanner-framework options.  Explicit approval is obtained through a Chrome
@@ -202,6 +214,8 @@ class DocumentScanAPIHandler : public BrowserContextKeyedAPI {
                              crosapi::mojom::OpenScannerResponsePtr response);
   void OnCloseScannerResponse(CloseScannerCallback callback,
                               crosapi::mojom::CloseScannerResponsePtr response);
+  void OnSetOptionsResponse(SetOptionsCallback callback,
+                            crosapi::mojom::SetOptionsResponsePtr response);
   void OnStartScanResponse(
       std::unique_ptr<StartScanRunner> runner,
       StartScanCallback callback,