323028 - [FIX]Using background-image CSS style for treechildren::-moz-tree-row causes excessive CPU load

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[Matthew Gertner]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

We are using PNG images as background for a XUL tree using the background-image style attached to treechildren::-moz-tree-row. When the tree is visible, CPU usage immediately goes to 100%. I don't think that the specific images are the problem since I tried converting them to GIFs, changing the size, etc. and the problem persists.

Reproducible: Always

Comment 1

[Matthew Gertner]

I just noticed that the problem only occurs when properties are specified for the tree row using treechildren::-moz-tree-row(X, Y, ...).

Comment 2

[Matthew Gertner]

Attachment: Example of a stylesheet that causes the problem.

Comment 3

[Matthew Gertner]

Attachment: XUL file that uses the CSS

This is the XUL file associated with the CSS. The important part is the vbox with id="apPeerTreeBox".

Comment 4

[Boris Zbarsky [:bzbarsky]]

So.. I linked to that CSS file from that XUL, changed the images in the CSS file to https://www.mozilla.org/images/header_logo.gif and there was no CPU usage at all.  I suspect at least part of this is that there are no treerows in that XUL... What's a XUL file that actually _does_ show the problem?  

Comment 5

[Matthew Gertner]

Attachment: Improved XUL example

Here's a new XUL file that reproduces the bug without the need for additional JavaScript. Obviously the path to the CSS must be tweaked. The problem is definitely linked to the properties of the tree rows. If I set properties for just one row, there is no problem. If I set "checked leaf" for both there is also no problem. It is only when I set one to "checked leaf" and the other to "unchecked leaf" that the process goes to 100%.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Stylesheet pointing at actual image

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: XUL pointing to that CSS

Matthew, with this testcase (which points to the CSS, which points to the bugzilla image), I see zero CPU usage in a current trunk seamonkey build.  Do you see the CPU problem with it in a trunk build?

Comment 8

[Boris Zbarsky [:bzbarsky]]

I also see zero CPU usage in Firefox 1.0.7 (release build) and Firefox 1.5.0.1 release candidate (debug build).

All of this on Linux.  Is this a Windows-only issue?

Comment 9

[Matthew Gertner]

If you aren't seeing the symptoms under Linux, it's very possible this is a Windows-only issue. I've only tested on Windows with the 1.8 branch.

Comment 10

[Boris Zbarsky [:bzbarsky]]

But you do see the problem when loading from bugzilla the XUL file I attached?

Comment 11

[Matthew Gertner]

Yes, but only when I select the first row in the tree. Not sure what's going on since I didn't need to do that in the original example, but I assume it's the same problem and this should enable you to reproduce it.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Hmm.... That worksforme too.  :(

Do you have a build with symbols you could test in a debugger, by chance?  If so, just breaking randomly and seeing where you are in the code would help...

Comment 13

[Martijn Wargers (dead)]

Fwiw, this is worksforme with current trunk build on WindowsXP.

Comment 14

[Matthew Gertner]

Attachment: New stylesheet referencing original images

I think the problem may be somehow linked to the actual images we are using, so I'm uploading a new stylesheet that references them instead of the images used by Boris.

Comment 15

[Matthew Gertner]

Attachment: XUL file using new stylesheet

Comment 16

[Matthew Gertner]

Try the latest XUL file. I get the problem on both my official release and debug version of FF 1.5 on Windows XP (CPU immediately goes to 100%).

Comment 17

[Martijn Wargers (dead)]

With the latest XUL file, I can see the 100% cpu.
I've made a modified testcase here (I can't seem to use data uri inside ::-moz-tree-row selector?):
http://wargers.org/mozilla/bug323028/323028_excessive_cpu.xul
Here I have a backtrace with a breakpoint in nsTreeSelection::InvalidateSelection:
http://wargers.org/mozilla/bug323028/bt_323028.txt
This regressed between 2004-09-20 and 2004-09-21:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-09-20+06%3A00%3A00&maxdate=2004-09-21+10%3A00%3A00&cvsroot=%2Fcvsroot

Comment 18

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 259139

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to fix

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 209711 [details] [diff] [review]
Patch to fix

Replace "return" with "return rv" and r+sr=dbaron.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 22

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 209781 [details] [diff] [review]
Updated to comments

I think this is worth it for the 1.8 branch.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 209781 [details] [diff] [review]
Updated to comments

It's likely worth it to take this on the 1.8.0 branch too.  This is a very safe fix that just reverts to the behavior this code had in 1.7.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Fixed on 1.8.1 branch.

Comment 26

[Daniel Veditz [:dveditz]]

Comment on attachment 209781 [details] [diff] [review]
Updated to comments

approved for 1.8.0 branch, a=dveditz

Comment 27

[Boris Zbarsky [:bzbarsky]]

Fixed for 1.8.0.2.

Comment 28

[Dave Liebreich [:davel]]

Marking [rft-dl] (ready for testing in Firefox 1.5.0.2 release candidates)

Comment 29

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2, cpu usage normal with latest xul testcase.