34743 2010-02-09 02:42:56 -0800 [Chromium] Implement WebFrameImpl::numberOfPages 2010-02-16 01:01:40 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC All RESOLVED INVALID P2 Normal --- 1 hamaji webkit-unassigned abarth fishd hayato yuzo oldest_to_newest 188942 0 hamaji 2010-02-09 02:42:56 -0800 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. 188945 1 48398 hamaji 2010-02-09 02:49:06 -0800 Created attachment 48398 Patch v1 189118 2 48398 abarth 2010-02-09 12:16:05 -0800 Comment on attachment 48398 Patch v1 LGTM, but fishd should look at his change because it touches the WebKit API. 189132 3 48398 fishd 2010-02-09 12:46:14 -0800 Comment on attachment 48398 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 189343 4 hamaji 2010-02-10 06:03:03 -0800 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. 48398 2010-02-09 02:49:06 -0800 2010-02-10 06:03:18 -0800 Patch v1 bug-34743-20100209024904.patch text/plain 2709 hamaji ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDk3ZDE5MDIuLmJiNzQ5MDAgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE1IEBACisyMDEwLTAyLTA5ICBTaGluaWNoaXJvIEhhbWFqaSAgPGhhbWFqaUBjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW0No cm9taXVtXSBJbXBsZW1lbnQgV2ViRnJhbWVJbXBsOjpudW1iZXJPZlBhZ2VzCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNDc0MworCisgICAgICAgICog cHVibGljL1dlYkZyYW1lLmg6CisgICAgICAgICogc3JjL1dlYkZyYW1lSW1wbC5jcHA6CisgICAg ICAgIChXZWJLaXQ6OldlYkZyYW1lSW1wbDo6bnVtYmVyT2ZQYWdlcyk6CisgICAgICAgICogc3Jj L1dlYkZyYW1lSW1wbC5oOgorCiAyMDEwLTAyLTA5ICBBdmkgRHJpc3NtYW4gIDxhdmlAY2hyb21p dW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhdmlkIExldmluLgpkaWZmIC0tZ2l0IGEv V2ViS2l0L2Nocm9taXVtL3B1YmxpYy9XZWJGcmFtZS5oIGIvV2ViS2l0L2Nocm9taXVtL3B1Ymxp Yy9XZWJGcmFtZS5oCmluZGV4IGE1NmU2Y2IuLmM1ZmFiNzcgMTAwNjQ0Ci0tLSBhL1dlYktpdC9j aHJvbWl1bS9wdWJsaWMvV2ViRnJhbWUuaAorKysgYi9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dl YkZyYW1lLmgKQEAgLTQ5MCw2ICs0OTAsMTEgQEAgcHVibGljOgogICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICBmbG9hdCBwYWdlV2lkdGhJblBpeGVscywKICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgZmxvYXQgcGFnZUhlaWdodEluUGl4ZWxz KSBjb25zdCA9IDA7CiAKKyAgICAvLyBSZXR1cm5zIHRoZSBudW1iZXIgb2YgcGFnZXMgdG8gYmUg cHJpbnRlZC4KKyAgICAvLyBUaGlzIG1ldGhvZCBpcyB1c2VkIHRvIHN1cHBvcnQgbGF5b3V0IHRl c3RzLgorICAgIHZpcnR1YWwgaW50IG51bWJlck9mUGFnZXMoZmxvYXQgcGFnZVdpZHRoSW5QaXhl bHMsCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBmbG9hdCBwYWdlSGVpZ2h0SW5QaXhl bHMpIGNvbnN0ID0gMDsKKwogcHJvdGVjdGVkOgogICAgIH5XZWJGcmFtZSgpIHsgfQogfTsKZGlm ZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcCBiL1dlYktpdC9j aHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAppbmRleCAyZjkxMWYxLi5kNjZjZWE0IDEwMDY0 NAotLS0gYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1lSW1wbC5jcHAKKysrIGIvV2ViS2l0 L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuY3BwCkBAIC0xNTMzLDYgKzE1MzMsMTYgQEAgaW50 IFdlYkZyYW1lSW1wbDo6cGFnZU51bWJlckZvckVsZW1lbnRCeUlkKGNvbnN0IFdlYlN0cmluZyYg aWQsCiAgICAgcmV0dXJuIFByaW50Q29udGV4dDo6cGFnZU51bWJlckZvckVsZW1lbnQoZWxlbWVu dCwgcGFnZVNpemUpOwogfQogCitpbnQgV2ViRnJhbWVJbXBsOjpudW1iZXJPZlBhZ2VzKGZsb2F0 IHBhZ2VXaWR0aEluUGl4ZWxzLAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBmbG9h dCBwYWdlSGVpZ2h0SW5QaXhlbHMpIGNvbnN0Cit7CisgICAgaWYgKCFtX2ZyYW1lKQorICAgICAg ICByZXR1cm4gLTE7CisKKyAgICBGbG9hdFNpemUgcGFnZVNpemUocGFnZVdpZHRoSW5QaXhlbHMs IHBhZ2VIZWlnaHRJblBpeGVscyk7CisgICAgcmV0dXJuIFByaW50Q29udGV4dDo6bnVtYmVyT2ZQ YWdlcyhtX2ZyYW1lLCBwYWdlU2l6ZSk7Cit9CisKIC8vIFdlYkZyYW1lSW1wbCBwdWJsaWMgLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAK IFBhc3NSZWZQdHI8V2ViRnJhbWVJbXBsPiBXZWJGcmFtZUltcGw6OmNyZWF0ZShXZWJGcmFtZUNs aWVudCogY2xpZW50KQpkaWZmIC0tZ2l0IGEvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUlt cGwuaCBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmgKaW5kZXggY2NiYTZkNC4u NDU1NzNiNyAxMDA2NDQKLS0tIGEvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuaAor KysgYi9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1lSW1wbC5oCkBAIC0xNjcsNiArMTY3LDgg QEAgcHVibGljOgogICAgIHZpcnR1YWwgaW50IHBhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZChjb25z dCBXZWJTdHJpbmcmIGlkLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICBmbG9hdCBwYWdlV2lkdGhJblBpeGVscywKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgZmxvYXQgcGFnZUhlaWdodEluUGl4ZWxzKSBjb25zdDsKKyAgICB2aXJ0dWFs IGludCBudW1iZXJPZlBhZ2VzKGZsb2F0IHBhZ2VXaWR0aEluUGl4ZWxzLAorICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgZmxvYXQgcGFnZUhlaWdodEluUGl4ZWxzKSBjb25zdDsKIAogICAg IHN0YXRpYyBQYXNzUmVmUHRyPFdlYkZyYW1lSW1wbD4gY3JlYXRlKFdlYkZyYW1lQ2xpZW50KiBj bGllbnQpOwogICAgIH5XZWJGcmFtZUltcGwoKTsK