[webkit-reviews] review granted: [Bug 135103] [iOS WK1] Single touch div scrolling in Mobile Safari doesn't work in framesets (breaks Word previews) : [Attachment 235188] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 135103: [iOS WK1] Single touch div scrolling in Mobile Safari doesn't work
in framesets (breaks Word previews)
https://bugs.webkit.org/show_bug.cgi?id=135103

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

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


r=me, not sure if you need to switch to a post-order traversal

> Source/WebCore/rendering/RenderLayerCompositor.cpp:474
> +    notifySubframesAfterLayerFlush();
> +    didFlushLayers();

It seems strange to flush the top frame separately.

Also, I think the function should be named for what it does rather than in what
circumstance it’s called in. Maybe flushDescendantLayers, except that argues
against putting it in there.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:500
> +    for (Frame* currFrame = frame.tree().firstChild(); currFrame; currFrame
= currFrame->tree().traverseNext(&frame)) {

I suggest naming this variable subframe or descendant rather than currFrame.

Since we want to flush subframes before their parent, this traversal is not the
one we want. This is a pre-order traversal, where each frame is visited before
its children. I think what we want is a post-order traversal, where children
are visited before their parents.

Unfortunately, we don’t have a post-order traversal function in FrameTree. It’s
not hard to write one. The nextPostOrder function in NodeTraversal.cpp is not a
bad model:

    static Frame* firstDescendantFramePostOrder(Frame& frame)
    {
	Frame* descendant = &frame;
	while (Frame* firstChild = descendant->tree()->firstChild())
	    descendant = firstChild;
	return descendant;
    }

    static Frame* nextFramePostOrder(Frame* frame)
    {
	Frame* next = frame->tree()->nextSibling();
	if (!next)
	    return frame->tree()->parent();
	return firstDescendantFramePostOrder(*next);
    }

And the loop is a little different:

    for (Frame* subframe = firstDescendantFramePostOrder(frame); subframe !=
&frame; subframe = nextFramePostOrder(subframe)) {
	...
    }