Clean up windows printing workflow.

Clean up windows printing workflow.

BUG=none
TEST=printing works after code change.

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

[kmadhusu, 2011-01-19 03:03:03]

Renamed some variables in PrintPage() function. Refactored
UpdatePrintableSizeInPrintParameters() function.

[Lei Zhang, 2011-01-19 05:10:04]

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
File chrome/renderer/print_web_view_helper_win.cc (right):

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
chrome/renderer/print_web_view_helper_win.cc:81: int width = static_cast<int>(
width/height is suppose to be the same as size_x/size_y, right? I see different
values for the two:

width = size_x * scale_factor.

[kmadhusu, 2011-01-19 17:09:41]

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
File chrome/renderer/print_web_view_helper_win.cc (right):

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
chrome/renderer/print_web_view_helper_win.cc:81: int width = static_cast<int>(
On 2011/01/19 05:10:04, Lei Zhang wrote:
> width/height is suppose to be the same as size_x/size_y, right? I see
different
> values for the two:
> 
> width = size_x * scale_factor.

In the old code, we calculated |size_x| and |size_y| with 
|content_width_in_points| and |content_height_in_points| respectively.

In the new code, |width| and |height| are calculated using
|content_width_in_points| and |content_height_in_points| which are scaled based
on dpi values  (Scaling is done in UpdatePrintableSizeInPrintParameters
function). I made this change because all the values set to |page_params| are
scaled based on dpi. I verified the printed output page and I did not see any
difference.

[M-A Ruel, 2011-01-19 21:38:07]

Adding Stephen since he has more recent experience in that part.

[Lei Zhang, 2011-01-19 21:44:02]

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
chrome/renderer/print_web_view_helper.cc:404: double page_width =
content_width_in_points + margin_left_in_points +
nit: |page_width| -> |page_width_in_points|

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
File chrome/renderer/print_web_view_helper_win.cc (right):

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
chrome/renderer/print_web_view_helper_win.cc:81: int width = static_cast<int>(
On 2011/01/19 17:09:41, kmadhusu wrote:
> On 2011/01/19 05:10:04, Lei Zhang wrote:
> > width/height is suppose to be the same as size_x/size_y, right? I see
> different
> > values for the two:
> > 
> > width = size_x * scale_factor.
> 
> In the old code, we calculated |size_x| and |size_y| with 
> |content_width_in_points| and |content_height_in_points| respectively.
> 
> In the new code, |width| and |height| are calculated using
> |content_width_in_points| and |content_height_in_points| which are scaled
based
> on dpi values  (Scaling is done in UpdatePrintableSizeInPrintParameters
> function). I made this change because all the values set to |page_params| are
> scaled based on dpi. I verified the printed output page and I did not see any
> difference.  

(repeating our in person conversation for senorblanco's benefit)

The final output does looks the same, but I'm concerned we are creating a canvas
that's |scale_factor| times bigger in both width and height. How does that
affect memory usage? Does drawing to a canvas at a higher resolution make the
output look better?

[Stephen White, 2011-01-19 21:47:10]

I'm far from an expert on this code, but I can't see anything obviously wrong.

You break it, you bought it.  :)

[kmadhusu, 2011-01-19 23:25:11]

Reverted my changes. Please review and let me know your comments.

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
chrome/renderer/print_web_view_helper.cc:404: double page_width =
content_width_in_points + margin_left_in_points +
On 2011/01/19 21:44:03, Lei Zhang wrote:
> nit: |page_width| -> |page_width_in_points|

Done.

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
File chrome/renderer/print_web_view_helper_win.cc (right):

http://codereview.chromium.org/6374004/diff/1/chrome/renderer/print_web_view_...
chrome/renderer/print_web_view_helper_win.cc:81: int width = static_cast<int>(
On 2011/01/19 21:44:03, Lei Zhang wrote:
> On 2011/01/19 17:09:41, kmadhusu wrote:
> > On 2011/01/19 05:10:04, Lei Zhang wrote:
> > > width/height is suppose to be the same as size_x/size_y, right? I see
> > different
> > > values for the two:
> > > 
> > > width = size_x * scale_factor.
> > 
> > In the old code, we calculated |size_x| and |size_y| with 
> > |content_width_in_points| and |content_height_in_points| respectively.
> > 
> > In the new code, |width| and |height| are calculated using
> > |content_width_in_points| and |content_height_in_points| which are scaled
> based
> > on dpi values  (Scaling is done in UpdatePrintableSizeInPrintParameters
> > function). I made this change because all the values set to |page_params|
are
> > scaled based on dpi. I verified the printed output page and I did not see
any
> > difference.  
> 
> (repeating our in person conversation for senorblanco's benefit)
> 
> The final output does looks the same, but I'm concerned we are creating a
canvas
> that's |scale_factor| times bigger in both width and height. How does that
> affect memory usage? Does drawing to a canvas at a higher resolution make the
> output look better?

New code caused some random errors while handling alpha blend transparency. So
reverting my changes.

[vandebo (ex-Chrome), 2011-01-20 00:05:54]

LGTM

[Lei Zhang, 2011-01-20 00:20:09]

LGTM +1.