I don't think we're going to figure out on time why adding the richlistitems using a DOM fragment doesn't hint gecko that it could avoid all these layout flushes during the giant appendChild call, not to mention fix that. However, I found out that the multiple reflows can be easily avoided by temporarily removing the richlixtbox from the document. This isn't noticeable at at all, and at least on my machine it almost completely resolves the slowness we see on Alice's profile. So, ugly as it is, I think we should go ahead and do it. I also found out that "un-attaching" the richlistitem binding from the inactive elements has a notable positive effect. This hack I like less, because it could have some side-effects (for example, it'd be a little tricky to have searchLabel working for this items). However, the gain is notable enough that it's at least worth trying. We can take it back later if it turns out to be problematic. Patch attached. I've also removed the _wasDone setting for history downloads as it's not necessary for them (it's only used in onStateChange).

Comment on attachment 699532 [details] [diff] [review] patch Review of attachment 699532 [details] [diff] [review]: ----------------------------------------------------------------- I didn't test the patch locally yet, now going to do that. ::: browser/components/downloads/content/allDownloadsViewOverlay.css @@ +3,5 @@ > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > +richlistitem.download { > + -moz-binding: none; > +} please add comment explaining what this is for and which kind of bad consequences may have. ::: browser/components/downloads/content/allDownloadsViewOverlay.js @@ +150,1 @@ > this._targetFileInfoFetched = false; Instead of doing this _wasDone hack, go to http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/src/DownloadsCommon.jsm#944 aState there is the previous state, though we lose that information along the way http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/src/DownloadsCommon.jsm#984 we should instead forward that information to onStateChange, so we can know whether the download was done. fwiw this is the proper definition of done (not sure why the panel checks !wasDone and openable while the view uses !finished and done?) http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/src/DownloadsCommon.jsm#1259 @@ +1071,5 @@ > + let parentNode = this._richlistbox.parentNode; > + let nextSibling = this._richlistbox.nextSibling; > + parentNode.removeChild(this._richlistbox); > + this._richlistbox.appendChild(aDOMFragment); > + parentNode.insertBefore(this._richlistbox, nextSibling); trailing spaces. Is this absolutely needed to avoid the reflows? does setting hidden on the RLB suffice?

I fixed the "_wasDone" part in bug 828243.

Backed out for browser-chrome leaks: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=912fb83c1820&jobname=browser-chrome https://hg.mozilla.org/integration/mozilla-inbound/rev/b143f74ed255

Regardless of the leak issue, there's another problem with this patch: I need to remove the controller from the richlistbox and add it back after it's inserted back to the tree.

Comment on attachment 700025 [details] [diff] [review] address controller issue [Approval Request Comment] Bug caused by (feature/regressing bug #): Downloads panel feature User impact if declined: Performance issues Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): Limited to the feature String or UUID changes made by this patch: none

Comment on attachment 700025 [details] [diff] [review] address controller issue Is there a bug tracking the removal of this workaround once the core issue is fixed? We should have one on file.