Closed Bug 69142 Opened 19 years ago Closed 19 years ago
Setting attributes on menus thrashes in Recreate
Frame For Content on Mac OS
As part of profiling window activation (when a window comes to the fg), i noticed that 50% of activation processing was spent in RecreateFrameForContent() after setting attributes on content nodes representing menu items. Recall that on the mac, menus on the menubar are native and have a rule that makes them display:none. This won't ever change, yet the style system, upon seeing the attributeChanged message, tries its hardest to squeeze out a frame, wastes lots of time, and eventually ends up back at the start w/out a frame. 50% of window activate translates into 25% of window refresh time (the time from when you click in a bg window until the paint completes updating all the areas damaged from bringing the window to the front). This is very noticable on mac (flashing) and contributes to it feeling sluggish. ideas?
Hmm. I wonder if we can optimize the computation of a style change's impact such that display:none causes nothing to happen. Sounds like it is worth looking into. So, what attribute is changing? Is there some style rule that has an attribute selector with that attribute? If not, I wonder why shaver's optimization isn't kicking in (the optimization to skip attribute changed notification reresolves if there is not rule based on the attrib).
there are rules based on that attribute for _some_ menus, just not those on the menubar. i think shaver's optimizations fall by the wayside when anyone, anywhere, uses a rule with that attribute. what say you, shaver? I'm guessing the attribute is the "disabled" attribute, since we're updating menu/command state when bringing the window to the fg.
I have a patch that takes this off the map. Basically, I looked at the code and you can't stop RecreateFramesForContent from being called. It's correct that the code calls it. The only reason the method is slow in the case where you have no parent frame is that ContentInserted is stupid. I have a simple patch to ContentInserted that makes it much smarter about not doing any work when the parent content node has no frame. This takes RecreateFramesForContent off the map on my Quantify runs.
Status: NEW → ASSIGNED
Looking for r and sr.
Patch looks good to me r/sr=attinasi
Make a note about why the container might be null (e.g., ``this happens if we're inserting into an element that is currently mapped `display: none', for example''). Do that, sr=waterson
Fix was checked in ages ago. Not sure how this bug stayed open.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.