Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(359)

Issue 973993003: Instead of ArrayBuffer, pass blob with printerProvider.onPrintRequested (Closed)

Created:
5 years, 9 months ago by tbarzic
Modified:
5 years, 9 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, tzik, nhiroki, darin-cc_chromium.org, chromium-apps-reviews_chromium.org, kinuko+fileapi
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Instead of ArrayBuffer, pass blob with printerProvider.onPrintRequested This will allow us to pass larger amout of data to the extension. With array buffer sending more than ~60MB documents would crash the renderer. For data converted to PWG raster, the data is written to a temp file during conversion. To avoid reading the data back into memory, use file backed blob to represent this data. The blob passed to the extension is created in the API custom bindings using newly added chrome.printerProviderInternal.getPrintData function, which creates the blob and returns data needed by the renderer to take the ownership of the blob. For creating file backed blob in printer_provider_internal_api.cc, this add BrowserContext::CreateFileBackedBlob (analogous to existing CreateMemoryBackedBlob) method, which can be used from extensions/ BUG=461115 TEST=Manually verified that a ~150MB PWG raster data can be passed to a test preintrProvider extension. extensions_browsertests --gtest_filter=PrinterProviderAPI* unit_tests --gtest_filter=ExtensionPrinterHandler* Committed: https://crrev.com/db71268248fb1221faa828cbc8dcba7fc2218c4a Cr-Commit-Position: refs/heads/master@{#319416}

Patch Set 1 #

Patch Set 2 : . #

Total comments: 7

Patch Set 3 : kalman feedback #

Total comments: 2

Patch Set 4 : vitaly's feedback #

Patch Set 5 : rebase & add ext fun histogram #

