Bug 27137 – REGRESSION (r44311): Reproducible crash due to infinite recursion into FrameLoader::gotoAnchor() -> FrameView::layout()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

<rdar://problem/7043124>

VIsiting the URL makes Safari crash with a stack showing many repititions of

  com.apple.WebCore              0x9547f226 WebCore::FrameLoader::gotoAnchor() + 0x56
  com.apple.WebCore              0x9549a348 WebCore::FrameView::layout(bool) + 0x828
  com.apple.WebCore              0x956124ad WebCore::FrameLoader::gotoAnchor(WebCore::String const&) + 0x26d

We may need to make the going-to-anchor a post-layoiut task. I suspect that this was introduced by a recent patch to improve locking-to-anchor during loading, but I haven’t verified this yet.

<http://trac.webkit.org/changeset/44311/> is the change I had in mind.

Confirmed that reverting r44311 fixes the crash.

Having looked at the code, I don’t think moving to post-layout is the right solution here. If the frame needs layout at that point, there’s no use scrolling it until that layout occurs. But also, it seems somewhat inefficient to go through gotoAnchor() every time. Perhaps instead of the lockedToAnchor boolean, a RefPtr<Node> could be used and the frame could track that node without calling back into the loader.

[Adam Barth]

Concensus on IRC was not to back the patch out tonight.  Nate will look into this tomorrow.  If we can't find a fix shortly, we will revert the offending change.

Created attachment 32551 [details]
Make lock-scroll-to-anchor a function of FrameView, and perform it as a post-layout task

[Simon Fraser (smfr)]

Comment on attachment 32551 [details]
Make lock-scroll-to-anchor a function of FrameView, and perform it as a post-layout task



> +void FrameView::keepScrolledToAnchorNode(Node* anchorNode)

I'm not a big fan of this method name; maybe something like maintainScrollPositionToAnchorNode?

> +    if (anchorNode != m_frame->document())
> +        rect = anchorNode->getRect();

getRect(true) will make this transform-friendly!

r=me

Fixed in <http://trac.webkit.org/projects/webkit/changeset/45710> and <http://trac.webkit.org/projects/webkit/changeset/45711>.