Add IsScalingDisabled and PP_PRINTOUTPUTFORMAT_EMF to pepper printing interface.

Add IsScalingDisabled and PP_PRINTOUTPUTFORMAT_EMF to pepper printing interface.

IsScalingDisabled will let us handle PDFS that don't want to be scaled.
PP_PRINTOUTPUTFORMAT_EMF is to simply the printing flow on Windows.

BUG=67091, 92045, 91880, 92000, 92218, 95905
TEST=NONE

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=103177

[vandebo (ex-Chrome), 2011-09-26 23:41:33]

Lei has the PDF plugin changes for this ready to go.

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc:5: // WARNING
WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
I couldn't figure out how to generate these files, so I patched it by hand.

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_server.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_server.cc:5: // WARNING
WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
ditto

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/trusted/srpcgen/ppp_rpc.h
(right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/trusted/srpcgen/ppp_rpc.h:5: //
WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
ditto

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/untrusted/srpcgen/ppp_rpc.h
(right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/untrusted/srpcgen/ppp_rpc.h:5: //
WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
ditto

[vandebo (ex-Chrome), 2011-09-26 23:54:54]

+jam to reviewers for webkit/plugins/ changes

[dmichael (off chromium), 2011-09-27 15:28:38]

Is it intentional that you're not using the new EMF format yet? It probably does
make sense to do that part in a separate CL and get all the interface changes in
1 CL. I'm just checking.

http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h
File ppapi/c/dev/ppp_printing_dev.h (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h#...
ppapi/c/dev/ppp_printing_dev.h:68: bool (*IsScalingDisabled)(PP_Instance
instance);
We avoid using bool in C for various reasons (particularly, the C definition is
not necessarily compatible with C++ bool, which can lead to problems keeping a
consistent ABI). Please use PP_Bool.

Also...  we'll generally want to add functions on to the end instead of in the
middle. It makes the backwards-compatibility slightly easier in some cases.
(E.g., a plugin that wants to support 0.5 and 0.4 could safely return a 0.5
struct for either if the new function was on the end). Since this is still a Dev
interface though, it's fine to put it here if this seems like the most logical
place for it to go long term.

http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc
File ppapi/cpp/dev/printing_dev.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc#n...
ppapi/cpp/dev/printing_dev.cc:29: return 0;
0->false?

http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc
File ppapi/example/example.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc#newcod...
ppapi/example/example.cc:331: // TODO(mball,dmichael) Replace this with the
PPP_PRINTING_DEV_USE_0_5 version
Oops. This is dead code now that I must have missed. Would you be willing to
update it in this CL and remove the TODO? It should be pretty easy... I think
the equivalent implementation for the newer interface will just be:
"""
return PP_PRINTOUTPUTFORMAT_RASTER;
"""

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/browser_ppp_printing.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppp_printing.cc:95:
DebugPrintf("PPP_Printing_Dev::PrintPages: %s\n",
nice catch, thanks!

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc:19: 'outputs':
[['result', 'bool'],
This will be int32_t for PP_Bool (note I'm not highlighting all the places that
will change)

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc:5: // WARNING
WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
On 2011/09/26 23:41:33, vandebo wrote:
> I couldn't figure out how to generate these files, so I patched it by hand.
I haven't had to do this yet since things moved to Chromium.

Please try:
> cd native_client
> ./scons srpcgen

If that works for you, I can send an e-mail to pepper-team and try to document
it somewhere. It's definitely not obvious.

http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webp...
File webkit/plugins/ppapi/ppapi_webplugin_impl.h (right):

http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webp...
webkit/plugins/ppapi/ppapi_webplugin_impl.h:76: virtual bool
isPrintScalingDisabled();
It looks like this function is not available yet...  is there a WebKit patch
pending?

If possible, it would be best for these functions to have OVERRIDE on them to
protect against bugs like I made in the ppapi example, where an override turned
in to dead code:
http://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thre...

[dmichael (off chromium), 2011-09-27 15:30:09]

One more thing...  I think we should consider keeping backwards compatibility. I
know the normal PDF plugin can be fixed quickly, but the NaCl PDF extension will
break with this change unless there's backwards compatibility. +elijahtaylor

[vandebo (ex-Chrome), 2011-09-27 20:02:24]

On 2011/09/27 15:28:38, dmichael wrote:
> Is it intentional that you're not using the new EMF format yet? It probably
does
> make sense to do that part in a separate CL and get all the interface changes
in
> 1 CL. I'm just checking.

Yes, there are a few changes needed to use the new EMF format, but it makes
sense to add it to the interface while iterating it.  I'm not really using
IsPrintScalingDisabled in this CL either.

On 2011/09/27 15:30:09, dmichael wrote:
> One more thing...  I think we should consider keeping backwards compatibility.
I
> know the normal PDF plugin can be fixed quickly, but the NaCl PDF extension
will
> break with this change unless there's backwards compatibility. +elijahtaylor

I think you mean to just move the new method to be the last one?  I've done
that.

http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h
File ppapi/c/dev/ppp_printing_dev.h (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h#...
ppapi/c/dev/ppp_printing_dev.h:68: bool (*IsScalingDisabled)(PP_Instance
instance);
On 2011/09/27 15:28:38, dmichael wrote:
> We avoid using bool in C for various reasons (particularly, the C definition
is
> not necessarily compatible with C++ bool, which can lead to problems keeping a
> consistent ABI). Please use PP_Bool.

Done.
 
> Also...  we'll generally want to add functions on to the end instead of in the
> middle. It makes the backwards-compatibility slightly easier in some cases.
> (E.g., a plugin that wants to support 0.5 and 0.4 could safely return a 0.5
> struct for either if the new function was on the end). Since this is still a
Dev
> interface though, it's fine to put it here if this seems like the most logical
> place for it to go long term.

Because there's an End method, I think anyone changing this is going to want to
add to the middle somewhere.  I've pulled my new method to the end because I
think you requested that in a later comment.

http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc
File ppapi/cpp/dev/printing_dev.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc#n...
ppapi/cpp/dev/printing_dev.cc:29: return 0;
On 2011/09/27 15:28:38, dmichael wrote:
> 0->false?

Done.

http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc
File ppapi/example/example.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc#newcod...
ppapi/example/example.cc:331: // TODO(mball,dmichael) Replace this with the
PPP_PRINTING_DEV_USE_0_5 version
On 2011/09/27 15:28:38, dmichael wrote:
> Oops. This is dead code now that I must have missed. Would you be willing to
> update it in this CL and remove the TODO? It should be pretty easy... I think
> the equivalent implementation for the newer interface will just be:
> """
> return PP_PRINTOUTPUTFORMAT_RASTER;
> """

Done.

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc:19: 'outputs':
[['result', 'bool'],
On 2011/09/27 15:28:38, dmichael wrote:
> This will be int32_t for PP_Bool (note I'm not highlighting all the places
that
> will change)

Done.

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
File ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc (right):

http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc:5: // WARNING
WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
On 2011/09/27 15:28:38, dmichael wrote:
> On 2011/09/26 23:41:33, vandebo wrote:
> > I couldn't figure out how to generate these files, so I patched it by hand.
> I haven't had to do this yet since things moved to Chromium.
> 
> Please try:
> > cd native_client
> > ./scons srpcgen
> 
> If that works for you, I can send an e-mail to pepper-team and try to document
> it somewhere. It's definitely not obvious.

That works.  How about adding the instructions/command line used to the auto
generated output (i.e. here)?

http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webp...
File webkit/plugins/ppapi/ppapi_webplugin_impl.h (right):

http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webp...
webkit/plugins/ppapi/ppapi_webplugin_impl.h:76: virtual bool
isPrintScalingDisabled();
On 2011/09/27 15:28:38, dmichael wrote:
> It looks like this function is not available yet...  is there a WebKit patch
> pending?

Yes: https://bugs.webkit.org/show_bug.cgi?id=68853

> If possible, it would be best for these functions to have OVERRIDE on them to
> protect against bugs like I made in the ppapi example, where an override
turned
> in to dead code:
>
http://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thre...

Done.  But that required pulling the webkit/plugins changes into a different CL:
http://codereview.chromium.org/8051018/

[dmichael (off chromium), 2011-09-27 20:49:22]

> > One more thing...  I think we should consider keeping backwards
compatibility.
> I
> > know the normal PDF plugin can be fixed quickly, but the NaCl PDF extension
> will
> > break with this change unless there's backwards compatibility. +elijahtaylor
> 
> I think you mean to just move the new method to be the last one?  I've done
> that.
> 
No, that's actually orthogonal. With your change, the Browser will ask plugins
for version 0.5 of the printing interface, *only*. So PDF will be broken in the
brief time before Lei lands the PDF fix. And the NaCl PDF extension will be
broken for longer. Or...  at least printing will be.

Putting the function at the end is less important to me. You can put it in the
middle if you prefer; that's kind of a comment for future work. It will be more
important for non-dev interfaces.

What you would need to do to provide backwards-compatibility is to request the
new 0.5 interface from the plugin, and if that fails, request the 0.4 version of
the interface. This requires a way for the browser to be able to talk to
whichever version. Here is a CL that *removed* back-compat code for this
interface recently, to give you an idea of how to do it in the browser side:
http://codereview.chromium.org/7794007
Unfortunately, you'd also have to pipe it through the proxy in a
backwards-compatible way.

I suspect we can live without backwards-compatibility here, but I wanted to get
Elijah's opinion as the maintainer of the NaCl PDF extension.

> http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h
> File ppapi/c/dev/ppp_printing_dev.h (right):
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h#...
> ppapi/c/dev/ppp_printing_dev.h:68: bool (*IsScalingDisabled)(PP_Instance
> instance);
> On 2011/09/27 15:28:38, dmichael wrote:
> > We avoid using bool in C for various reasons (particularly, the C definition
> is
> > not necessarily compatible with C++ bool, which can lead to problems keeping
a
> > consistent ABI). Please use PP_Bool.
> 
> Done.
>  
> > Also...  we'll generally want to add functions on to the end instead of in
the
> > middle. It makes the backwards-compatibility slightly easier in some cases.
> > (E.g., a plugin that wants to support 0.5 and 0.4 could safely return a 0.5
> > struct for either if the new function was on the end). Since this is still a
> Dev
> > interface though, it's fine to put it here if this seems like the most
logical
> > place for it to go long term.
> 
> Because there's an End method, I think anyone changing this is going to want
to
> add to the middle somewhere.  I've pulled my new method to the end because I
> think you requested that in a later comment.
> 
> http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc
> File ppapi/cpp/dev/printing_dev.cc (right):
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc#n...
> ppapi/cpp/dev/printing_dev.cc:29: return 0;
> On 2011/09/27 15:28:38, dmichael wrote:
> > 0->false?
> 
> Done.
> 
> http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc
> File ppapi/example/example.cc (right):
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc#newcod...
> ppapi/example/example.cc:331: // TODO(mball,dmichael) Replace this with the
> PPP_PRINTING_DEV_USE_0_5 version
> On 2011/09/27 15:28:38, dmichael wrote:
> > Oops. This is dead code now that I must have missed. Would you be willing to
> > update it in this CL and remove the TODO? It should be pretty easy... I
think
> > the equivalent implementation for the newer interface will just be:
> > """
> > return PP_PRINTOUTPUTFORMAT_RASTER;
> > """
> 
> Done.
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
> File ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc (right):
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
> ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc:19: 'outputs':
> [['result', 'bool'],
> On 2011/09/27 15:28:38, dmichael wrote:
> > This will be int32_t for PP_Bool (note I'm not highlighting all the places
> that
> > will change)
> 
> Done.
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
> File ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc (right):
> 
>
http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/...
> ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc:5: // WARNING
> WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
> On 2011/09/27 15:28:38, dmichael wrote:
> > On 2011/09/26 23:41:33, vandebo wrote:
> > > I couldn't figure out how to generate these files, so I patched it by
hand.
> > I haven't had to do this yet since things moved to Chromium.
> > 
> > Please try:
> > > cd native_client
> > > ./scons srpcgen
> > 
> > If that works for you, I can send an e-mail to pepper-team and try to
document
> > it somewhere. It's definitely not obvious.
> 
> That works.  How about adding the instructions/command line used to the auto
> generated output (i.e. here)?
> 
>
http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webp...
> File webkit/plugins/ppapi/ppapi_webplugin_impl.h (right):
> 
>
http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webp...
> webkit/plugins/ppapi/ppapi_webplugin_impl.h:76: virtual bool
> isPrintScalingDisabled();
> On 2011/09/27 15:28:38, dmichael wrote:
> > It looks like this function is not available yet...  is there a WebKit patch
> > pending?
> 
> Yes: https://bugs.webkit.org/show_bug.cgi?id=68853
> 
> > If possible, it would be best for these functions to have OVERRIDE on them
to
> > protect against bugs like I made in the ppapi example, where an override
> turned
> > in to dead code:
> >
>
http://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thre...
> 
> Done.  But that required pulling the webkit/plugins changes into a different
CL:
> http://codereview.chromium.org/8051018/

[elijahtaylor (use chromium), 2011-09-27 20:56:03]

On Tue, Sep 27, 2011 at 1:49 PM, <dmichael@chromium.org> wrote:

> > One more thing...  I think we should consider keeping backwards
>>
> compatibility.
>
>> I
>> > know the normal PDF plugin can be fixed quickly, but the NaCl PDF
>> extension
>> will
>> > break with this change unless there's backwards compatibility.
>> +elijahtaylor
>>
>
>  I think you mean to just move the new method to be the last one?  I've
>> done
>> that.
>>
>
> No, that's actually orthogonal. With your change, the Browser will ask
> plugins
> for version 0.5 of the printing interface, *only*. So PDF will be broken in
> the
> brief time before Lei lands the PDF fix. And the NaCl PDF extension will be
> broken for longer. Or...  at least printing will be.
>
> Putting the function at the end is less important to me. You can put it in
> the
> middle if you prefer; that's kind of a comment for future work. It will be
> more
> important for non-dev interfaces.
>
> What you would need to do to provide backwards-compatibility is to request
> the
> new 0.5 interface from the plugin, and if that fails, request the 0.4
> version of
> the interface. This requires a way for the browser to be able to talk to
> whichever version. Here is a CL that *removed* back-compat code for this
> interface recently, to give you an idea of how to do it in the browser
> side:
>
http://codereview.chromium.**org/7794007<http://codereview.chromium.org/7794007>
> Unfortunately, you'd also have to pipe it through the proxy in a
> backwards-compatible way.
>
> I suspect we can live without backwards-compatibility here, but I wanted to
> get
> Elijah's opinion as the maintainer of the NaCl PDF extension.


I appreciate the heads-up.  As long as changes are made to the NaCl proxy at
the same time as the actual interfaces, we should be able to build the NaCl
PDF extension.  This has been much easier now that the code all lives in
chrome.  I'll admit I haven't been keeping up with it very well and it
likely doesn't work right now anyway.

I'm looking to get NaCl PDF compiled in a similar process to how PDF is
already compiled, but in the meantime I wouldn't worry about holding up
changes as long as NaCl can use the same interfaces at the same time.


>
>
>  http://codereview.chromium.**org/8041052/diff/1/ppapi/c/**
>>
dev/ppp_printing_dev.h<http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h>
>> File ppapi/c/dev/ppp_printing_dev.h (right):
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/c/**
>
dev/ppp_printing_dev.h#**newcode68<http://codereview.chromium.org/8041052/diff/1/ppapi/c/dev/ppp_printing_dev.h#newcode68>
>
>> ppapi/c/dev/ppp_printing_dev.**h:68: bool (*IsScalingDisabled)(PP_**
>> Instance
>> instance);
>> On 2011/09/27 15:28:38, dmichael wrote:
>> > We avoid using bool in C for various reasons (particularly, the C
>> definition
>> is
>> > not necessarily compatible with C++ bool, which can lead to problems
>> keeping
>>
> a
>
>> > consistent ABI). Please use PP_Bool.
>>
>
>  Done.
>>
>
>  > Also...  we'll generally want to add functions on to the end instead of
>> in
>>
> the
>
>> > middle. It makes the backwards-compatibility slightly easier in some
>> cases.
>> > (E.g., a plugin that wants to support 0.5 and 0.4 could safely return a
>> 0.5
>> > struct for either if the new function was on the end). Since this is
>> still a
>> Dev
>> > interface though, it's fine to put it here if this seems like the most
>>
> logical
>
>> > place for it to go long term.
>>
>
>  Because there's an End method, I think anyone changing this is going to
>> want
>>
> to
>
>> add to the middle somewhere.  I've pulled my new method to the end because
>> I
>> think you requested that in a later comment.
>>
>
>  http://codereview.chromium.**org/8041052/diff/1/ppapi/cpp/**
>>
dev/printing_dev.cc<http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc>
>> File ppapi/cpp/dev/printing_dev.cc (right):
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/cpp/**
>
dev/printing_dev.cc#newcode29<http://codereview.chromium.org/8041052/diff/1/ppapi/cpp/dev/printing_dev.cc#newcode29>
>
>> ppapi/cpp/dev/printing_dev.cc:**29: return 0;
>> On 2011/09/27 15:28:38, dmichael wrote:
>> > 0->false?
>>
>
>  Done.
>>
>
>  http://codereview.chromium.**org/8041052/diff/1/ppapi/**
>>
example/example.cc<http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc>
>> File ppapi/example/example.cc (right):
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/**
>
example/example.cc#newcode331<http://codereview.chromium.org/8041052/diff/1/ppapi/example/example.cc#newcode331>
>
>> ppapi/example/example.cc:331: // TODO(mball,dmichael) Replace this with
>> the
>> PPP_PRINTING_DEV_USE_0_5 version
>> On 2011/09/27 15:28:38, dmichael wrote:
>> > Oops. This is dead code now that I must have missed. Would you be
>> willing to
>> > update it in this CL and remove the TODO? It should be pretty easy... I
>>
> think
>
>> > the equivalent implementation for the newer interface will just be:
>> > """
>> > return PP_PRINTOUTPUTFORMAT_RASTER;
>> > """
>>
>
>  Done.
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/**
>
native_client/src/shared/**ppapi_proxy/ppp_printing.srpc<http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc>
>
>> File ppapi/native_client/src/**shared/ppapi_proxy/ppp_**printing.srpc
>> (right):
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/**
>
native_client/src/shared/**ppapi_proxy/ppp_printing.srpc#**newcode19<http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/ppapi_proxy/ppp_printing.srpc#newcode19>
>
>> ppapi/native_client/src/**shared/ppapi_proxy/ppp_**printing.srpc:19:
>> 'outputs':
>> [['result', 'bool'],
>> On 2011/09/27 15:28:38, dmichael wrote:
>> > This will be int32_t for PP_Bool (note I'm not highlighting all the
>> places
>> that
>> > will change)
>>
>
>  Done.
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/**
>
native_client/src/shared/**ppapi_proxy/ppp_rpc_client.cc<http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc>
>
>> File ppapi/native_client/src/**shared/ppapi_proxy/ppp_rpc_**client.cc
>> (right):
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/ppapi/**
>
native_client/src/shared/**ppapi_proxy/ppp_rpc_client.cc#**newcode5<http://codereview.chromium.org/8041052/diff/1/ppapi/native_client/src/shared/ppapi_proxy/ppp_rpc_client.cc#newcode5>
>
>> ppapi/native_client/src/**shared/ppapi_proxy/ppp_rpc_**client.cc:5: //
>> WARNING
>> WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
>> On 2011/09/27 15:28:38, dmichael wrote:
>> > On 2011/09/26 23:41:33, vandebo wrote:
>> > > I couldn't figure out how to generate these files, so I patched it by
>>
> hand.
>
>> > I haven't had to do this yet since things moved to Chromium.
>> >
>> > Please try:
>> > > cd native_client
>> > > ./scons srpcgen
>> >
>> > If that works for you, I can send an e-mail to pepper-team and try to
>>
> document
>
>> > it somewhere. It's definitely not obvious.
>>
>
>  That works.  How about adding the instructions/command line used to the
>> auto
>> generated output (i.e. here)?
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/webkit/**
>
plugins/ppapi/ppapi_webplugin_**impl.h<http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webplugin_impl.h>
>
>> File webkit/plugins/ppapi/ppapi_**webplugin_impl.h (right):
>>
>
>
> http://codereview.chromium.**org/8041052/diff/1/webkit/**
>
plugins/ppapi/ppapi_webplugin_**impl.h#newcode76<http://codereview.chromium.org/8041052/diff/1/webkit/plugins/ppapi/ppapi_webplugin_impl.h#newcode76>
>
>> webkit/plugins/ppapi/ppapi_**webplugin_impl.h:76: virtual bool
>> isPrintScalingDisabled();
>> On 2011/09/27 15:28:38, dmichael wrote:
>> > It looks like this function is not available yet...  is there a WebKit
>> patch
>> > pending?
>>
>
>  Yes:
https://bugs.webkit.org/show_**bug.cgi?id=68853<https://bugs.webkit.org/show_...
>>
>
>  > If possible, it would be best for these functions to have OVERRIDE on
>> them
>>
> to
>
>> > protect against bugs like I made in the ppapi example, where an override
>> turned
>> > in to dead code:
>> >
>>
>
> http://groups.google.com/a/**chromium.org/group/chromium-**
>
dev/browse_thread/thread/**cf377eca03e92cf4/**fb40dfe3f24a2807<http://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thread/cf377eca03e92cf4/fb40dfe3f24a2807>
>
>  Done.  But that required pulling the webkit/plugins changes into a
>> different
>>
> CL:
>
>>
http://codereview.chromium.**org/8051018/<http://codereview.chromium.org/8051...
>>
>
>
>
>
http://codereview.chromium.**org/8041052/<http://codereview.chromium.org/8041...
>

[dmichael (off chromium), 2011-09-27 21:12:41]

Given what Elijah said, LGTM

Feel free to move the function back to the middle of the interface, if you
prefer it that way. I'm fine either way.

[vandebo (ex-Chrome), 2011-09-27 22:23:00]

As you said, putting the new method at the end makes it easier to do backward
compatibility.  This way wasn't too hard.  If you don't like the general
approach, I'll just undo it.

[vandebo (ex-Chrome), 2011-09-28 20:50:23]

On 2011/09/27 22:23:00, vandebo wrote:
> As you said, putting the new method at the end makes it easier to do backward
> compatibility.  This way wasn't too hard.  If you don't like the general
> approach, I'll just undo it.

In IM David thought it was too much of a hack.  Deleting patch set 3.