Unified diffs Side-by-side diffs Delta from patch set Stats (+566 lines, -152 lines) Patch
M chrome/browser/ui/webui/print_preview/extension_printer_handler.h View 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/ui/webui/print_preview/extension_printer_handler.cc View 1 6 chunks +30 lines, -34 lines 0 comments Download
M chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc View 1 2 3 4 13 chunks +48 lines, -38 lines 0 comments Download
M content/browser/browser_context.cc View 2 chunks +21 lines, -0 lines 0 comments Download
M content/browser/fileapi/chrome_blob_storage_context.h View 2 chunks +12 lines, -0 lines 0 comments Download
M content/browser/fileapi/chrome_blob_storage_context.cc View 1 chunk +21 lines, -0 lines 0 comments Download
M content/public/browser/browser_context.h View 2 chunks +9 lines, -0 lines 0 comments Download
M extensions/browser/api/printer_provider/printer_provider_api.h View 1 3 chunks +7 lines, -5 lines 0 comments Download
M extensions/browser/api/printer_provider/printer_provider_api.cc View 1 2 3 4 7 chunks +40 lines, -19 lines 0 comments Download
M extensions/browser/api/printer_provider/printer_provider_apitest.cc View 1 2 3 4 8 chunks +118 lines, -15 lines 0 comments Download
M extensions/browser/api/printer_provider/printer_provider_print_job.h View 3 chunks +14 lines, -4 lines 0 comments Download
M extensions/browser/api/printer_provider_internal/printer_provider_internal_api.h View 3 chunks +23 lines, -0 lines 0 comments Download
M extensions/browser/api/printer_provider_internal/printer_provider_internal_api.cc View 2 chunks +92 lines, -0 lines 0 comments Download
M extensions/browser/extension_function_histogram_value.h View 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/api/printer_provider.idl View 1 chunk +3 lines, -2 lines 0 comments Download
M extensions/common/api/printer_provider_internal.idl View 2 chunks +24 lines, -0 lines 0 comments Download
M extensions/renderer/resources/printer_provider_custom_bindings.js View 1 2 3 chunks +41 lines, -5 lines 0 comments Download
M extensions/test/data/api_test/printer_provider/request_print/test.js View 1 2 2 chunks +56 lines, -25 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 17 (4 generated)
tbarzic
an you please take a look? kalman: extensions/common and extensions/renderer michaeln: content/browser/fileapi (and printer_provider_internal_api.cc for ...
5 years, 9 months ago (2015-03-04 21:37:39 UTC) #2
not at google - send to devlin
https://codereview.chromium.org/973993003/diff/20001/extensions/common/api/printer_provider_internal.idl File extensions/common/api/printer_provider_internal.idl (right): https://codereview.chromium.org/973993003/diff/20001/extensions/common/api/printer_provider_internal.idl#newcode28 extensions/common/api/printer_provider_internal.idl:28: long size; Wow, big print job :-) https://codereview.chromium.org/973993003/diff/20001/extensions/renderer/resources/printer_provider_custom_bindings.js File ...
5 years, 9 months ago (2015-03-04 21:54:50 UTC) #3
not at google - send to devlin
lgtm https://codereview.chromium.org/973993003/diff/20001/extensions/test/data/api_test/printer_provider/request_print/test.js File extensions/test/data/api_test/printer_provider/request_print/test.js (right): https://codereview.chromium.org/973993003/diff/20001/extensions/test/data/api_test/printer_provider/request_print/test.js#newcode46 extensions/test/data/api_test/printer_provider/request_print/test.js:46: } else if (test == 'OK') { Looks ...
5 years, 9 months ago (2015-03-04 21:54:50 UTC) #4
tbarzic
https://codereview.chromium.org/973993003/diff/20001/extensions/renderer/resources/printer_provider_custom_bindings.js File extensions/renderer/resources/printer_provider_custom_bindings.js (right): https://codereview.chromium.org/973993003/diff/20001/extensions/renderer/resources/printer_provider_custom_bindings.js#newcode83 extensions/renderer/resources/printer_provider_custom_bindings.js:83: resultReporter(args[0], null); On 2015/03/04 21:54:49, kalman wrote: > document ...
5 years, 9 months ago (2015-03-04 23:24:34 UTC) #5
jam
On 2015/03/04 21:37:39, tbarzic wrote: > an you please take a look? > > kalman: ...
5 years, 9 months ago (2015-03-05 18:26:33 UTC) #6
Vitaly Buka (NO REVIEWS)
lgtm https://codereview.chromium.org/973993003/diff/40001/extensions/browser/api/printer_provider/printer_provider_api.cc File extensions/browser/api/printer_provider/printer_provider_api.cc (right): https://codereview.chromium.org/973993003/diff/40001/extensions/browser/api/printer_provider/printer_provider_api.cc#newcode92 extensions/browser/api/printer_provider/printer_provider_api.cc:92: struct PrintRequest { nested in PendingPrintRequests?
5 years, 9 months ago (2015-03-05 22:10:01 UTC) #7
tbarzic
https://codereview.chromium.org/973993003/diff/40001/extensions/browser/api/printer_provider/printer_provider_api.cc File extensions/browser/api/printer_provider/printer_provider_api.cc (right): https://codereview.chromium.org/973993003/diff/40001/extensions/browser/api/printer_provider/printer_provider_api.cc#newcode92 extensions/browser/api/printer_provider/printer_provider_api.cc:92: struct PrintRequest { On 2015/03/05 22:10:01, Vitaly Buka wrote: ...
5 years, 9 months ago (2015-03-05 23:57:36 UTC) #8
michaeln
content/browser/fileapi and printer_provider_internal_api.cc lgtm 2
5 years, 9 months ago (2015-03-06 00:20:51 UTC) #9
tbarzic
+isherman for histograms
5 years, 9 months ago (2015-03-06 03:18:25 UTC) #11
Ilya Sherman
histograms lgtm
5 years, 9 months ago (2015-03-06 03:47:49 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/973993003/80001
5 years, 9 months ago (2015-03-06 03:52:26 UTC) #15
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 9 months ago (2015-03-06 06:06:02 UTC) #16
commit-bot: I haz the power
5 years, 9 months ago (2015-03-06 06:06:54 UTC) #17
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/db71268248fb1221faa828cbc8dcba7fc2218c4a
Cr-Commit-Position: refs/heads/master@{#319416}

Powered by Google App Engine
This is Rietveld 408576698