Add PPAPI C++ wrappers for PDF.

Add PPAPI C++ wrappers for PDF.

Add C++ wrappers for PDF. The wrappers aren't entirely idiomatic (e.g. there
is a case where we just return a PP_Resource instead of wrapping it in an
object). This is because of the weird way that the interface is used in the
plugin which just makes it more convenient to return the raw resource ID.

BUG=


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188909

[yzshen1, 2013-03-18 17:55:46]

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc
File ppapi/cpp/private/pdf.cc (right):

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:52: const struct PP_FontDescription_Dev* description,
nit: 'struct' is not needed (and there are other places in this file).

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:109: void PDF::HistogramPDFPageCount(const
InstanceHandle& instance,
Please use /* instance */, otherwise it may cause warnings.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:111: if (has_interface<PPB_PDF>()) {
nit: no need to have {} (as line 95-96).

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:117: void PDF::UserMetricsRecordAction(const
InstanceHandle& instance,
Please use /* instance */, otherwise it may cause warnings.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:142: bool PDF::IsFeatureEnabled(const InstanceHandle&
instance,
ditto.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:150: pp::ImageData PDF::GetResourceImageForScale(const
InstanceHandle& instance,
you don't need pp:: (and there are other places that have the same problem).

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h
File ppapi/cpp/private/pdf.h (right):

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:25: static pp::ImageData GetResourceImage(const
InstanceHandle& instance,
you don't need pp::

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:29: const struct PP_FontDescription_Dev* description,
you don't need 'struct'.

nit, question: I don't know for sure, do we usually follow the C interface to
use const* or change to const& in this case? (I  notice that we use const* in
tcp_socket_private.)

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:40: struct PP_PrivateFindResult** results,
you don't need 'struct'.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:55: static pp::ImageData GetResourceImageForScale(const
InstanceHandle& instance,
you don't need pp::

[raymes, 2013-03-18 18:15:39]

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc
File ppapi/cpp/private/pdf.cc (right):

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:52: const struct PP_FontDescription_Dev* description,
On 2013/03/18 17:55:47, yzshen1 wrote:
> nit: 'struct' is not needed (and there are other places in this file).

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:109: void PDF::HistogramPDFPageCount(const
InstanceHandle& instance,
On 2013/03/18 17:55:47, yzshen1 wrote:
> Please use /* instance */, otherwise it may cause warnings.

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:111: if (has_interface<PPB_PDF>()) {
On 2013/03/18 17:55:47, yzshen1 wrote:
> nit: no need to have {} (as line 95-96).

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:117: void PDF::UserMetricsRecordAction(const
InstanceHandle& instance,
On 2013/03/18 17:55:47, yzshen1 wrote:
> Please use /* instance */, otherwise it may cause warnings.

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:142: bool PDF::IsFeatureEnabled(const InstanceHandle&
instance,
On 2013/03/18 17:55:47, yzshen1 wrote:
> ditto.

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.cc#n...
ppapi/cpp/private/pdf.cc:150: pp::ImageData PDF::GetResourceImageForScale(const
InstanceHandle& instance,
On 2013/03/18 17:55:47, yzshen1 wrote:
> you don't need pp:: (and there are other places that have the same problem).

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h
File ppapi/cpp/private/pdf.h (right):

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:25: static pp::ImageData GetResourceImage(const
InstanceHandle& instance,
On 2013/03/18 17:55:47, yzshen1 wrote:
> you don't need pp::

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:29: const struct PP_FontDescription_Dev* description,
Done.

I'm not sure - probably a reference is more idiomatic. These functions should be
removed when we move to Bill's API though so I'm not too concerned. I can change
it if you prefer.

On 2013/03/18 17:55:47, yzshen1 wrote:
> you don't need 'struct'.
> 
> nit, question: I don't know for sure, do we usually follow the C interface to
> use const* or change to const& in this case? (I  notice that we use const* in
> tcp_socket_private.)

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:40: struct PP_PrivateFindResult** results,
On 2013/03/18 17:55:47, yzshen1 wrote:
> you don't need 'struct'.

Done.

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:55: static pp::ImageData GetResourceImageForScale(const
InstanceHandle& instance,
On 2013/03/18 17:55:47, yzshen1 wrote:
> you don't need pp::

Done.

[yzshen1, 2013-03-18 19:25:14]

lgtm

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h
File ppapi/cpp/private/pdf.h (right):

https://codereview.chromium.org/12527012/diff/2001/ppapi/cpp/private/pdf.h#ne...
ppapi/cpp/private/pdf.h:29: const struct PP_FontDescription_Dev* description,
I don't think it is very important. Thanks!
On 2013/03/18 18:15:39, raymes wrote:
> Done.
> 
> I'm not sure - probably a reference is more idiomatic. These functions should
be
> removed when we move to Bill's API though so I'm not too concerned. I can
change
> it if you prefer.
> 
> On 2013/03/18 17:55:47, yzshen1 wrote:
> > you don't need 'struct'.
> > 
> > nit, question: I don't know for sure, do we usually follow the C interface
to
> > use const* or change to const& in this case? (I  notice that we use const*
in
> > tcp_socket_private.)
>

[commit-bot: I haz the power, 2013-03-18 21:08:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/12527012/5004

[commit-bot: I haz the power, 2013-03-18 21:28:06]

Failed to trigger a try job on win_x64_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-03-18 21:28:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/12527012/3004

[commit-bot: I haz the power, 2013-03-19 01:44:17]

Change committed as 188909