From 8f1e0b802cdd948d2eb40652eb34e1dbbcdbfbe9 Mon Sep 17 00:00:00 2001 From: Benjamin Gordon Date: Thu, 14 Dec 2023 23:47:17 +0000 Subject: [PATCH] documentScan: Add setOptions function setOptions takes a scanner handle returned from openScanner and sets zero or more supplied options to new values. The response includes the result of setting each option and an updated set of option values in the same format as the original response from openScanner. The real work is done in DocumentScanAPIHandler::SetOptions. This CL adds the JS function entry points and the matching apitests. Bug: b:297435721 Test: Scan from test extension with non-default options on a Chromebook Change-Id: Ib5dab0e804c3b61823cbf7cf0c69b148203a6e8b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5096610 Reviewed-by: Devlin Cronin Commit-Queue: Benjamin Gordon Cr-Commit-Position: refs/heads/main@{#1237823} --- .../api/document_scan/document_scan_api.cc | 23 +++ .../api/document_scan/document_scan_api.h | 19 ++ .../common/extensions/api/_api_features.json | 3 + .../common/extensions/api/document_scan.idl | 14 ++ .../document_scan/load_permissions.js | 1 + .../api_test/document_scan/perform_scan.js | 177 ++++++++++++++++++ .../api_test/document_scan/scan_utils.js | 7 + .../extension_function_histogram_value.h | 1 + tools/metrics/histograms/enums.xml | 1 + 9 files changed, 246 insertions(+) diff --git a/chrome/browser/extensions/api/document_scan/document_scan_api.cc b/chrome/browser/extensions/api/document_scan/document_scan_api.cc index b1cc66add50819..825051267086ae 100644 --- a/chrome/browser/extensions/api/document_scan/document_scan_api.cc +++ b/chrome/browser/extensions/api/document_scan/document_scan_api.cc @@ -141,6 +141,29 @@ void DocumentScanCloseScannerFunction::OnResponseReceived( api::document_scan::CloseScanner::Results::Create(response))); } +DocumentScanSetOptionsFunction::DocumentScanSetOptionsFunction() = default; +DocumentScanSetOptionsFunction::~DocumentScanSetOptionsFunction() = default; + +ExtensionFunction::ResponseAction DocumentScanSetOptionsFunction::Run() { + auto params = api::document_scan::SetOptions::Params::Create(args()); + EXTENSION_FUNCTION_VALIDATE(params); + + DocumentScanAPIHandler::Get(browser_context()) + ->SetOptions( + extension_, std::move(params->scanner_handle), + std::move(params->options), + base::BindOnce(&DocumentScanSetOptionsFunction::OnResponseReceived, + this)); + + return did_respond() ? AlreadyResponded() : RespondLater(); +} + +void DocumentScanSetOptionsFunction::OnResponseReceived( + api::document_scan::SetOptionsResponse response) { + Respond( + ArgumentList(api::document_scan::SetOptions::Results::Create(response))); +} + DocumentScanStartScanFunction::DocumentScanStartScanFunction() = default; DocumentScanStartScanFunction::~DocumentScanStartScanFunction() = default; diff --git a/chrome/browser/extensions/api/document_scan/document_scan_api.h b/chrome/browser/extensions/api/document_scan/document_scan_api.h index 968803e45f74e8..f8b8411bba53ea 100644 --- a/chrome/browser/extensions/api/document_scan/document_scan_api.h +++ b/chrome/browser/extensions/api/document_scan/document_scan_api.h @@ -92,6 +92,25 @@ class DocumentScanCloseScannerFunction : public ExtensionFunction { DOCUMENTSCAN_CLOSESCANNER) }; +class DocumentScanSetOptionsFunction : public ExtensionFunction { + public: + DocumentScanSetOptionsFunction(); + DocumentScanSetOptionsFunction(const DocumentScanSetOptionsFunction&) = + delete; + DocumentScanSetOptionsFunction& operator=( + const DocumentScanSetOptionsFunction&) = delete; + + protected: + ~DocumentScanSetOptionsFunction() override; + + // ExtensionFunction: + ResponseAction Run() override; + + private: + void OnResponseReceived(api::document_scan::SetOptionsResponse response); + DECLARE_EXTENSION_FUNCTION("documentScan.setOptions", DOCUMENTSCAN_SETOPTIONS) +}; + class DocumentScanStartScanFunction : public ExtensionFunction { public: DocumentScanStartScanFunction(); diff --git a/chrome/common/extensions/api/_api_features.json b/chrome/common/extensions/api/_api_features.json index 7e005a49399717..5c34186af2f805 100644 --- a/chrome/common/extensions/api/_api_features.json +++ b/chrome/common/extensions/api/_api_features.json @@ -368,6 +368,9 @@ "documentScan.closeScanner": { "channel": "dev" }, + "documentScan.setOptions": { + "channel": "dev" + }, "documentScan.startScan": { "channel": "dev" }, diff --git a/chrome/common/extensions/api/document_scan.idl b/chrome/common/extensions/api/document_scan.idl index 873b46ec107d56..ac49bc77722160 100644 --- a/chrome/common/extensions/api/document_scan.idl +++ b/chrome/common/extensions/api/document_scan.idl @@ -497,6 +497,20 @@ namespace documentScan { [nodoc, supportsPromises] static void closeScanner( DOMString scannerHandle, CloseScannerCallback callback); + // Sends the list of new option values in options as a bundle + // to be set on scannerHandle. Each option will be set by the + // backend the order specified. Returns a backend response indicating the + // result of each option setting and a new set of final option values after + // all options have been updated. + // |scannerHandle| : Open scanner handle previously returned from + // openScanner. + // |options| : A list of OptionSettings that will be applied to + // scannerHandle. + // |callback| : Called with the result. + [nodoc, supportsPromises] static void setOptions( + DOMString scannerHandle, OptionSetting[] options, + SetOptionsCallback callback); + // Starts a scan using a previously opened scanner handle. A response // indicating the outcome will be sent to the callback. If successful, the // response will include a job handle that can be used in subsequent calls diff --git a/chrome/test/data/extensions/api_test/document_scan/load_permissions.js b/chrome/test/data/extensions/api_test/document_scan/load_permissions.js index 0dfc0348f99d26..8475a9aef98398 100644 --- a/chrome/test/data/extensions/api_test/document_scan/load_permissions.js +++ b/chrome/test/data/extensions/api_test/document_scan/load_permissions.js @@ -9,6 +9,7 @@ chrome.test.runTests([ chrome.test.assertTrue(!!chrome.documentScan.getScannerList); chrome.test.assertTrue(!!chrome.documentScan.openScanner); chrome.test.assertTrue(!!chrome.documentScan.closeScanner); + chrome.test.assertTrue(!!chrome.documentScan.setOptions); chrome.test.assertTrue(!!chrome.documentScan.startScan); chrome.test.assertTrue(!!chrome.documentScan.cancelScan); chrome.test.assertTrue(!!chrome.documentScan.readScanData); diff --git a/chrome/test/data/extensions/api_test/document_scan/perform_scan.js b/chrome/test/data/extensions/api_test/document_scan/perform_scan.js index d0b2b3f1d442ac..2e7065179bd0d3 100644 --- a/chrome/test/data/extensions/api_test/document_scan/perform_scan.js +++ b/chrome/test/data/extensions/api_test/document_scan/perform_scan.js @@ -218,6 +218,183 @@ chrome.test.runTests([ chrome.test.assertEq(null, readResponse2.data); chrome.test.assertEq(null, readResponse2.estimatedCompletion); + chrome.test.succeed(); + }, + + async function setOptionsBeforeOpenFails() { + const response = await setOptions('invalid-handle', [ + {name: 'option', type: OptionType.INT}]); + chrome.test.assertEq('invalid-handle', response.scannerHandle); + chrome.test.assertEq(1, response.results.length); + chrome.test.assertEq(OperationResult.INVALID, response.results[0].result); + chrome.test.assertEq('option', response.results[0].name); + chrome.test.assertEq(null, response.options); + chrome.test.succeed(); + }, + + async function setOptionsRequiresMatchingTypes_Fixed() { + // Fixed options can be set from int or double because JS doesn't have a + // clear distinction between these. + const options = [ + {name: 'fixed1', type: OptionType.FIXED, value: 42}, // OK, mapped. + {name: 'fixed2', type: OptionType.FIXED, value: 42.0}, // OK, mapped. + {name: 'fixed3', type: OptionType.FIXED, value: 42.5}, // OK. + {name: 'fixed4', type: OptionType.FIXED, value: '1.0'}, // Wrong type. + {name: 'fixed5', type: OptionType.FIXED, value: [42, 43]}, // OK, mapped. + {name: 'fixed6', type: OptionType.FIXED, + value: [42.0, 43.0]}, // OK, mapped. + {name: 'fixed7', type: OptionType.FIXED, value: [42.5, 43.5]} // OK. + ]; + + const scannerHandle = await getScannerHandle(); + chrome.test.assertNe(null, scannerHandle); + + const response = await setOptions(scannerHandle, options); + chrome.test.assertEq(scannerHandle, response.scannerHandle); + chrome.test.assertEq(options.length, response.results.length); + // Match each result individually instead of one big array to make it easier + // to tell where any failures occur. + chrome.test.assertEq( + {name: 'fixed1', result: OperationResult.SUCCESS}, response.results[0]); + chrome.test.assertEq( + {name: 'fixed2', result: OperationResult.SUCCESS}, response.results[1]); + chrome.test.assertEq( + {name: 'fixed3', result: OperationResult.SUCCESS}, response.results[2]); + chrome.test.assertEq( + {name: 'fixed4', result: OperationResult.WRONG_TYPE}, + response.results[3]); + chrome.test.assertEq( + {name: 'fixed5', result: OperationResult.SUCCESS}, response.results[4]); + chrome.test.assertEq( + {name: 'fixed6', result: OperationResult.SUCCESS}, response.results[5]); + chrome.test.assertEq( + {name: 'fixed7', result: OperationResult.SUCCESS}, response.results[6]); + + chrome.test.assertNe(null, response.options); + chrome.test.succeed(); + }, + + async function setOptionsRequiresMatchingTypes_Int() { + // Int options can be set from int values or from double values with a zero + // fractional part because JS doesn't have a clear distinction between + // these. + const options = [ + {name: 'int1', type: OptionType.INT, value: 42}, // OK. + {name: 'int2', type: OptionType.INT, value: 42.0}, // OK, mapped. + {name: 'int3', type: OptionType.INT, value: 42.5}, // Wrong type. + {name: 'int4', type: OptionType.INT, value: '1.0'}, // Wrong type. + {name: 'int5', type: OptionType.INT, value: [42, 42]}, // OK. + {name: 'int6', type: OptionType.INT, value: [42.0, 42.0]}, // OK, mapped. + {name: 'int7', type: OptionType.INT, value: [42.5]}, // Wrong type. + {name: 'int8', type: OptionType.INT, value: 1e300}, // Wrong type. + {name: 'int9', type: OptionType.INT, value: -1e300}, // Wrong type. + {name: 'int10', type: OptionType.INT, value: [1e300]}, // Wrong type. + {name: 'int11', type: OptionType.INT, value: [-1e300]} // Wrong type. + ]; + + const scannerHandle = await getScannerHandle(); + chrome.test.assertNe(null, scannerHandle); + + const response = await setOptions(scannerHandle, options); + chrome.test.assertEq(scannerHandle, response.scannerHandle); + chrome.test.assertEq(options.length, response.results.length); + // Match each result individually instead of one big array to make it easier + // to tell where any failures occur. + chrome.test.assertEq( + {name: 'int1', result: OperationResult.SUCCESS}, response.results[0]); + chrome.test.assertEq( + {name: 'int2', result: OperationResult.SUCCESS}, response.results[1]); + chrome.test.assertEq( + {name: 'int3', result: OperationResult.WRONG_TYPE}, response.results[2]); + chrome.test.assertEq( + {name: 'int4', result: OperationResult.WRONG_TYPE}, response.results[3]); + chrome.test.assertEq( + {name: 'int5', result: OperationResult.SUCCESS}, response.results[4]); + chrome.test.assertEq( + {name: 'int6', result: OperationResult.SUCCESS}, response.results[5]); + chrome.test.assertEq( + {name: 'int7', result: OperationResult.WRONG_TYPE}, response.results[6]); + chrome.test.assertEq( + {name: 'int8', result: OperationResult.WRONG_TYPE}, response.results[7]); + chrome.test.assertEq( + {name: 'int9', result: OperationResult.WRONG_TYPE}, response.results[8]); + chrome.test.assertEq( + {name: 'int10', result: OperationResult.WRONG_TYPE}, + response.results[9]); + chrome.test.assertEq( + {name: 'int11', result: OperationResult.WRONG_TYPE}, + response.results[10]); + + chrome.test.assertNe(null, response.options); + chrome.test.succeed(); + }, + + async function setOptionsRequiresMatchingTypes_Bool() { + // Bool options can only be set from a bool. + const options = [ + {name: 'bool1', type: OptionType.BOOL, value: true}, // OK. + {name: 'bool2', type: OptionType.BOOL, value: 1}, // Wrong type. + {name: 'bool3', type: OptionType.BOOL, value: 'true'}, // Wrong type. + {name: 'bool4', type: OptionType.BOOL, value: [1]} // Wrong type. + ]; + + const scannerHandle = await getScannerHandle(); + chrome.test.assertNe(null, scannerHandle); + + const response = await setOptions(scannerHandle, options); + chrome.test.assertEq(scannerHandle, response.scannerHandle); + chrome.test.assertEq(options.length, response.results.length); + // Match each result individually instead of one big array to make it easier + // to tell where any failures occur. + chrome.test.assertEq( + {name: 'bool1', result: OperationResult.SUCCESS}, response.results[0]); + chrome.test.assertEq( + {name: 'bool2', result: OperationResult.WRONG_TYPE}, + response.results[1]); + chrome.test.assertEq( + {name: 'bool3', result: OperationResult.WRONG_TYPE}, + response.results[2]); + chrome.test.assertEq( + {name: 'bool4', result: OperationResult.WRONG_TYPE}, + response.results[3]); + + chrome.test.assertNe(null, response.options); + chrome.test.succeed(); + }, + + async function setOptionsRequiresMatchingTypes_String() { + // String options can only be set from a string. + const options = [ + {name: 'string1', type: OptionType.STRING, value: 's'}, // OK. + {name: 'string2', type: OptionType.STRING, value: ''}, // OK. + {name: 'string3', type: OptionType.STRING, value: 1}, // Wrong type. + {name: 'string4', type: OptionType.STRING, value: [1]}, // Wrong type. + {name: 'string5', type: OptionType.STRING, value: true}, // Wrong type. + ]; + + const scannerHandle = await getScannerHandle(); + chrome.test.assertNe(null, scannerHandle); + + const response = await setOptions(scannerHandle, options); + chrome.test.assertEq(scannerHandle, response.scannerHandle); + chrome.test.assertEq(options.length, response.results.length); + // Match each result individually instead of one big array to make it easier + // to tell where any failures occur. + chrome.test.assertEq( + {name: 'string1', result: OperationResult.SUCCESS}, response.results[0]); + chrome.test.assertEq( + {name: 'string2', result: OperationResult.SUCCESS}, response.results[1]); + chrome.test.assertEq( + {name: 'string3', result: OperationResult.WRONG_TYPE}, + response.results[2]); + chrome.test.assertEq( + {name: 'string4', result: OperationResult.WRONG_TYPE}, + response.results[3]); + chrome.test.assertEq( + {name: 'string5', result: OperationResult.WRONG_TYPE}, + response.results[4]); + + chrome.test.assertNe(null, response.options); chrome.test.succeed(); } ]); diff --git a/chrome/test/data/extensions/api_test/document_scan/scan_utils.js b/chrome/test/data/extensions/api_test/document_scan/scan_utils.js index a5a1c9f9aac4c8..652c503ee31c48 100644 --- a/chrome/test/data/extensions/api_test/document_scan/scan_utils.js +++ b/chrome/test/data/extensions/api_test/document_scan/scan_utils.js @@ -3,6 +3,7 @@ // found in the LICENSE file. OperationResult = chrome.documentScan.OperationResult; +OptionType = chrome.documentScan.OptionType; async function getScannerList(filter) { return new Promise(resolve => { @@ -67,3 +68,9 @@ async function readScanData(jobHandle) { chrome.documentScan.readScanData(jobHandle, resolve); }); } + +async function setOptions(scannerHandle, options) { + return new Promise(resolve => { + chrome.documentScan.setOptions(scannerHandle, options, resolve); + }); +} diff --git a/extensions/browser/extension_function_histogram_value.h b/extensions/browser/extension_function_histogram_value.h index daa9e07094544d..9b8500c5c42963 100644 --- a/extensions/browser/extension_function_histogram_value.h +++ b/extensions/browser/extension_function_histogram_value.h @@ -1919,6 +1919,7 @@ enum HistogramValue { DOCUMENTSCAN_STARTSCAN = 1857, DOCUMENTSCAN_CANCELSCAN = 1858, DOCUMENTSCAN_READSCANDATA = 1859, + DOCUMENTSCAN_SETOPTIONS = 1860, // Last entry: Add new entries above, then run: // tools/metrics/histograms/update_extension_histograms.py ENUM_BOUNDARY diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index fecd2f42664a36..5ba1a74c77dac4 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -16517,6 +16517,7 @@ Called by update_extension_histograms.py.--> +