423736 - Proto Theme landing regressed sunspider

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P1)

Description

[Robert Sayre]

1-3% regression shown on mac tinderboxen.

Comment 1

[Robert Sayre]

Are the text decorations in the status bar new? That's probably it. That text is very performance sensitive. We shouldn't be applying decorations to it.

Comment 2

[Dão Gottwald [:dao]]

yes, they are new.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Ryan, can we get that backed out for Beta 5? I can't remember if there's a specific bug on the text in the status bar, but if not, we can file a follow up and deal with it after.

Comment 4

[Johnathan Nightingale [:johnath]]

No separate bug, no, but the changes in question are here:

https://bugzilla.mozilla.org/attachment.cgi?id=309985&action=diff#mozilla/browser/themes/pinstripe/browser/browser.css_sec4

and here:

https://bugzilla.mozilla.org/attachment.cgi?id=309985&action=diff#mozilla/browser/themes/pinstripe/browser/browser.xml_sec2

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: revert statusbar XBL changes

This patch had no effect locally, but I can't reproduce the regression locally either. Backing out the patch for bug 423361 entirely didn't show a significant difference.

I tried landing this to see what the mac Talos machines will say.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

That seems to have fixed it.

Ryan, can you confirm that my backout didn't miss anything?

Beltzner, do we want a followup on re-enabling this styling in some way that doesn't regress performance?

Comment 7

[Jesse Ruderman]

I don't think a new bug is needed.  We just need to mention in bug 414728 that the initial attempt did in fact regress perf.

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

(In reply to comment #6)
> Ryan, can you confirm that my backout didn't miss anything?
Looks fine to me :)

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #7)
> I don't think a new bug is needed.  We just need to mention in bug 414728 that
> the initial attempt did in fact regress perf.

Yeah, I didn't realize there was already a bug on it.

This is FIXED.