Added autorotate flag in PDF rendering and wiring it through service-utility channel.

Added autorotate flag in PDF rendering and wiring it through service-utility channel.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104371

[Lei Zhang, 2011-10-05 20:36:19]

http://codereview.chromium.org/8146004/diff/18/chrome/service/service_utility...
File chrome/service/service_utility_process_host.cc (right):

http://codereview.chromium.org/8146004/diff/18/chrome/service/service_utility...
chrome/service/service_utility_process_host.cc:42: const gfx::Rect& render_area,
Why not make this a PdfRenderSettings as well?

http://codereview.chromium.org/8146004/diff/18/printing/pdf_render_settings.h
File printing/pdf_render_settings.h (right):

http://codereview.chromium.org/8146004/diff/18/printing/pdf_render_settings.h...
printing/pdf_render_settings.h:18: typedef Tuple3<gfx::Rect, int, bool>
PdfRenderSettings;
How do you feel about making this a struct? That way you can refer to
pdf_render_settings.dpi instead of pdf_render_settings.b in
chrome_content_utility_client.cc.

[gene1, 2011-10-05 20:49:45]

The initial idea was to make PdfRenderSettings a type used to pass
parameters between processes only. The advantage of using Tuple<>, it
already defines traits so no extra code is needed to serialize/deserialize
data.

I see you point of making it more convenient structure. Let me see if I can
make this change without much overhead.

On Wed, Oct 5, 2011 at 1:36 PM, <thestig@chromium.org> wrote:

>
> http://codereview.chromium.**org/8146004/diff/18/chrome/**
>
service/service_utility_**process_host.cc<http://codereview.chromium.org/8146004/diff/18/chrome/service/service_utility_process_host.cc>
> File chrome/service/service_**utility_process_host.cc (right):
>
> http://codereview.chromium.**org/8146004/diff/18/chrome/**
>
service/service_utility_**process_host.cc#newcode42<http://codereview.chromium.org/8146004/diff/18/chrome/service/service_utility_process_host.cc#newcode42>
> chrome/service/service_**utility_process_host.cc:42: const gfx::Rect&
> render_area,
> Why not make this a PdfRenderSettings as well?
>
> http://codereview.chromium.**org/8146004/diff/18/printing/**
>
pdf_render_settings.h<http://codereview.chromium.org/8146004/diff/18/printing/pdf_render_settings.h>
> File printing/pdf_render_settings.h (right):
>
> http://codereview.chromium.**org/8146004/diff/18/printing/**
>
pdf_render_settings.h#**newcode18<http://codereview.chromium.org/8146004/diff/18/printing/pdf_render_settings.h#newcode18>
> printing/pdf_render_settings.**h:18: typedef Tuple3<gfx::Rect, int, bool>
> PdfRenderSettings;
> How do you feel about making this a struct? That way you can refer to
> pdf_render_settings.dpi instead of pdf_render_settings.b in
> chrome_content_utility_client.**cc.
>
>
http://codereview.chromium.**org/8146004/<http://codereview.chromium.org/8146...
>

[gene1, 2011-10-05 23:27:17]

Done. Cloud you please take a look?

On Wed, Oct 5, 2011 at 1:49 PM, Gene Gutnik <gene@google.com> wrote:

> The initial idea was to make PdfRenderSettings a type used to pass
> parameters between processes only. The advantage of using Tuple<>, it
> already defines traits so no extra code is needed to serialize/deserialize
> data.
>
> I see you point of making it more convenient structure. Let me see if I can
> make this change without much overhead.
>
>
> On Wed, Oct 5, 2011 at 1:36 PM, <thestig@chromium.org> wrote:
>
>>
>> http://codereview.chromium.**org/8146004/diff/18/chrome/**
>>
service/service_utility_**process_host.cc<http://codereview.chromium.org/8146004/diff/18/chrome/service/service_utility_process_host.cc>
>> File chrome/service/service_**utility_process_host.cc (right):
>>
>> http://codereview.chromium.**org/8146004/diff/18/chrome/**
>>
service/service_utility_**process_host.cc#newcode42<http://codereview.chromium.org/8146004/diff/18/chrome/service/service_utility_process_host.cc#newcode42>
>> chrome/service/service_**utility_process_host.cc:42: const gfx::Rect&
>> render_area,
>> Why not make this a PdfRenderSettings as well?
>>
>> http://codereview.chromium.**org/8146004/diff/18/printing/**
>>
pdf_render_settings.h<http://codereview.chromium.org/8146004/diff/18/printing/pdf_render_settings.h>
>> File printing/pdf_render_settings.h (right):
>>
>> http://codereview.chromium.**org/8146004/diff/18/printing/**
>>
pdf_render_settings.h#**newcode18<http://codereview.chromium.org/8146004/diff/18/printing/pdf_render_settings.h#newcode18>
>> printing/pdf_render_settings.**h:18: typedef Tuple3<gfx::Rect, int, bool>
>> PdfRenderSettings;
>> How do you feel about making this a struct? That way you can refer to
>> pdf_render_settings.dpi instead of pdf_render_settings.b in
>> chrome_content_utility_client.**cc.
>>
>>
http://codereview.chromium.**org/8146004/<http://codereview.chromium.org/8146...
>>
>
>

[Lei Zhang, 2011-10-05 23:37:49]

LGTM