Update proxy for PPP_Printing_Dev to deal with new QuerySupportedFormats

Update proxy for PPP_Printing_Dev to deal with new QuerySupportedFormats

BUG=80696
TEST=it compiles :-/ (printing seems to be broken already)

[dmichael (off chromium), 2011-08-24 17:57:45]

This isn't ready to land yet...  but I want to send this out as an FYI at least,
and to get a head start on the review. When the following CL lands:
http://codereview.chromium.org/7718004/
And we roll DEPS to it, the PPP_Printing_Dev proxy will break. This is the patch
that we'll need to fix it. I'll try to follow up the above Chromium CL with the
DEPS roll (hopefully today), but if I miss it, feel free to grab this patch.

[elijahtaylor (use chromium), 2011-08-24 20:08:23]

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
File src/shared/ppapi_proxy/browser_ppp_printing.cc (right):

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
src/shared/ppapi_proxy/browser_ppp_printing.cc:10: #include
"native_client/src/shared/ppapi_proxy/trusted/srpcgen/ppp_rpc.h"
Is this include kosher?  I see in other files:

#include "srpcgen/ppp_rpc.h"

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
File src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc (right):

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc:12: #include
"native_client/src/shared/ppapi_proxy/untrusted/srpcgen/ppp_rpc.h"
Same comment

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc:34: }
It looks like this is normally implemented in plugin_globals.cc

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc:71: if (ppp_printing ==
NULL || ppp_printing->Begin == NULL)
If you follow the pattern from plugin_globals.cc, they use the GetInterfaceSafe
function so you shouldn't need to check for NULL

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc:98: return;
same

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc:122: return;
same

[dmichael (off chromium), 2011-08-24 21:21:30]

Comments addressed; sorry for not proof-reading. Note I don't think I'll be able
to do the switcheroo until Monday, unless somebody else wants to do it. I'm OOO
8/25 and 8/26.

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
File src/shared/ppapi_proxy/browser_ppp_printing.cc (right):

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
src/shared/ppapi_proxy/browser_ppp_printing.cc:10: #include
"native_client/src/shared/ppapi_proxy/trusted/srpcgen/ppp_rpc.h"
On 2011/08/24 20:08:23, elijahtaylor wrote:
> Is this include kosher?  I see in other files:
> 
> #include "srpcgen/ppp_rpc.h"

I would contend that all the other code is wrong :-)

The 'srpcgen/ppp_rpc.h' is a relative path, which goes against guidelines and
triggers a pre-submit warning.

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
File src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc (right):

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/plugin_p...
src/shared/ppapi_proxy/plugin_ppp_printing_rpc_server.cc:71: if (ppp_printing ==
NULL || ppp_printing->Begin == NULL)
On 2011/08/24 20:08:23, elijahtaylor wrote:
> If you follow the pattern from plugin_globals.cc, they use the
GetInterfaceSafe
> function so you shouldn't need to check for NULL
Oops. I had the change in a git repo that was set up the old/wrong way, and when
I copied the files over to the new one, I skipped a refactoring change and undid
somebody's good work. Fixed.

[elijahtaylor (use chromium), 2011-08-24 21:55:28]

LGTM given: you check with Polina on the include issue, and the other CL lands
and all other caveats of DEPS rolling etc happens

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
File src/shared/ppapi_proxy/browser_ppp_printing.cc (right):

http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
src/shared/ppapi_proxy/browser_ppp_printing.cc:10: #include
"native_client/src/shared/ppapi_proxy/trusted/srpcgen/ppp_rpc.h"
On 2011/08/24 21:21:30, dmichael wrote:
> On 2011/08/24 20:08:23, elijahtaylor wrote:
> > Is this include kosher?  I see in other files:
> > 
> > #include "srpcgen/ppp_rpc.h"
> 
> I would contend that all the other code is wrong :-)
> 
> The 'srpcgen/ppp_rpc.h' is a relative path, which goes against guidelines and
> triggers a pre-submit warning.

How very enlightened :P

Can you check with Polina on this and get her take on it?  There may be a valid
reason why it *should* be the other way despite style guidelines.

[dmichael (off chromium), 2011-08-29 20:23:35]

On 2011/08/24 21:55:28, elijahtaylor wrote:
> LGTM given: you check with Polina on the include issue,
I couldn't get a hold of Polina today, but I had already vetted this in a prior
CL reviewed by sehr for input events.
> and the other CL lands
> and all other caveats of DEPS rolling etc happens
Since the proxy moved in to Chromium, the DEPS isn't happening. I rolled this
change in to the chrome CL:
http://codereview.chromium.org/7718004/
> 
>
http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
> File src/shared/ppapi_proxy/browser_ppp_printing.cc (right):
> 
>
http://codereview.chromium.org/7714041/diff/1/src/shared/ppapi_proxy/browser_...
> src/shared/ppapi_proxy/browser_ppp_printing.cc:10: #include
> "native_client/src/shared/ppapi_proxy/trusted/srpcgen/ppp_rpc.h"
> On 2011/08/24 21:21:30, dmichael wrote:
> > On 2011/08/24 20:08:23, elijahtaylor wrote:
> > > Is this include kosher?  I see in other files:
> > > 
> > > #include "srpcgen/ppp_rpc.h"
> > 
> > I would contend that all the other code is wrong :-)
> > 
> > The 'srpcgen/ppp_rpc.h' is a relative path, which goes against guidelines
and
> > triggers a pre-submit warning.
> 
> How very enlightened :P
> 
> Can you check with Polina on this and get her take on it?  There may be a
valid
> reason why it *should* be the other way despite style guidelines.
As mentioned, I'd vetted this with sehr@ previously. If there was a reason at
the time the first file was written that way, the reason's no longer valid. That
CL is here:
http://codereview.chromium.org/7395005
Though I think the include conversation was over IM.