Connect Skia PDF backend to printing subsystem.

+ This CL pulls in all the PDF code (i.e. we are now compiling the PDF backend on Chrome).
+ Add a Metafile to contain Skia PDF content.
+ Add a VectorPlatformDevice for use with the Skia PDF backend.

BUG=62889
TEST=NONE

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

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

[vandebo (ex-Chrome), 2011-02-15 21:32:58]

FY: This is the scaffolding I have been using to test my PDF backend.

[vandebo (ex-Chrome), 2011-03-18 19:01:05]

FYI, with http://codereview.chromium.org/6499024/ and the NativeMetaFileFactory,
the scaffolding doesn't have any hacks left in it....

[vandebo (ex-Chrome), 2011-03-22 18:58:38]

FYI Chris, new patch set.

[reed1, 2011-04-06 20:20:53]

I know this feature is a long time coming, but we should be sure to sync up with
Alok before checking this in. This CL is extending the current platformdevice
notion, which I think Alok is explicitly trying to end-of-life in chrome.

[vandebo (ex-Chrome), 2011-04-06 20:28:50]

On 2011/04/06 20:20:53, reed1 wrote:
> I know this feature is a long time coming, but we should be sure to sync up
with
> Alok before checking this in. This CL is extending the current platformdevice
> notion, which I think Alok is explicitly trying to end-of-life in chrome.

Indeed, I forgot to add Alok.

Alok, how will this interact with your changes?

[alokp, 2011-04-06 20:36:32]

I think this is fine for now. I am currently focusing on deprecating
PlatformCanvas. PlatformDevice will come later. It would be nice to start
thinking about ways if it can be implemented without subclassing SkDevice.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/platform_device_li...
File skia/ext/platform_device_linux.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/platform_device_li...
skia/ext/platform_device_linux.h:27: virtual void endPlatformPaint();
I have already added this function in one of the CLs pending review. That CL
also makes all function calls start with CAPS.

