Issue 2863019: Added support for vector printing for Pepper v1 plugins for Windows. Mac and ...

Issue 2863019: Added support for vector printing for Pepper v1 plugins for Windows. Mac and ... (Closed)

Created:
10 years, 6 months ago by sanjeevr

Modified:
9 years, 7 months ago

Reviewers:
jam

CC:
chromium-reviews, jam+cc_chromium.org, Erik does not do reviews, Aaron Boodman, stuartmorgan, pam+watch_chromium.org, brettw-cc_chromium.org, darin-cc_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Added support for vector printing for Pepper v1 plugins for Windows. Mac and Linux to follow. BUG=None. TEST=Test printing from Chrome PDF plugin on Windows. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50699

Patch Set 1 #

Patch Set 2 : Disabled for non-Windows #

Total comments: 6

Patch Set 3 : Code review changes #

Patch Set 4 : Typo fixed #

Created: 10 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+96 lines, -0 lines)			Patch
M	chrome/renderer/webplugin_delegate_pepper.h	View	1 2	2 chunks	+3 lines, -0 lines	0 comments	Download
M	chrome/renderer/webplugin_delegate_pepper.cc	View	1 2 3	8 chunks	+85 lines, -0 lines	0 comments	Download
M	third_party/npapi/bindings/npapi_extensions.h	View	1 2	2 chunks	+8 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

jam

lgtm http://codereview.chromium.org/2863019/diff/5001/6001 File chrome/renderer/webplugin_delegate_pepper.cc (right): http://codereview.chromium.org/2863019/diff/5001/6001#newcode1137 chrome/renderer/webplugin_delegate_pepper.cc:1137: HMODULE pdf_module = GetModuleHandle(L"pdf.dll"); can you use PathService::Get(chrome::FILE_PDF_PLUGIN, ...

10 years, 6 months ago (2010-06-23 22:46:51 UTC) #2

sanjeevr

10 years, 6 months ago (2010-06-24 00:38:54 UTC) #3

Made all the changes.

http://codereview.chromium.org/2863019/diff/5001/6001
File chrome/renderer/webplugin_delegate_pepper.cc (right):

http://codereview.chromium.org/2863019/diff/5001/6001#newcode1137
chrome/renderer/webplugin_delegate_pepper.cc:1137: HMODULE pdf_module =
GetModuleHandle(L"pdf.dll");
On 2010/06/23 22:46:52, John Abd-El-Malek wrote:
> can you use PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path) so we don't
> duplicate the name of the dll here?

Done.

http://codereview.chromium.org/2863019/diff/5001/6001#newcode1140
chrome/renderer/webplugin_delegate_pepper.cc:1140: typedef bool (__stdcall
*RenderPDFPageToDCProc)(
On 2010/06/23 22:46:52, John Abd-El-Malek wrote:
> nit: I haven't seen typedef's inside functions in the rest of the code,
perhaps
> put this at the top?

Done.

http://codereview.chromium.org/2863019/diff/5001/6001#newcode1199
chrome/renderer/webplugin_delegate_pepper.cc:1199: if
(VectorPrintPage(page_number, canvas)) {
On 2010/06/23 22:46:52, John Abd-El-Malek wrote:
> nit: i think the rest of this file doesn't use brace brackets for one line
> statements

Done.

Expand Messages | Collapse Messages