[webkit-reviews] review denied: [Bug 36159] Move code related with printing defined in WebFrame.mm to WebCore::PrintContext : [Attachment 50880] refactor-computePageRects-fix-chromium-build

[bugzilla-daemon at webkit.org]

Shinichiro Hamaji <hamaji at chromium.org> has denied Hayato Ito
<hayato at chromium.org>'s request for review:
Bug 36159: Move code related with printing defined in WebFrame.mm to
WebCore::PrintContext
https://bugs.webkit.org/show_bug.cgi?id=36159

Attachment 50880: refactor-computePageRects-fix-chromium-build
https://bugs.webkit.org/attachment.cgi?id=50880&action=review

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
Basically, looks good. r- for a few style nitpicks.

> -    RenderView* root = toRenderView(m_frame->document()->renderer());
> -
> -    if (!root) {
> -	   LOG_ERROR("document to be printed has no renderer");
> +    if (userScaleFactor <= 0) {
> +	   LOG_ERROR("userScaleFactor has bad value %.2f", userScaleFactor);
>	   return;
>      }
>  
> +    RenderView* root = toRenderView(m_frame->document()->renderer());
> +

It seems like NULL check for root was gone. Is this OK?

> +    const Vector<IntRect>& pageRects = printContext.pageRects();
> +    const size_t pageCount = pageRects.size();
> +    for (size_t pageNumber = 0; pageNumber < pageCount; ++pageNumber) {
> +	 const IntRect& pageRect = pageRects[pageNumber];
> +	 NSValue* val = [NSValue valueWithRect: NSMakeRect(pageRect.x(),
pageRect.y(), pageRect.width(), pageRect.height())];
> +	 [pages addObject: val];

- Two more spaces for indent here.
- Cannot we use IntRect::operator NSRect() to create NSRect? (maybe it's not in
WebCore.base.exp?)