Bug 135103

Summary: [iOS WK1] Single touch div scrolling in Mobile Safari doesn't work in framesets (breaks Word previews)
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Description Simon Fraser (smfr) 2014-07-20 10:54:58 PDT
[iOS WK1] Single touch div scrolling in Mobile Safari doesn't work in framesets (breaks Word previews)
Comment 1 Simon Fraser (smfr) 2014-07-20 11:04:51 PDT
Created attachment 235188 [details]
Patch
Comment 2 Simon Fraser (smfr) 2014-07-20 11:05:23 PDT
<rdar://problem/11830219>
Comment 3 Darin Adler 2014-07-20 13:41:00 PDT
Comment on attachment 235188 [details]
Patch

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)) {
        ...
    }
Comment 4 Simon Fraser (smfr) 2014-07-21 10:38:04 PDT
The order in which we notify subframes doesn't really matter in this code, and I'd rather not add an O(N^2) post-order frame traversal.
Comment 5 Simon Fraser (smfr) 2014-07-21 11:27:30 PDT
https://trac.webkit.org/r171306