Chromium Code Reviews
Help | Chromium Project | Sign in
(163)

Issue 286933006: Implement a JavaScript API for document scanning (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year ago by Paul Stewart
Modified:
6 months, 3 weeks ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, adlr
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Implement a JavaScript API for document scanning Provide an API for callers to list available document scanners, and to acquire a single-page scan from one such device. This currently takes advantage of the lorgnette scanning API in ChromeOS but can be extended to use native document acquisition APIs on other platforms. BUG=375334 R=asargent@chromium.org, mef@chromium.org, asvitkine@chromium.org Committed: https://crrev.com/ea73a785e4d97de63c928f0a574d25beb6236aae Cr-Commit-Position: refs/heads/master@{#302122}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Added entries to histograms.xml, clarified comments #

Total comments: 12

Patch Set 3 : Refactor OS specific changes; remove excessive WARNINGs #

Patch Set 4 : Fixes related to comments in Patch Set 2 #

Patch Set 5 : Drop vestigial file-related code; kOnstify strings #

Total comments: 2

Patch Set 6 : Remove vestigial reference to download folder #

Patch Set 7 : Add scanned image mime type to scanner info #

Patch Set 8 : Added sample application #

Patch Set 9 : API simplified to only "Scan" #

Patch Set 10 : Simplifying ScanOptions #

Patch Set 11 : Implementation not completely fixed yet, but IDL updated. #

Patch Set 12 : IDL indicates multiple images can be returned #

Total comments: 7

Patch Set 13 : API fully up to data (and functional) with respect to API submission, minus image selection API whi… #

Total comments: 14

Patch Set 14 : Split files per feedback; tests in next CL #

Patch Set 15 : Address kalman@'s comments. Still need tests. #

Total comments: 7

Patch Set 16 : Added unit tests, address typos from asargent@; Rebase in next CL #

Patch Set 17 : Rebase to ToT complete #

Total comments: 4

Patch Set 18 : Nits from asargent addressed. One last rebase coming. #

Patch Set 19 : Rebase to ToT -- manual rebase for new API constants #

Patch Set 20 : Copyright the CSS #

Patch Set 21 : Explicit constructor for struct (required by some goma/clang instances) #

Patch Set 22 : Another clang error I couldn't replicate locally #

Patch Set 23 : Fix minor build errors in other platforms #

Patch Set 24 : Similar mods needed for MockLorgnetteManagerClient #

Patch Set 25 : Starting over. This used to be 286933006 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1121 lines, -13 lines) Patch
A chrome/browser/extensions/api/document_scan/document_scan_api.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +55 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +119 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_api_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +123 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_interface.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +64 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_interface.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +26 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_interface_chromeos.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +51 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_interface_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +161 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_interface_chromeos_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +168 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/document_scan_interface_nonchromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +44 lines, -0 lines 0 comments Download
A chrome/browser/extensions/api/document_scan/mock_document_scan_interface.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +35 lines, -0 lines 0 comments Download
A + chrome/browser/extensions/api/document_scan/mock_document_scan_interface.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +8 lines, -3 lines 0 comments Download
M chrome/chrome_browser_extensions.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 3 chunks +6 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/common/extensions/api/_api_features.json View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/common/extensions/api/_permission_features.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +4 lines, -0 lines 0 comments Download
A chrome/common/extensions/api/document_scan.idl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +36 lines, -0 lines 0 comments Download
M chrome/common/extensions/api/schemas.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -0 lines 0 comments Download
A chrome/common/extensions/docs/examples/api/document_scan/README.md View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +10 lines, -0 lines 0 comments Download
A + chrome/common/extensions/docs/examples/api/document_scan/background.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +6 lines, -4 lines 0 comments Download
A chrome/common/extensions/docs/examples/api/document_scan/manifest.json View 1 2 3 4 5 6 7 9 10 1 chunk +13 lines, -0 lines 0 comments Download
A chrome/common/extensions/docs/examples/api/document_scan/scan.css View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +37 lines, -0 lines 0 comments Download
A chrome/common/extensions/docs/examples/api/document_scan/scan.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +18 lines, -0 lines 0 comments Download
A chrome/common/extensions/docs/examples/api/document_scan/scan.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +69 lines, -0 lines 0 comments Download
M chrome/common/extensions/permissions/chrome_api_permissions.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +3 lines, -0 lines 0 comments Download
M chromeos/chromeos.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +2 lines, -0 lines 0 comments Download
A chromeos/dbus/mock_lorgnette_manager_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +29 lines, -0 lines 0 comments Download
A + chromeos/dbus/mock_lorgnette_manager_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +6 lines, -4 lines 0 comments Download
M extensions/browser/extension_function_histogram_value.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/permissions/api_permission.h View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +3 lines, -2 lines 0 comments Download
M extensions/common/permissions/permission_message.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -0 lines 0 comments Download
M extensions/extensions_strings.grd View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +5 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +7 lines, -0 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 55 (14 generated)
Paul Stewart
1 year ago (2014-05-20 18:39:47 UTC) #1
Alexei Svitkine
https://codereview.chromium.org/286933006/diff/1/extensions/browser/extension_function_histogram_value.h File extensions/browser/extension_function_histogram_value.h (right): https://codereview.chromium.org/286933006/diff/1/extensions/browser/extension_function_histogram_value.h#newcode837 extensions/browser/extension_function_histogram_value.h:837: DOCUMENT_SCAN_SCAN, Please also update histograms.xml. https://codereview.chromium.org/286933006/diff/1/extensions/common/permissions/permission_message.h File extensions/common/permissions/permission_message.h (right): ...
1 year ago (2014-05-20 20:50:41 UTC) #2
Paul Stewart
Thanks for the review! PTAL. https://codereview.chromium.org/286933006/diff/1/extensions/browser/extension_function_histogram_value.h File extensions/browser/extension_function_histogram_value.h (right): https://codereview.chromium.org/286933006/diff/1/extensions/browser/extension_function_histogram_value.h#newcode837 extensions/browser/extension_function_histogram_value.h:837: DOCUMENT_SCAN_SCAN, On 2014/05/20 20:50:42, ...
1 year ago (2014-05-21 01:09:29 UTC) #3
Alexei Svitkine
metrics LGTM, thanks!
1 year ago (2014-05-21 06:30:33 UTC) #4
Paul Stewart
Thanks. I have more work to do, and just noticed that I still have logging ...
1 year ago (2014-05-21 15:55:19 UTC) #5
Antony Sargent
Cool API! Have you done the API proposal process yet? (http://dev.chromium.org/developers/design-documents/extensions/proposed-changes/apis-under-development) https://codereview.chromium.org/286933006/diff/20001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): ...
1 year ago (2014-05-21 18:06:24 UTC) #6
Paul Stewart
Thank you for all your comments Antony! I believe I've addressed your comments in a ...
1 year ago (2014-05-22 18:37:50 UTC) #7
mef
https://codereview.chromium.org/286933006/diff/80001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): https://codereview.chromium.org/286933006/diff/80001/chrome/common/extensions/api/document_scan.idl#newcode40 chrome/common/extensions/api/document_scan.idl:40: // is also saved to the user's download folder. ...
1 year ago (2014-05-23 18:38:17 UTC) #8
Paul Stewart
https://codereview.chromium.org/286933006/diff/80001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): https://codereview.chromium.org/286933006/diff/80001/chrome/common/extensions/api/document_scan.idl#newcode40 chrome/common/extensions/api/document_scan.idl:40: // is also saved to the user's download folder. ...
1 year ago (2014-05-23 19:45:36 UTC) #9
Antony Sargent
FYI in case you were wondering about the radio silence from me, I'm just waiting ...
12 months ago (2014-05-29 17:45:09 UTC) #10
Paul Stewart
PTAL. I believe the API proposal has gotten LGTMs. The code and example extension has ...
7 months, 2 weeks ago (2014-10-12 14:38:20 UTC) #11
mef
TBH I'm not sure why I'm a reviewer on this as I don't know anything ...
7 months, 1 week ago (2014-10-13 15:59:54 UTC) #12
Paul Stewart
https://codereview.chromium.org/286933006/diff/230001/chrome/browser/extensions/api/document_scan/document_scan_api.cc File chrome/browser/extensions/api/document_scan/document_scan_api.cc (right): https://codereview.chromium.org/286933006/diff/230001/chrome/browser/extensions/api/document_scan/document_scan_api.cc#newcode41 chrome/browser/extensions/api/document_scan/document_scan_api.cc:41: #if defined(OS_CHROMEOS) On 2014/10/13 15:59:54, mef wrote: > Suggest: ...
7 months, 1 week ago (2014-10-14 19:28:31 UTC) #13
kalman
API nits (referred to from API proposal). https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl#newcode10 chrome/common/extensions/api/document_scan.idl:10: DOMString[]? accepted_mime_types; ...
7 months ago (2014-10-20 22:36:13 UTC) #15
Paul Stewart
https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl#newcode10 chrome/common/extensions/api/document_scan.idl:10: DOMString[]? accepted_mime_types; On 2014/10/20 22:36:13, kalman wrote: > "accepted" ...
7 months ago (2014-10-21 02:42:17 UTC) #16
Paul Stewart
7 months ago (2014-10-21 02:42:18 UTC) #17
kalman
https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl#newcode20 chrome/common/extensions/api/document_scan.idl:20: callback ScanCallback = void (DOMString[] data_urls, DOMString mime_type); On ...
7 months ago (2014-10-21 17:30:01 UTC) #18
Paul Stewart
On 2014/10/21 17:30:01, kalman wrote: > https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl > File chrome/common/extensions/api/document_scan.idl (right): > > https://codereview.chromium.org/286933006/diff/210001/chrome/common/extensions/api/document_scan.idl#newcode20 > ...
7 months ago (2014-10-21 18:14:01 UTC) #19
kalman
On 2014/10/21 18:14:01, Paul Stewart wrote: > On 2014/10/21 17:30:01, kalman wrote: > > > ...
7 months ago (2014-10-21 18:19:20 UTC) #20
Antony Sargent
https://codereview.chromium.org/286933006/diff/420001/chrome/browser/extensions/api/document_scan/document_scan_api.cc File chrome/browser/extensions/api/document_scan/document_scan_api.cc (right): https://codereview.chromium.org/286933006/diff/420001/chrome/browser/extensions/api/document_scan/document_scan_api.cc#newcode95 chrome/browser/extensions/api/document_scan/document_scan_api.cc:95: // TODO(pstew): Display receied scan in the UI and ...
7 months ago (2014-10-23 22:15:57 UTC) #21
Paul Stewart
https://codereview.chromium.org/286933006/diff/420001/chrome/common/extensions/api/document_scan.idl File chrome/common/extensions/api/document_scan.idl (right): https://codereview.chromium.org/286933006/diff/420001/chrome/common/extensions/api/document_scan.idl#newcode13 chrome/common/extensions/api/document_scan.idl:13: long? maxImages; On 2014/10/23 22:15:57, asargent wrote: > How ...
7 months ago (2014-10-23 23:06:58 UTC) #22
Paul Stewart
Added stevenjb@ to the review due to added mock_lorgnette_manager_client.h
7 months ago (2014-10-23 23:07:55 UTC) #24
stevenjb
chromeos/dbus/mock_lorgnette_manager_client.h lgtm
7 months ago (2014-10-23 23:09:47 UTC) #25
Paul Stewart
PTAL. Tests are now in place.
7 months ago (2014-10-24 04:05:09 UTC) #26
Paul Stewart
Gentle ping, mostly because I need to rebase quite often to account for other APIs ...
6 months, 4 weeks ago (2014-10-29 00:42:48 UTC) #27
Antony Sargent
lgtm https://codereview.chromium.org/286933006/diff/460001/chrome/browser/extensions/api/document_scan/document_scan_api.cc File chrome/browser/extensions/api/document_scan/document_scan_api.cc (right): https://codereview.chromium.org/286933006/diff/460001/chrome/browser/extensions/api/document_scan/document_scan_api.cc#newcode14 chrome/browser/extensions/api/document_scan/document_scan_api.cc:14: using std::vector; nit: having using statements for std:: ...
6 months, 3 weeks ago (2014-10-29 23:05:21 UTC) #29
Paul Stewart
Okay. Done with these comments, and rebased to ToT. https://codereview.chromium.org/286933006/diff/460001/chrome/browser/extensions/api/document_scan/document_scan_api.cc File chrome/browser/extensions/api/document_scan/document_scan_api.cc (right): https://codereview.chromium.org/286933006/diff/460001/chrome/browser/extensions/api/document_scan/document_scan_api.cc#newcode14 chrome/browser/extensions/api/document_scan/document_scan_api.cc:14: ...
6 months, 3 weeks ago (2014-10-30 06:31:51 UTC) #30
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/286933006/500001
6 months, 3 weeks ago (2014-10-30 06:32:45 UTC) #32
I haz the power - commit-bot
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/21171)
6 months, 3 weeks ago (2014-10-30 06:36:39 UTC) #34
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/286933006/520001
6 months, 3 weeks ago (2014-10-30 06:54:01 UTC) #36
I haz the power - commit-bot
Try jobs failed on following builders: linux_chromium_gn_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_dbg/builds/12993)
6 months, 3 weeks ago (2014-10-30 07:01:52 UTC) #38
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/286933006/540001
6 months, 3 weeks ago (2014-10-30 14:44:10 UTC) #40
I haz the power - commit-bot
Try jobs failed on following builders: linux_chromium_gn_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_dbg/builds/13063) linux_chromium_gn_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_rel/builds/29803)
6 months, 3 weeks ago (2014-10-30 14:51:33 UTC) #42
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/286933006/560001
6 months, 3 weeks ago (2014-10-30 15:55:49 UTC) #44
I haz the power - commit-bot
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/1450)
6 months, 3 weeks ago (2014-10-30 16:25:59 UTC) #46
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/286933006/580001
6 months, 3 weeks ago (2014-10-30 16:40:51 UTC) #48
I haz the power - commit-bot
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel/builds/6980)
6 months, 3 weeks ago (2014-10-30 17:25:20 UTC) #50
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/286933006/600001
6 months, 3 weeks ago (2014-10-30 17:34:54 UTC) #52
I haz the power - commit-bot
Committed patchset #24 (id:600001)
6 months, 3 weeks ago (2014-10-30 18:55:47 UTC) #53
I haz the power - commit-bot
Patchset 24 (id:??) landed as https://crrev.com/ea73a785e4d97de63c928f0a574d25beb6236aae Cr-Commit-Position: refs/heads/master@{#302122}
6 months, 3 weeks ago (2014-10-30 18:56:35 UTC) #54
stevenjb
6 months, 3 weeks ago (2014-11-03 18:16:10 UTC) #55
Message was sent while issue was closed.
src/chromeos/ lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be