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

Issue 591053002: Support print preset options for pdf document (Closed)

Created:
6 years, 3 months ago by Nikhil
Modified:
6 years, 1 month ago
CC:
dcheng, abarth-chromium, blink-reviews, dglazkov+blink, jamesr, mkwst+moarreviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Support print preset options for pdf document Blink changes to add support for print preset options for pdf documents. When pdf document has print preset options set, then print preview mode should use those options. This patch adds changes required to get print preset options from plugin. Also, this patch removes getPrintCopiesForPlugin() that was added initially and is no longer required. BUG=169120 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185276

Patch Set 1 #

Patch Set 2 : out-params should be by pointer #

Total comments: 12

Patch Set 3 : Review feedback (comment, initialization) #

Total comments: 6

Patch Set 4 : Review feedback (move API to WebLocalFrame.h from WebFrame.h) #

Patch Set 5 : Fix build error #

Patch Set 6 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -20 lines) Patch
M Source/web/WebLocalFrameImpl.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M Source/web/WebLocalFrameImpl.cpp View 1 2 3 4 5 2 chunks +4 lines, -3 lines 0 comments Download
M Source/web/WebPluginContainerImpl.h View 1 2 3 4 5 2 chunks +3 lines, -2 lines 0 comments Download
M Source/web/WebPluginContainerImpl.cpp View 1 2 3 4 5 2 chunks +3 lines, -2 lines 0 comments Download
M Source/web/WebRemoteFrameImpl.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M Source/web/WebRemoteFrameImpl.cpp View 1 2 3 1 chunk +0 lines, -6 lines 0 comments Download
M public/web/WebFrame.h View 1 2 3 4 5 1 chunk +0 lines, -3 lines 0 comments Download
M public/web/WebLocalFrame.h View 1 2 3 4 5 2 chunks +7 lines, -0 lines 0 comments Download
M public/web/WebPlugin.h View 1 2 2 chunks +3 lines, -2 lines 0 comments Download
A public/web/WebPrintPresetOptions.h View 1 2 1 chunk +48 lines, -0 lines 0 comments Download

Messages

Total messages: 25 (7 generated)
Nikhil
vitalybuka@ - PTAL at blink side changes to support print presets feature. Chrome side changes ...
6 years, 3 months ago (2014-09-23 18:51:38 UTC) #2
Nikhil
vitalybuka@, raymes@ - Could you please take a look at this? I'll request blink OWNERS ...
6 years, 2 months ago (2014-09-29 04:52:42 UTC) #4
raymes
https://codereview.chromium.org/591053002/diff/20001/Source/web/WebPluginContainerImpl.h File Source/web/WebPluginContainerImpl.h (right): https://codereview.chromium.org/591053002/diff/20001/Source/web/WebPluginContainerImpl.h#newcode142 Source/web/WebPluginContainerImpl.h:142: // Returns true if print preset options are updated ...
6 years, 2 months ago (2014-09-29 05:24:14 UTC) #5
Nikhil
raymes@ - Updated the patch based on your comments. PTAL. Thanks! https://codereview.chromium.org/591053002/diff/20001/Source/web/WebPluginContainerImpl.h File Source/web/WebPluginContainerImpl.h (right): ...
6 years, 2 months ago (2014-10-09 08:43:56 UTC) #7
dcheng
https://codereview.chromium.org/591053002/diff/60001/Source/web/WebRemoteFrameImpl.cpp File Source/web/WebRemoteFrameImpl.cpp (right): https://codereview.chromium.org/591053002/diff/60001/Source/web/WebRemoteFrameImpl.cpp#newcode664 Source/web/WebRemoteFrameImpl.cpp:664: bool WebRemoteFrameImpl::getPrintPresetOptionsForPlugin(const WebNode&, WebPrintPresetOptions*) Since we're changing this signature ...
6 years, 2 months ago (2014-10-09 08:53:42 UTC) #9
raymes
lgtm from my side I still think it would be nice to expose the information ...
6 years, 2 months ago (2014-10-09 20:44:02 UTC) #10
dcheng
https://codereview.chromium.org/591053002/diff/60001/public/web/WebFrame.h File public/web/WebFrame.h (right): https://codereview.chromium.org/591053002/diff/60001/public/web/WebFrame.h#newcode499 public/web/WebFrame.h:499: virtual bool getPrintPresetOptionsForPlugin(const WebNode&, WebPrintPresetOptions*) = 0; Actually it ...
6 years, 2 months ago (2014-10-09 20:45:03 UTC) #11
Vitaly Buka (NO REVIEWS)
lgtm https://codereview.chromium.org/591053002/diff/60001/public/web/WebPrintPresetOptions.h File public/web/WebPrintPresetOptions.h (right): https://codereview.chromium.org/591053002/diff/60001/public/web/WebPrintPresetOptions.h#newcode21 public/web/WebPrintPresetOptions.h:21: Should we put WebPageRange, WebPageRanges and WebDuplexMode inside ...
6 years, 2 months ago (2014-10-10 00:51:01 UTC) #12
Nikhil
dcheng@, raymes@, vitalybuka@ - Sorry for the delay! I've updated the patch based on your ...
6 years, 2 months ago (2014-10-20 10:04:23 UTC) #14
Nikhil
dcheng@ - <ping!> Could you please take a look at this?
6 years, 1 month ago (2014-11-05 09:05:08 UTC) #15
dcheng
Sorry, I was at BlinkOn the last few days. I'll be able to take a ...
6 years, 1 month ago (2014-11-06 22:45:29 UTC) #16
dcheng
lgtm, sorry for the delay
6 years, 1 month ago (2014-11-07 18:44:42 UTC) #17
Nikhil
mkwst@ - OWNERS review for Source/web/* please :) Chromium changes for this are already done ...
6 years, 1 month ago (2014-11-10 16:35:16 UTC) #19
Nikhil
mkwst@ - <ping!> Could you please take a look at this for OWNERS review for ...
6 years, 1 month ago (2014-11-12 16:04:51 UTC) #20
Mike West
LGTM.
6 years, 1 month ago (2014-11-12 17:56:36 UTC) #21
Nikhil
Thank you all for reviewing this! CQing this CL now :)
6 years, 1 month ago (2014-11-13 07:34:25 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/591053002/250001
6 years, 1 month ago (2014-11-13 07:35:18 UTC) #24
commit-bot: I haz the power
6 years, 1 month ago (2014-11-13 08:39:43 UTC) #25
Message was sent while issue was closed.
Committed patchset #6 (id:250001) as 185276

Powered by Google App Engine
This is Rietveld 408576698