[webkit-reviews] review granted: [Bug 23211] Add QWebFrame::renderContents to API : [Attachment 26567] Implements new API

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Adam Treat
<treat at kde.org>'s request for review:
Bug 23211: Add QWebFrame::renderContents to API
https://bugs.webkit.org/show_bug.cgi?id=23211

Attachment 26567: Implements new API
https://bugs.webkit.org/attachment.cgi?id=26567&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
> diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp
> index 2ec76cf..1de9496 100644
> --- a/WebKit/qt/Api/qwebframe.cpp
> +++ b/WebKit/qt/Api/qwebframe.cpp
> @@ -781,6 +781,33 @@ void QWebFrame::render(QPainter *painter)
>  }
>  
>  /*!
> +  \since 4.6
> +  Render the frame's \a contents into \a painter while clipping to \a
contents.
> +*/
> +void QWebFrame::renderContents(QPainter *painter, const QRegion &contents)
> +{
> +    if (!d->frame->view() || !d->frame->contentRenderer())
> +	   return;
> +
> +    d->frame->view()->layoutIfNeededRecursive();
> +
> +    GraphicsContext ctx(painter);
I'd name it 'context', as we dislike abbrevations in WebCore.

> +    QVector<QRect> vector = contents.rects();
> +    WebCore::FrameView* view = d->frame->view();
> +    for (int i = 0; i < vector.size(); ++i) {
> +	   if (i > 0) {
> +	       painter->save();
> +	       painter->setClipRect(vector.at(i), Qt::IntersectClip);
> +	   }
> +
> +	   view->paintContents(&ctx, vector.at(i));
> +
> +	   if (i > 0)
> +	       painter->restore();
> +    }
> +}

I'd rewrite the function as follows, to save some cycles:
    QVector<QRect> vector = contents.rects();
    if (vector.isEmpty())
	return;

    WebCore::FrameView* view = d->frame->view();
    view->paintContents(&context, vector.first());

    for (int i = 0; i < vector.size(); ++i) {
	const QRect& clipRect = vector.at(i);
	painter->save();
	painter->setClipRect(clipRect, Qt::IntersectClip);
	view->paintContents(&context, clipRect);
	painter->restore();
    }
}

r=me, with those changes.