Bug 185394 – Avoid traversing the FrameTree up in FrameTree::top()

• Status: RESOLVED
• Resolution: FIXED
• Component: Frames

[Chris Dumez]

Avoid traversing the FrameTree up in FrameTree::top() and rely on Frame::mainFrame() instead to get the answer right away.

[Chris Dumez]

Created attachment 339754 [details]
Patch

[Sam Weinig]

Comment on attachment 339754 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339754&action=review

> Source/WebCore/ChangeLog:12
> +        of returning the frame itself). However, it should not matter in practice as we should
> +        not be dealing with the FrameTree of a detached frame.

Do we want to / can we assert this?

[Chris Dumez]

Comment on attachment 339754 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339754&action=review

>> Source/WebCore/ChangeLog:12
>> +        not be dealing with the FrameTree of a detached frame.
> 
> Do we want to / can we assert this?

Like an ASSERT(m_thisFrame.isMainFrame() || tree().parent()); inside FrameTree::top() ? This would make sure no one calls FrameTree::top() on a detached Frame. I am not sure if such assertion would be hit or not. I could try.

[Ahmad Saleem]

Checking via BugID, it seems this r+ patch didn't landed and old code is still present:

https://github.com/WebKit/WebKit/blob/8f0e98e67893ed3be45684a65a42de9bf244ed81/Source/WebCore/page/FrameTree.cpp#L454

Do we need this? Thanks!

[Ahmad Saleem]

Frame& FrameTree::top() const
{
    ASSERT(m_thisFrame.isMainFrame() || tree().parent());
    return m_thisFrame.mainFrame();


}

This compiles with --release but didn't check with --debug.

[EWS]

Committed 273230@main (80993fda030b): <https://commits.webkit.org/273230@main>

Reviewed commits have been landed. Closing PR #22760 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/121272959>