Issue 12087005: Async frame preparation for Preview with support if selection.

Issue 12087005: Async frame preparation for Preview with support if selection. (Closed)

Created:
7 years, 11 months ago by Vitaly Buka (NO REVIEWS)

Modified:
7 years, 10 months ago

Reviewers:
Lei Zhang

CC:
chromium-reviews, darin-cc_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Async frame preparation for Preview with support if selection. Browser test for selection. BUG=83098 NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=179316

Patch Set 5 : #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+190 lines, -60 lines)			Patch
M	chrome/renderer/chrome_mock_render_thread.cc	View		1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/renderer/mock_printer.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/renderer/mock_printer.cc	View		4 chunks	+4 lines, -0 lines	0 comments	Download
M	chrome/renderer/printing/print_web_view_helper.h	View	1 2 3 4	5 chunks	+25 lines, -7 lines	0 comments	Download
M	chrome/renderer/printing/print_web_view_helper.cc	View	1 2 3	18 chunks	+75 lines, -38 lines	0 comments	Download
M	chrome/renderer/printing/print_web_view_helper_browsertest.cc	View		14 chunks	+63 lines, -10 lines	0 comments	Download
M	chrome/renderer/printing/print_web_view_helper_linux.cc	View	1 2	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/renderer/printing/print_web_view_helper_mac.mm	View	1 2	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/renderer/printing/print_web_view_helper_win.cc	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	printing/print_job_constants.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	printing/print_job_constants.cc	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	printing/printing_context.cc	View		1 chunk	+5 lines, -0 lines	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

Lei Zhang

https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/print_web_view_helper.cc File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/print_web_view_helper.cc#newcode947 chrome/renderer/printing/print_web_view_helper.cc:947: const std::vector<int>& pages = print_pages_params_->pages; not used? https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/print_web_view_helper.cc#newcode1794 chrome/renderer/printing/print_web_view_helper.cc:1794: ...

7 years, 11 months ago (2013-01-26 08:40:36 UTC) #2

Vitaly Buka (NO REVIEWS)

7 years, 11 months ago (2013-01-26 10:29:30 UTC) #3

Vitaly Buka (NO REVIEWS)

reverted total_page_count() Added todo to fix context life-cycle On 2013/01/26 10:29:30, Vitaly Buka wrote: > ...

7 years, 10 months ago (2013-01-29 01:32:32 UTC) #4

reverted total_page_count()
Added todo to fix context life-cycle

On 2013/01/26 10:29:30, Vitaly Buka wrote:
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> File chrome/renderer/printing/print_web_view_helper.cc (right):
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> chrome/renderer/printing/print_web_view_helper.cc:947: const std::vector<int>&
> pages = print_pages_params_->pages;
> On 2013/01/26 08:40:36, Lei Zhang wrote:
> > not used?
> 
> Done.
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> chrome/renderer/printing/print_web_view_helper.cc:1794: bool
> PrintWebViewHelper::PrintPreviewContext::HasSelection() const {
> On 2013/01/26 08:40:36, Lei Zhang wrote:
> > Is this going to be used in a future CL?
> 
> 
> I'll submit this with that changes
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> chrome/renderer/printing/print_web_view_helper.cc:1796: // The only kind of
node
> we can print right now is a PDF node.
> or bad merge
> On 2013/01/26 08:40:36, Lei Zhang wrote:
> > copy + paste error?
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> chrome/renderer/printing/print_web_view_helper.cc:1850: return
> prep_frame_view_->GetExpectedPageCount();
> You are correct, bat that's confusing.
> Changed to avoid re-use of old context.
> 
> On 2013/01/26 08:40:36, Lei Zhang wrote:
> > Not sure if this works. In PrintWebViewHelper::OnPrintPreview(), the first
> thing
> > it does is call PrintPreviewContext::OnPrintPreview(), which calls
> > PrintPreviewContext::ClearContext() to reset this |prep_frame_view_|.
> > PrintWebViewHelper::OnPrintPreview() may then call total_page_count() before
> > CreatePreviewDocument().
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> File chrome/renderer/printing/print_web_view_helper.h (right):
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> chrome/renderer/printing/print_web_view_helper.h:361: bool
> CreatePreviewDocument(PrepareFrameAndViewForPrint* prepared_frame,
> On 2013/01/26 08:40:36, Lei Zhang wrote:
> > Mention this takes ownership of |prepared_frame|.
> 
> Done.
> 
>
https://codereview.chromium.org/12087005/diff/9001/chrome/renderer/printing/p...
> chrome/renderer/printing/print_web_view_helper.h:393: WebKit::WebFrame*
> prepared_frame();
> On 2013/01/26 08:40:36, Lei Zhang wrote:
> > Please mention the difference between the source and prepared versions.
> 
> Done.

Lei Zhang

lgtm https://chromiumcodereview.appspot.com/12087005/diff/4010/chrome/renderer/printing/print_web_view_helper.h File chrome/renderer/printing/print_web_view_helper.h (right): https://chromiumcodereview.appspot.com/12087005/diff/4010/chrome/renderer/printing/print_web_view_helper.h#newcode348 chrome/renderer/printing/print_web_view_helper.h:348: // use. Now it's interaction with veriouse messages ...

7 years, 10 months ago (2013-01-29 01:40:47 UTC) #5

Vitaly Buka (NO REVIEWS)

https://codereview.chromium.org/12087005/diff/4010/chrome/renderer/printing/print_web_view_helper.h File chrome/renderer/printing/print_web_view_helper.h (right): https://codereview.chromium.org/12087005/diff/4010/chrome/renderer/printing/print_web_view_helper.h#newcode348 chrome/renderer/printing/print_web_view_helper.h:348: // use. Now it's interaction with veriouse messages is ...

7 years, 10 months ago (2013-01-29 05:14:57 UTC) #6

commit-bot: I haz the power

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

7 years, 10 months ago (2013-01-29 05:15:12 UTC) #7

commit-bot: I haz the power

Sorry for I got bad news for ya. Compile failed with a clobber build on ...

7 years, 10 months ago (2013-01-29 05:36:36 UTC) #8

commit-bot: I haz the power

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

7 years, 10 months ago (2013-01-29 08:47:58 UTC) #9

Message was sent while issue was closed.

Change committed as 179316

Expand Messages | Collapse Messages