[webkit-reviews] review granted: [Bug 127722] REGRESSION(r162837): 5% regression on html5-full-render and 3% regression in DoYouEvenBench : [Attachment 222466] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 127722: REGRESSION(r162837): 5% regression on html5-full-render and 3%
regression in DoYouEvenBench
https://bugs.webkit.org/show_bug.cgi?id=127722

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=222466&action=review


> Source/WebCore/dom/Document.cpp:4317
>  Document* Document::topDocument() const
>  {
> -    // FIXME: Why does this walk up owner elements instead using the frame
tree as parentDocument does?
> -    // The frame tree even has a top() function.
> -    Document* document = const_cast<Document*>(this);
> -    while (Element* element = document->ownerElement())
> -	   document = &element->document();
> -    return document;
> +    return m_frame ? m_frame->mainFrame().document() :
const_cast<Document*>(this);
>  }

How is this change related to the rest of the patch? Does something depend on
it?

Looks like this never returns null, so it should be changed to return a
reference.

> Source/WebCore/rendering/RenderView.h:238
> +    class RepaintRegionAccumulator {

This has no members that are not copyable; we should probably mark it
non-copyable explicitly.