[webkit-reviews] review denied: [Bug 46152] [Chromium] Need setPrinting : [Attachment 108881] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 28 18:52:40 PDT 2011


MORITA Hajime <morrita at google.com> has denied Stephen Chenney
<schenney at chromium.org>'s request for review:
Bug 46152: [Chromium] Need setPrinting
https://bugs.webkit.org/show_bug.cgi?id=46152

Attachment 108881: Patch
https://bugs.webkit.org/attachment.cgi?id=108881&action=review

------- Additional Comments from MORITA Hajime <morrita at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=108881&action=review


> Source/WebKit/chromium/src/WebFrameImpl.h:178
> +    virtual WebCanvas* printPagesWithBoundaries(WebCanvas*, const WebSize&);


I guess this method can be part of DRT instead of WebKit API. In general,
test-specific API should be stayed in DRT if possible,
we have some of such unfortunate API in WebKit though.
Also, you need to cc fishd@ if you add new API to chromium WebKit interface.

> Source/WebKit/chromium/src/WebFrameImpl.h:192
> +    virtual WebString renderTreeAsText(bool showDebugInfo = false, bool
showPrintedText = false) const;

How about to define an enum instead of use bool? You can find two-sided enum
example in AssertMatchingEnums.cpp


More information about the webkit-reviews mailing list