1373821 - Browser getting unresponsive after clicking on tag/folder in bookmarks bar

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[[:philipp]]

Version 	56.0a1
Build-ID 	20170616030207
User-Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

i was clicking on a tag/folder in my bookmarks toolbar folder containing around 10 bookmarks. before the drop-down with the results appeared, the browser ui hung for about 10 seconds and windows' program not responding appeared in the titlebar.

this is a snapshot taken with the gecko profiler: http://bit.ly/2rFo2SJ

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to [:philipp] from comment #0)
> this is a snapshot taken with the gecko profiler: http://bit.ly/2rFo2SJ

Florian, can you look into this profile?

Comment 2

[Florian Quèze [:florian]]

(In reply to Panos Astithas [:past] (56 Regression Engineering Owner) (please ni?) from comment #1)
> (In reply to [:philipp] from comment #0)
> > this is a snapshot taken with the gecko profiler: http://bit.ly/2rFo2SJ
> 
> Florian, can you look into this profile?

Main thread I/O in nsNavBookmarks::GetDescendantFolders at http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/toolkit/components/places/nsNavBookmarks.cpp#1067

Blocking the main thread for at least 3.2s (and more likely 12.7s, but the profile buffer wasn't large enough to capture samples during all of the main thread jank).

I've already seen main thread I/O block for more than 30s a couple times (typically I had Windows Defender running a background scan of the whole disk when it happened).

Comment 3

[Marco Bonardo [:mak]]

GetDescendantFolders is indeed a sync API we use when querying in very specific cases, basically for queries specifying both a folder=N and other conditions, then we want to pick all the bookmarks below that folder in the tree.

For tags we should not go through it, but we wrongly do.
It's sort of an edge case, but probably bad for tag addicted users.

I think this would be fixed by bug 1293445. We could even fix it earlier with a workaround if necessary.

Comment 4

[Marco Bonardo [:mak]]

This should be gone with the fix for bug 1293445. I'll file a separate bug to completely remove GetDescendantFolders, I think we can replace it with a recursive CTE subquery.