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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 months, 2 weeks ago by Paul Stewart
Modified:
4 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
10 months, 2 weeks 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): ...
10 months, 2 weeks 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, ...
10 months, 2 weeks ago (2014-05-21 01:09:29 UTC) #3
Alexei Svitkine
metrics LGTM, thanks!
10 months, 1 week 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 ...
10 months, 1 week 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): ...
10 months, 1 week 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 ...
10 months, 1 week 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. ...
10 months, 1 week 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. ...
10 months, 1 week 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 ...
10 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 ...
5 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 ...
5 months, 2 weeks 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: ...
5 months, 2 weeks 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; ...
5 months, 1 week 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" ...
5 months, 1 week ago (2014-10-21 02:42:17 UTC) #16
Paul Stewart
5 months, 1 week 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 ...
5 months, 1 week 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 > ...
5 months, 1 week 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: > > > ...
5 months, 1 week 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 ...
5 months, 1 week 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 ...
5 months, 1 week ago (2014-10-23 23:06:58 UTC) #22
Paul Stewart
Added stevenjb@ to the review due to added mock_lorgnette_manager_client.h
5 months, 1 week ago (2014-10-23 23:07:55 UTC) #24
stevenjb
chromeos/dbus/mock_lorgnette_manager_client.h lgtm
5 months, 1 week ago (2014-10-23 23:09:47 UTC) #25
Paul Stewart
PTAL. Tests are now in place.
5 months, 1 week 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 ...
5 months 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:: ...
5 months 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: ...
5 months 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
5 months 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)
5 months 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
5 months 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)
5 months 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
5 months 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)
5 months 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
5 months 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)
5 months 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
5 months 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)
5 months 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
5 months ago (2014-10-30 17:34:54 UTC) #52
I haz the power (commit-bot)
Committed patchset #24 (id:600001)
5 months 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}
5 months ago (2014-10-30 18:56:35 UTC) #54
stevenjb
4 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 cf4c24d