Bug 135129 – Assertion failure in WebPage::reload (!m_pendingNavigationID) when reloading after a same-document back navigation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

<rdar://problem/17593701>

To reproduce: navigate to data:text/html,<a%20href="#">link</a> then tap the link, go back, and reload.

Created attachment 235245 [details]
Don't create new navigations for same-document back-forward navigations

[Darin Adler]

Comment on attachment 235245 [details]
Don't create new navigations for same-document back-forward navigations

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

> Source/WebKit2/Shared/WebBackForwardListItem.cpp:74
> +    for (const auto& child : a.children) {
> +        const FrameState* otherChild = childItemWithDocumentSequenceNumber(b, child.documentSequenceNumber);
> +        if (!otherChild || !documentTreesAreEqual(child, *otherChild))
> +            return false;
> +    }

Too bad this is n^2 the number of children at each level of the tree, and recursive.

It also unnecessarily compares every document sequence number during the recursion even though it finds each child by sequence number.

I could imagine a more version that copied and sorted the vectors of children to avoid the n^2.

And we might be able to cleanly avoid the recursion by using a Deque to keep track of children to compare.

But I’m not sure either of these changes would actually be an improvement.

> Source/WebKit2/Shared/WebBackForwardListItem.cpp:84
> +    // The following logic must be kept in sync with WebCore::HistoryItem::shouldDoSameDocumentNavigationTo.

I find the logic below utterly mysterious. Maybe the “why” comments are in shouldDoSameDocumentNavigationTo?

> Source/WebKit2/Shared/WebBackForwardListItem.cpp:95
> +    WebCore::URL url = WebCore::URL(WebCore::ParsedURLString, mainFrameState.urlString);
> +    WebCore::URL otherURL = WebCore::URL(WebCore::ParsedURLString, otherMainFrameState.urlString);
> +
> +    if ((url.hasFragmentIdentifier() || otherURL.hasFragmentIdentifier()) && equalIgnoringFragmentIdentifier(url, otherURL))

Seems a real shame we have to reconstitute the URL objects just to find the "#" characters in them. I am pretty sure this can be done more efficiently without fully re-parsing the URLs, but it might make the code harder to maintain so I guess we can leave it like this.

Fixed in <http://trac.webkit.org/r171574>.