http://codereview.chromium.org/6691067/

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_skia.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_skia.h:31: class VectorPlatformDeviceSkia :
public PlatformDevice {
This is consistent with other vector-devices and should be fine for now. But we
should start thinking about getting rid of subclassing. It would be great if we
could accomplish the same thing using skia meta-data.

[reed1, 2011-04-06 20:40:46]

We *definitely* have to be moving toward not subclassing SkDevice per-platform,
but only functionally. Alok, if you think this CL doesn't make that situation
worse, then we can let it go (unless you want to have it land *after* you land
your canvas work...)

[alokp, 2011-04-06 20:43:15]

On 2011/04/06 20:40:46, reed1 wrote:
> We *definitely* have to be moving toward not subclassing SkDevice
per-platform,
> but only functionally. Alok, if you think this CL doesn't make that situation
> worse, then we can let it go (unless you want to have it land *after* you land
> your canvas work...)

This does not affect my canvas work.

[Lei Zhang, 2011-04-06 20:43:56]

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia.cc
File printing/pdf_metafile_skia.cc (right):

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:74: FinishPage();
nit: indentation

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:75: data_->pdf_doc_.emitPDF(&data_->pdf_stream_);
check emitPDF's return value?

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:85: if (dst_buffer_size > GetDataSize())
is this reversed?

dst_buffer_size = 10, GetDataSize() returns 5
==> Data fits in |dst_buffer|.

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia.h
File printing/pdf_metafile_skia.h (right):

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:10: #include "base/logging.h"
nit: logging before scoped_ptr. Also, you don't need logging.h again in
pdf_metafile_skia.cc.

http://codereview.chromium.org/6499024/diff/23001/skia/config/SkUserConfig.h
File skia/config/SkUserConfig.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/config/SkUserConfig.h#...
skia/config/SkUserConfig.h:116: /*  To enable additional blitters (and
fontscaler code) to support separate
Is this change related to this CL?

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_skia.cc (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_skia.cc:11: #include
"third_party/skia/include/core/SkRegion.h"
nit: SkRegion before SKScalar.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_skia.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_skia.h:13: #include
"third_party/skia/include/core/SkRect.h"
I think you can forward declare?

[reed1, 2011-04-06 21:33:41]

http://codereview.chromium.org/6499024/diff/23001/skia/config/SkUserConfig.h
File skia/config/SkUserConfig.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/config/SkUserConfig.h#...
skia/config/SkUserConfig.h:119: //#define SK_SUPPORT_LCDTEXT
I second Lei's question. Commenting this out will break lots of linux
layouttests I think.

[vandebo (ex-Chrome), 2011-04-06 22:01:37]

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia.cc
File printing/pdf_metafile_skia.cc (right):

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:74: FinishPage();
On 2011/04/06 20:43:56, Lei Zhang wrote:
> nit: indentation

Done.

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:75: data_->pdf_doc_.emitPDF(&data_->pdf_stream_);
On 2011/04/06 20:43:56, Lei Zhang wrote:
> check emitPDF's return value?

Done.

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:85: if (dst_buffer_size > GetDataSize())
On 2011/04/06 20:43:56, Lei Zhang wrote:
> is this reversed?
> 
> dst_buffer_size = 10, GetDataSize() returns 5
> ==> Data fits in |dst_buffer|.

Done.

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia.h
File printing/pdf_metafile_skia.h (right):

http://codereview.chromium.org/6499024/diff/23001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:10: #include "base/logging.h"
On 2011/04/06 20:43:56, Lei Zhang wrote:
> nit: logging before scoped_ptr. Also, you don't need logging.h again in
> pdf_metafile_skia.cc.

Done.

http://codereview.chromium.org/6499024/diff/23001/skia/config/SkUserConfig.h
File skia/config/SkUserConfig.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/config/SkUserConfig.h#...
skia/config/SkUserConfig.h:116: /*  To enable additional blitters (and
fontscaler code) to support separate
On 2011/04/06 20:43:56, Lei Zhang wrote:
> Is this change related to this CL?

No, but it resyncs our copy of the config file with Skia's.  Looking at it more
closely, SK_SUPPORT_LCDTEXT is defined in skia.gyp, so it shouldn't be here. 
Removed.

However, new config options have been added since I last looked at this so I'll
add those.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/platform_device_li...
File skia/ext/platform_device_linux.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/platform_device_li...
skia/ext/platform_device_linux.h:27: virtual void endPlatformPaint();
On 2011/04/06 20:36:32, Alok Priyadarshi wrote:
> I have already added this function in one of the CLs pending review. That CL
> also makes all function calls start with CAPS.
> 
> http://codereview.chromium.org/6691067/

I see your CL got a LGTM.  I'll rebase after you commit it.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_skia.cc (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_skia.cc:11: #include
"third_party/skia/include/core/SkRegion.h"
On 2011/04/06 20:43:56, Lei Zhang wrote:
> nit: SkRegion before SKScalar.

Done.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_skia.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_skia.h:13: #include
"third_party/skia/include/core/SkRect.h"
On 2011/04/06 20:43:56, Lei Zhang wrote:
> I think you can forward declare?

Done.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_skia.h:31: class VectorPlatformDeviceSkia :
public PlatformDevice {
On 2011/04/06 20:36:32, Alok Priyadarshi wrote:
> This is consistent with other vector-devices and should be fine for now. But
we
> should start thinking about getting rid of subclassing. It would be great if
we
> could accomplish the same thing using skia meta-data.

I have to override all the methods to get them to an SkPDFDevice.  Once Chrome
accepts a SkDevice instead of a PlatformDevice, a lot of this goes away.  I
think setInitialTransform will get pushed down to SkPDFDevice, and the other
things will push up to the non inheriting device wrapper.

[vandebo (ex-Chrome), 2011-04-07 00:25:35]

There are a lot of reviewers listed.  Please speak up if you have a comment but
modulo further comments, I will wait for a LG from Lei and Mike.

http://codereview.chromium.org/6499024/diff/23001/skia/ext/platform_device_li...
File skia/ext/platform_device_linux.h (right):

http://codereview.chromium.org/6499024/diff/23001/skia/ext/platform_device_li...
skia/ext/platform_device_linux.h:27: virtual void endPlatformPaint();
On 2011/04/06 22:01:37, vandebo wrote:
> On 2011/04/06 20:36:32, Alok Priyadarshi wrote:
> > I have already added this function in one of the CLs pending review. That CL
> > also makes all function calls start with CAPS.
> > 
> > http://codereview.chromium.org/6691067/
> 
> I see your CL got a LGTM.  I'll rebase after you commit it.

Rebased.

[reed1, 2011-04-07 13:14:40]

LGTM but I defer to others.

[Lei Zhang, 2011-04-07 17:48:57]

LGTM

http://codereview.chromium.org/6499024/diff/32001/printing/pdf_metafile_skia.h
File printing/pdf_metafile_skia.h (right):

http://codereview.chromium.org/6499024/diff/32001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:16: #include <vector>
nit: no need for vector.

[vandebo (ex-Chrome), 2011-04-07 23:05:39]

Chris, ok the changes between patch set 13 & 14?

http://codereview.chromium.org/6499024/diff/32001/printing/pdf_metafile_skia.h
File printing/pdf_metafile_skia.h (right):

http://codereview.chromium.org/6499024/diff/32001/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:16: #include <vector>
On 2011/04/07 17:48:57, Lei Zhang wrote:
> nit: no need for vector.

Done.

[Chris Guillory, 2011-04-07 23:36:32]

On 2011/04/07 23:05:39, vandebo wrote:
> Chris, ok the changes between patch set 13 & 14?
LGTM
> 
> http://codereview.chromium.org/6499024/diff/32001/printing/pdf_metafile_skia.h
> File printing/pdf_metafile_skia.h (right):
> 
>
http://codereview.chromium.org/6499024/diff/32001/printing/pdf_metafile_skia....
> printing/pdf_metafile_skia.h:16: #include <vector>
> On 2011/04/07 17:48:57, Lei Zhang wrote:
> > nit: no need for vector.
> 
> Done.