Add Print Selection support to Chrome. This change is fairly involved since ...

Add Print Selection support to Chrome.  This change is fairly involved since this means that the printing is done async instead of the fully synchronous mode the normal full page printing is.
This means we create an in memory copy of the selected text for printing.
This is the next step to move to fully async printing with print frame support.
This change also removes the print on demand functionality that was no longer used.

BUG=http://crbug.com/1682
TEST=The print dialog on Windows now contains an option to print selection only.  Test that with various pages and various selections.



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

[Sverrir, 2009-06-12 22:16:08]

This change is NOT ready for submitting but I wanted an early feedback.
There are a few TODO's in there that I plan to fix before submitting.  They
mainly have to do with teardown.

This works as is - you can print a selection with the above caveat (meaning
asserts no shutdown etc).

[M-A Ruel, 2009-06-15 16:43:36]

Seems to fix the issue with waiting for the page to be finished loading before
printing?

lgtm

http://codereview.chromium.org/125082/diff/1/6
File chrome/browser/printing/print_view_manager.cc (right):

http://codereview.chromium.org/125082/diff/1/6#newcode64
Line 64: // TODO(sverrir): fix: || cookie != document->cookie()) {
Can you add an explanation on why?

http://codereview.chromium.org/125082/diff/1/4
File chrome/browser/printing/printed_document.h (left):

http://codereview.chromium.org/125082/diff/1/4#oldcode178
Line 178: void PrintPage_ThreadJump(int page_number);
Yay!

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

http://codereview.chromium.org/125082/diff/1/13#newcode201
Line 201: print_web_view_->GetMainFrame()->LoadRequest(request.get());
Awesome

http://codereview.chromium.org/125082/diff/1/14
File chrome/renderer/print_web_view_helper.h (right):

http://codereview.chromium.org/125082/diff/1/14#newcode49
Line 49: bool CopyAndPrint(const ViewMsg_PrintPages_Params& params, WebFrame*
web_frame);
80 cols

http://codereview.chromium.org/125082/diff/1/12
File chrome/renderer/render_view.h (right):

http://codereview.chromium.org/125082/diff/1/12#newcode804
Line 804: // Need for printing.  Note that this object is constructed when
printing for
Can you replace "Need for printing" for something clearer?

[Sverrir, 2009-06-17 16:34:58]

Uploaded a new patch.  Added the IPC to notify when printing is done and do
proper cleanup.  This seems to be working fine from end to end but I will do
some more testing today to make sure.
I want to add a method that does a messagebox from the renderer when printing
fails (with some boring text like "Sorry but it seems like printing failed"
since we don't really know what failed and why).  But I will do that in a
separate change since this requires a new translated string.

http://codereview.chromium.org/125082/diff/1/6
File chrome/browser/printing/print_view_manager.cc (right):

http://codereview.chromium.org/125082/diff/1/6#newcode64
Line 64: // TODO(sverrir): fix: || cookie != document->cookie()) {
On 2009/06/15 16:43:36, Marc-Antoine Ruel wrote:
> Can you add an explanation on why?

Turns out this was unnecessary - reverted

http://codereview.chromium.org/125082/diff/1/14
File chrome/renderer/print_web_view_helper.h (right):

http://codereview.chromium.org/125082/diff/1/14#newcode49
Line 49: bool CopyAndPrint(const ViewMsg_PrintPages_Params& params, WebFrame*
web_frame);
On 2009/06/15 16:43:36, Marc-Antoine Ruel wrote:
> 80 cols

Done.

http://codereview.chromium.org/125082/diff/1/12
File chrome/renderer/render_view.h (right):

http://codereview.chromium.org/125082/diff/1/12#newcode804
Line 804: // Need for printing.  Note that this object is constructed when
printing for
On 2009/06/15 16:43:36, Marc-Antoine Ruel wrote:
> Can you replace "Need for printing" for something clearer?

Done.

[M-A Ruel, 2009-06-17 19:07:41]

http://codereview.chromium.org/125082/diff/1005/1024
File chrome/common/render_messages.h (right):

http://codereview.chromium.org/125082/diff/1005/1024#newcode354
Line 354: struct ViewMsg_PrintingDone_Params {
It's not used by ViewMsg_PrintingDone so please remove.

http://codereview.chromium.org/125082/diff/1005/1017
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/125082/diff/1005/1017#newcode144
Line 144: CopyAndPrint(print_settings, frame);
Add a // TODO: We will eventually always do a copy.

http://codereview.chromium.org/125082/diff/1005/1017#newcode194
Line 194: GURL url(url_str);
GURL url("data:text/html;charset=utf-8,"); doesn't work?

http://codereview.chromium.org/125082/diff/1005/1019
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/125082/diff/1005/1019#newcode449
Line 449: // TODO(maruel):  bug 1123882 Show some kind of notification.
Can you use an external bug instead?

[Sverrir, 2009-06-17 21:11:43]

I moved the error handling to one place in PrintWebViewHelper and renamed the
member variable in RenderView to print_helper_.

http://codereview.chromium.org/125082/diff/1005/1024
File chrome/common/render_messages.h (right):

http://codereview.chromium.org/125082/diff/1005/1024#newcode354
Line 354: struct ViewMsg_PrintingDone_Params {
On 2009/06/17 19:07:41, Marc-Antoine Ruel wrote:
> It's not used by ViewMsg_PrintingDone so please remove.

Done.

http://codereview.chromium.org/125082/diff/1005/1017
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/125082/diff/1005/1017#newcode144
Line 144: CopyAndPrint(print_settings, frame);
On 2009/06/17 19:07:41, Marc-Antoine Ruel wrote:
> Add a // TODO: We will eventually always do a copy.

Done.

http://codereview.chromium.org/125082/diff/1005/1017#newcode194
Line 194: GURL url(url_str);
On 2009/06/17 19:07:41, Marc-Antoine Ruel wrote:
> GURL url("data:text/html;charset=utf-8,"); doesn't work?
Sure, but I need to add the full HTML of the page as well.  There is no simple
way to prevent creation of 3 string objects (either explicitly as here or
through temporaries).

http://codereview.chromium.org/125082/diff/1005/1019
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/125082/diff/1005/1019#newcode449
Line 449: // TODO(maruel):  bug 1123882 Show some kind of notification.
On 2009/06/17 19:07:41, Marc-Antoine Ruel wrote:
> Can you use an external bug instead?

Done.

[M-A Ruel, 2009-06-17 21:14:51]

lgtm

[Sverrir, 2009-06-18 21:55:07]

Can you take a quick look again.  I found a regression in print ranges so I
reverted a change in page_number and changed it in win_printing_context instead.

Also I fixed a crash in glue because of the print selection usage of WebView is
non-standard.