Bug 194538 – Crash in WebCore::ScrollingTree::updateTreeFromStateNode

• Component: Scrolling
• Status: RESOLVED FIXED

[Antti Koivisto]

3 libsystem_c.dylib: abort
          3 libc++abi.dylib: abort_message
            3 libc++abi.dylib: __cxa_pure_virtual
       ==> 3 WebCore: WebCore::ScrollingTree::updateTreeFromStateNode(WebCore::ScrollingStateNode const*, WTF::HashMap<unsigned long long, WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> >, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> > > >&) <==
                3 WebCore: WebCore::ScrollingTree::updateTreeFromStateNode(WebCore::ScrollingStateNode const*, WTF::HashMap<unsigned long long, WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> >, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> > > >&)
                  pruning: 2 WebCore: WebCore::ScrollingTree::commitTreeState(std::__1::unique_ptr<WebCore::ScrollingStateTree, std::__1::default_delete<WebCore::ScrollingStateTree> >)
                  pruning: 1 WebCore: WebCore::ScrollingTree::updateTreeFromStateNode(WebCore::ScrollingStateNode const*, WTF::HashMap<unsigned long long, WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> >, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> > > >&)

[Antti Koivisto]

<rdar://problem/47841926>

[Antti Koivisto]

Created attachment 361794 [details]
patch

[WebKit Commit Bot]

Comment on attachment 361794 [details]
patch

Clearing flags on attachment: 361794

Committed r241296: <https://trac.webkit.org/changeset/241296>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Justin Cohen]

Chrome for iOS is seeing a big increase in this crash on 12.2.  And while volume is low, we also see it on 12.3


For example:
0x000000019ae7d0dc	(libsystem_kernel.dylib + 0x000230dc )	__pthread_kill
0x000000019aef6090	(libsystem_pthread.dylib + 0x00002090 )	pthread_kill$VARIANT$mp
0x000000019add6ea4	(libsystem_c.dylib + 0x0005aea4 )	abort
0x000000019a4a3784	(libc++abi.dylib + 0x00001784 )	abort_message
0x000000019a4b0a3c	(libc++abi.dylib + 0x0000ea3c )	__cxa_pure_virtual
0x00000001a4cd68d4	(WebCore + 0x011da8d4 )	WebCore::ScrollingTree::updateTreeFromStateNode(WebCore::ScrollingStateNode const*, WTF::HashMap<unsigned long long, WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> >, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> > > >&)
0x00000001a4cd69bc	(WebCore + 0x011da9bc )	WebCore::ScrollingTree::updateTreeFromStateNode(WebCore::ScrollingStateNode const*, WTF::HashMap<unsigned long long, WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> >, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WTF::RefPtr<WebCore::ScrollingTreeNode, WTF::DumbPtrTraits<WebCore::ScrollingTreeNode> > > >&)
0x00000001a4cd636c	(WebCore + 0x011da36c )	WebCore::ScrollingTree::commitTreeState(std::__1::unique_ptr<WebCore::ScrollingStateTree, std::__1::default_delete<WebCore::ScrollingStateTree> >)
0x00000001aaa080cc	(WebKit + 0x002a50cc )	WebKit::RemoteScrollingCoordinatorProxy::commitScrollingTreeState(WebKit::RemoteScrollingCoordinatorTransaction const&, WebKit::RemoteScrollingCoordinatorProxy::RequestedScrollInfo&)
0x00000001aa9115d0	(WebKit + 0x001ae5d0 )	WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree(WebKit::RemoteLayerTreeTransaction const&, WebKit::RemoteScrollingCoordinatorTransaction const&)
0x00000001aa7b915c	(WebKit + 0x0005615c )	void IPC::handleMessage<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree, WebKit::RemoteLayerTreeDrawingAreaProxy, void (WebKit::RemoteLayerTreeDrawingAreaProxy::*)(WebKit::RemoteLayerTreeTransaction const&, WebKit::RemoteScrollingCoordinatorTransaction const&)>(IPC::Decoder&, WebKit::RemoteLayerTreeDrawingAreaProxy*, void (WebKit::RemoteLayerTreeDrawingAreaProxy::*)(WebKit::RemoteLayerTreeTransaction const&, WebKit::RemoteScrollingCoordinatorTransaction const&))
0x00000001aa79ceac	(WebKit + 0x00039eac )	IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
0x00000001aa95a31c	(WebKit + 0x001f731c )	WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
0x00000001aa78da1c	(WebKit + 0x0002aa1c )	IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
0x00000001aa79050c	(WebKit + 0x0002d50c )	IPC::Connection::dispatchIncomingMessages()

[Simon Fraser (smfr)]

Any information you can gather about reproducibility would help in the diagnosis.

[Ali Juma]

(In reply to Simon Fraser (smfr) from comment #6)
> Any information you can gather about reproducibility would help in the
> diagnosis.

We don't have specific repro steps, but we know that users that have our new session restoration logic enabled are running into this crash at a rate that's 30 times higher than other users, and are crashing much earlier -- the median process uptime when this particular crash occurs is about 16 seconds for those users, compared to 4.3 minutes for other users.

This new session restoration logic works by loading a sequence of file URLs that each return a page that can redirect to the actual site the user visited. So as the user navigates back/forward, we'd first visit that file URL, then get a redirect, and then load the actual URL. (We have to restore sessions in this weird way in order to workaround bug 169618.)

This approach has the side effect of adding process swaps. So suppose the user's actual history is:
a) www.google.com/search?q=cats
b) www.google.com/search?q=dogs
c) www.google.com/search?q=mice

And say that at startup, we restore this history and the user is now at (c). At this point, if the user taps on the back button, we'd first load our file URL that then redirects to (b). So ordinarily on a navigation from (c) to (b) there wouldn't be a process swap and we'd continue committing to the same ScrollTree, but with this change we swap to a different process for the file URL and then back to the original process after the redirect. And this time, when we continue in the original process, we're committing to a new scroll tree.

So my guess is that somehow we're winding up with a parent-less ScrollTreeNode at the end of all this. (And afaik, https://bugs.webkit.org/show_bug.cgi?id=193907 is not in 12.2 or 12.3, so we don't have the change there to track and remove unvisited nodes in ScrollingTree::commitTreeState.)