Bug 34743 - [Chromium] Implement WebFrameImpl::numberOfPages
Summary: [Chromium] Implement WebFrameImpl::numberOfPages
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-09 02:42 PST by Shinichiro Hamaji
Modified: 2010-02-16 01:01 PST (History)
4 users (show)

See Also:


Attachments
Patch v1 (2.65 KB, patch)
2010-02-09 02:49 PST, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinichiro Hamaji 2010-02-09 02:42:56 PST
This is necessary to implement layoutTestController.numberOfPages in
test_shell. This API was added in http://trac.webkit.org/changeset/54533

Note that this is similar to Bug 34471, but a different API.
Comment 1 Shinichiro Hamaji 2010-02-09 02:49:06 PST
Created attachment 48398 [details]
Patch v1
Comment 2 Adam Barth 2010-02-09 12:16:05 PST
Comment on attachment 48398 [details]
Patch v1

LGTM, but fishd should look at his change because it touches the WebKit API.
Comment 3 Darin Fisher (:fishd, Google) 2010-02-09 12:46:14 PST
Comment on attachment 48398 [details]
Patch v1

> +++ b/WebKit/chromium/public/WebFrame.h
> @@ -490,6 +490,11 @@ public:
>                                           float pageWidthInPixels,
>                                           float pageHeightInPixels) const = 0;
>  
> +    // Returns the number of pages to be printed.
> +    // This method is used to support layout tests.
> +    virtual int numberOfPages(float pageWidthInPixels,
> +                              float pageHeightInPixels) const = 0;

Since this is about printing, should this be moved into the "printing"
section of WebFrame?  I realize that you probably stuck this nearby
pageNumberForElementById intentionally, and that was probably put next
to counterValueForElementById because they are only used by layout tests.

Is it necessary to call printBegin before using this function?  Would
it be advantageous to do so?

If we wanted to have a print preview system, perhaps we'd want to know
how many pages will be generated, so this API might be nice outside the
context of layout tests.

At any rate, it seems like we should either make these features of the
printing API, or we should create a new section in WebFrame that is
dedicated to providing utilities for layout tests.

One more question:  is it necessary to be able to specify pageWidth
and pageHeight in fractions of a pixel?  Do these need to be floats,
or can they be integers?

-Darin
Comment 4 Shinichiro Hamaji 2010-02-10 06:03:03 PST
Thanks for your review! I checked printBegin and found printBegin and printEnd are doing the same stuff as PrintContext::numberOfPages (printBegin returns the number of pages). So, I think we don't need to add a new API. We can implement layoutTestController.numberOfPages only with printBegin and printEnd. I'll close this bug.

> At any rate, it seems like we should either make these features of the
> printing API, or we should create a new section in WebFrame that is
> dedicated to providing utilities for layout tests.

It seems there are 3 functions for layout tests (renderTreeAsText, counterValueForElementById, and pageNumberForElementById). I'll open another bug (or re-title this bug) to create a section for them. By the way, both counterValueForElementById and pageNumberForElementById were putted by me and you are right, I put them here because I wanted to put them nearby renderTreeAsText.

> One more question:  is it necessary to be able to specify pageWidth
> and pageHeight in fractions of a pixel?  Do these need to be floats,
> or can they be integers?

I made them float just because PrintContext takes float parameters. As I don't think we'll want to use float sized pages from layout tests, printBegin, which takes WebSize (a struct which has two integers), is sufficient to implement layoutTestController.numberOfPages.