Rasterize page before printing.

Some printers has issue with metafiles produces from PDF so added switch and about:flag as workaround for affected users.

Rasterisation is implemented and RasterizeMetafile function.
It replace metafile with new one where content is only bitmap created by playback of original metafile.

FlattenTransparency replaced with full rasterisation.

Fixed rectangle returned by Emf::GetPageBounds.

Removed SkDevice::AlphaBlendUsed().

Removed return value from PrintWebViewHelper::RenderPage on windows.

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

[Vitaly Buka (NO REVIEWS), 2012-08-20 06:06:35]

Please review.

[Vitaly Buka (NO REVIEWS), 2012-08-20 16:41:51]

https://chromiumcodereview.appspot.com/10836330/diff/5/chrome/renderer/print_...
File chrome/renderer/print_web_view_helper_win.cc (right):

https://chromiumcodereview.appspot.com/10836330/diff/5/chrome/renderer/print_...
chrome/renderer/print_web_view_helper_win.cc:419: if (platform_device &&
platform_device->AlphaBlendUsed()) {
I found that FlattenTransparency use low-res bitmap.
May be for code simplicity just use FlattenAll here too?

[Albert Bodenhamer, 2012-08-20 18:18:10]

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
File chrome/browser/printing/printing_message_filter.cc (right):

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
chrome/browser/printing/printing_message_filter.cc:83: params->raster_type =
PrintMsg_Print_Params::RASTER_PLUGINS;
Do we need different modes?  Is there a need to have anything more than
everything or nothing?

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
chrome/browser/printing/printing_message_filter.cc:87: if
(base::StringToInt(cmdline->GetSwitchValueASCII(switches::kPrintRasterSize),
Line len

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
chrome/browser/printing/printing_message_filter.cc:90: params->raster_size =
raster_size;
I'm reluctant to make the size settable.  Can we calculate it somehow?

http://codereview.chromium.org/10836330/diff/5/chrome/common/chrome_switches.cc
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/10836330/diff/5/chrome/common/chrome_switches....
chrome/common/chrome_switches.cc:1069: // Enable conversion from vector to
raster for any page.
I think we should probably also expose these in about:flags.

http://codereview.chromium.org/10836330/diff/5/chrome/renderer/print_web_view...
File chrome/renderer/print_web_view_helper_win.cc (right):

http://codereview.chromium.org/10836330/diff/5/chrome/renderer/print_web_view...
chrome/renderer/print_web_view_helper_win.cc:419: if (platform_device &&
platform_device->AlphaBlendUsed()) {
On 2012/08/20 16:41:51, Vitaly Buka wrote:
> I found that FlattenTransparency use low-res bitmap.
> May be for code simplicity just use FlattenAll here too?

I'm ok with that.  Just unify it all.

[Lei Zhang, 2012-08-21 08:09:14]

What's the long term plan for the raster flags?

[Albert Bodenhamer, 2012-08-21 16:52:57]

If this gets around the crashing driver issues we're seeing the next step
is to build a black list of printers (similar to the GPU black list) and
trigger this flow automatically.  Once that's working well we'll remove the
flag.


On Tue, Aug 21, 2012 at 1:09 AM, <thestig@chromium.org> wrote:

> What's the long term plan for the raster flags?
>
>
https://chromiumcodereview.**appspot.com/10836330/<https://chromiumcodereview...
>



-- 
Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com>
org

[Vitaly Buka (NO REVIEWS), 2012-08-21 19:40:54]

Please review again.
Main difference with previouse patch is that restarization moved out of renderer
process.

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
File chrome/browser/printing/printing_message_filter.cc (right):

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
chrome/browser/printing/printing_message_filter.cc:83: params->raster_type =
PrintMsg_Print_Params::RASTER_PLUGINS;
On 2012/08/20 18:18:10, Albert Bodenhamer wrote:
> Do we need different modes?  Is there a need to have anything more than
> everything or nothing?

Done.

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
chrome/browser/printing/printing_message_filter.cc:87: if
(base::StringToInt(cmdline->GetSwitchValueASCII(switches::kPrintRasterSize),
On 2012/08/20 18:18:10, Albert Bodenhamer wrote:
> Line len

Done.

http://codereview.chromium.org/10836330/diff/5/chrome/browser/printing/printi...
chrome/browser/printing/printing_message_filter.cc:90: params->raster_size =
raster_size;
On 2012/08/20 18:18:10, Albert Bodenhamer wrote:
> I'm reluctant to make the size settable.  Can we calculate it somehow?

Done.

http://codereview.chromium.org/10836330/diff/5/chrome/common/chrome_switches.cc
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/10836330/diff/5/chrome/common/chrome_switches....
chrome/common/chrome_switches.cc:1069: // Enable conversion from vector to
raster for any page.
On 2012/08/20 18:18:10, Albert Bodenhamer wrote:
> I think we should probably also expose these in about:flags.

Done.

http://codereview.chromium.org/10836330/diff/5/chrome/renderer/print_web_view...
File chrome/renderer/print_web_view_helper_win.cc (right):

http://codereview.chromium.org/10836330/diff/5/chrome/renderer/print_web_view...
chrome/renderer/print_web_view_helper_win.cc:419: if (platform_device &&
platform_device->AlphaBlendUsed()) {
On 2012/08/20 18:18:10, Albert Bodenhamer wrote:
> On 2012/08/20 16:41:51, Vitaly Buka wrote:
> > I found that FlattenTransparency use low-res bitmap.
> > May be for code simplicity just use FlattenAll here too?
> 
> I'm ok with that.  Just unify it all.

Done.

[Lei Zhang, 2012-08-21 20:18:32]

Please make the flag truly Windows-only.

What's the motivation for moving the rasterizing from renderer to browser?

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
chrome/app/generated_resources.grd:5364: +      <message
name="IDS_FLAGS_PRINT_RASTER_NAME" desc="Name of 'Print raster' flag.">
Put this inside a <if expr="is_win"> ?

http://codereview.chromium.org/10836330/diff/2008/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/browser/about_flags....
chrome/browser/about_flags.cc:226: {
Ditto, inside #if defined(OS_WIN)

http://codereview.chromium.org/10836330/diff/2008/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:289: extern const char kPrintRaster[];
and move this down towards line 433.

[Vitaly Buka (NO REVIEWS), 2012-08-21 20:39:55]

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
chrome/app/generated_resources.grd:5364: +      <message
name="IDS_FLAGS_PRINT_RASTER_NAME" desc="Name of 'Print raster' flag.">
Done. But it seems uncommon.
On 2012/08/21 20:18:33, Lei Zhang wrote:
> Put this inside a <if expr="is_win"> ?

http://codereview.chromium.org/10836330/diff/2008/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/browser/about_flags....
chrome/browser/about_flags.cc:226: {
On 2012/08/21 20:18:33, Lei Zhang wrote:
> Ditto, inside #if defined(OS_WIN)

Done.

http://codereview.chromium.org/10836330/diff/2008/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:289: extern const char kPrintRaster[];
On 2012/08/21 20:18:33, Lei Zhang wrote:
> and move this down towards line 433.

Done.

[Lei Zhang, 2012-08-21 20:51:00]

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
chrome/app/generated_resources.grd:5364: +      <message
name="IDS_FLAGS_PRINT_RASTER_NAME" desc="Name of 'Print raster' flag.">
On 2012/08/21 20:39:55, Vitaly Buka wrote:
> Done. But it seems uncommon.
> On 2012/08/21 20:18:33, Lei Zhang wrote:
> > Put this inside a <if expr="is_win"> ?
> 

Most flags are x-platform. is_os tags probably should be used a bit more to help
minimize the amount of useless data we ship.

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h
File printing/emf_win.h (right):

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h#newcode192
printing/emf_win.h:192: // Returns true if metafile contains alpha blend.
Why not make these two functions Emf methods?

http://codereview.chromium.org/10836330/diff/6003/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_emf_win.h (right):

http://codereview.chromium.org/10836330/diff/6003/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_emf_win.h:128: bool alpha_blend_used_;
You can remove |alpha_blend_used_| altogether since you removed
AlphaBlendUsed().

[Vitaly Buka (NO REVIEWS), 2012-08-21 21:14:26]

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10836330/diff/2008/chrome/app/generated_resour...
chrome/app/generated_resources.grd:5364: +      <message
name="IDS_FLAGS_PRINT_RASTER_NAME" desc="Name of 'Print raster' flag.">
I just noticed there are several windows only flags and I see no is_win in grd
for flags. However those flags are disabled in cc-files.
On 2012/08/21 20:51:01, Lei Zhang wrote:
> On 2012/08/21 20:39:55, Vitaly Buka wrote:
> > Done. But it seems uncommon.
> > On 2012/08/21 20:18:33, Lei Zhang wrote:
> > > Put this inside a <if expr="is_win"> ?
> > 
> 
> Most flags are x-platform. is_os tags probably should be used a bit more to
help
> minimize the amount of useless data we ship.

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h
File printing/emf_win.h (right):

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h#newcode192
printing/emf_win.h:192: // Returns true if metafile contains alpha blend.
It's matter of style, I can switch to methods if you like.
Some people prefer plain functions if implementations does not need access to
private stuff.
I am o.k. with either way.

On 2012/08/21 20:51:01, Lei Zhang wrote:
> Why not make these two functions Emf methods?

http://codereview.chromium.org/10836330/diff/6003/skia/ext/vector_platform_de...
File skia/ext/vector_platform_device_emf_win.h (right):

http://codereview.chromium.org/10836330/diff/6003/skia/ext/vector_platform_de...
skia/ext/vector_platform_device_emf_win.h:128: bool alpha_blend_used_;
Sure. Missed this one.
On 2012/08/21 20:51:01, Lei Zhang wrote:
> You can remove |alpha_blend_used_| altogether since you removed
> AlphaBlendUsed().

[Lei Zhang, 2012-08-21 21:38:37]

lgtm

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h
File printing/emf_win.h (right):

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h#newcode192
printing/emf_win.h:192: // Returns true if metafile contains alpha blend.
Maybe make these methods just so you have less symbols to export? I don't have a
strong opinion either.

On 2012/08/21 21:14:27, Vitaly Buka wrote:
> It's matter of style, I can switch to methods if you like.
> Some people prefer plain functions if implementations does not need access to
> private stuff.
> I am o.k. with either way.
> 
> On 2012/08/21 20:51:01, Lei Zhang wrote:
> > Why not make these two functions Emf methods?
>

[gene, 2012-08-21 22:10:50]

lgtm

http://codereview.chromium.org/10836330/diff/10031/chrome/browser/printing/pr...
File chrome/browser/printing/print_view_manager.cc (right):

http://codereview.chromium.org/10836330/diff/10031/chrome/browser/printing/pr...
chrome/browser/printing/print_view_manager.cc:257: int raster_size =
std::min(params.page_size.GetArea(), 16*1024*1024);
nit: Make 16*1024*1024 a const

[Vitaly Buka (NO REVIEWS), 2012-08-21 22:17:30]

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h
File printing/emf_win.h (right):

http://codereview.chromium.org/10836330/diff/6003/printing/emf_win.h#newcode192
printing/emf_win.h:192: // Returns true if metafile contains alpha blend.
On 2012/08/21 21:38:37, Lei Zhang wrote:
> Maybe make these methods just so you have less symbols to export? I don't have
a
> strong opinion either.
> 
> On 2012/08/21 21:14:27, Vitaly Buka wrote:
> > It's matter of style, I can switch to methods if you like.
> > Some people prefer plain functions if implementations does not need access
to
> > private stuff.
> > I am o.k. with either way.
> > 
> > On 2012/08/21 20:51:01, Lei Zhang wrote:
> > > Why not make these two functions Emf methods?
> > 
> 

Done.

http://codereview.chromium.org/10836330/diff/10031/chrome/browser/printing/pr...
File chrome/browser/printing/print_view_manager.cc (right):

http://codereview.chromium.org/10836330/diff/10031/chrome/browser/printing/pr...
chrome/browser/printing/print_view_manager.cc:257: int raster_size =
std::min(params.page_size.GetArea(), 16*1024*1024);
On 2012/08/21 22:10:50, gene wrote:
> nit: Make 16*1024*1024 a const

Done.

[commit-bot: I haz the power, 2012-08-21 22:18:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/10836330/12027

[Albert Bodenhamer, 2012-08-21 22:19:02]

lgtm

[commit-bot: I haz the power, 2012-08-22 00:10:10]

Change committed as 152681