Closed Bug 815591 Opened 12 years ago Closed 12 years ago

Avoid building unnecessary nsDisplayBackgroundColors


(Core :: Layout, defect)

18 Branch
Windows 7
Not set



Tracking Status
b2g18 --- fixed


(Reporter: roc, Assigned: roc)




(2 files)

      No description provided.
Attached patch fixSplinter Review
Attachment #685584 - Flags: review?(matt.woodrow)
Comment on attachment 685584 [details] [diff] [review]

Review of attachment 685584 [details] [diff] [review]:

Weird, I have the exact same patch in my b2g tree.

I think we can also skip building the nsDisplayBackgroundImage if bg->mLayers[i].mImage.IsEmpty().
Attachment #685584 - Flags: review?(matt.woodrow) → review+
The backout also fixed Windows mochitest-4 failures of the form:
16615 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug450930.xhtml | Didn't get subdoc invalidation while we were privileged (iframe2) 
The backout also seemed to have fixed Talos regressions:
I don't understand it at all!
Well, the fact that you're branching on an uninitialized |color| isn't great -- you should probably put drawBackgroundColor ? color : NS_RGBA(0, 0, 0, 0) in a variable and test that.

It doesn't explain failures/regressions, though.
This Mac reftest failure is still happening.
Mac reftest failure looks like a gradient rasterization issue. This patch has reduced the bounds of some display items and maybe some temporary surfaces causing the gradient to get rendered slightly differently after invalidation. I'll fuzz it.
The Tp results look OK now.
Since I just fuzzed the test, I'll reland.
Er, scratch that. Let me wait for the extra mochitest-4 runs that I triggered to come back.
Those try results still look good. I'll reland this when the tree isn't burning.
Apparently you didn't fuzz the OSX reftests enough, because they're still failing. Backed out.
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: 846144
Comment on attachment 685584 [details] [diff] [review]

NOTE: Please see to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: failure to get tiny improvement in scrolling/rendering performance (see bug 846901)
Testing completed: been on trunk for a while
Risk to taking this patch (and alternatives if risky): pretty low risk given trunk landing. Will need to uplift regression fix in bug 846144 as well.
String or UUID changes made by this patch: none.
Attachment #685584 - Flags: approval-mozilla-b2g18?
Comment on attachment 685584 [details] [diff] [review]

Approving for scrolling perf win.
Attachment #685584 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Backed out for reftest failures. And a reminder, the tree rules still state that people who push to release branches are expected to watch and star their pushes...
Landing this patch on b2g18 causes a couple of reftest failures because of bug 828146, which is not fixed on b2g18.
Landing bug 828146 on the b2g18 branch causes some new test failures. So I think the best solution is to just disable the failing tests on the b2g18 branch:

(Note that bug 828146 only affects elements using -moz-appearance which we don't actually use on B2G.)
Comment on attachment 728808 [details] [diff] [review]
Disable failing reftests

NOTE: Please see to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): existing bug revealed by patch for bug 815591
User impact if declined: won't be able to fix bug 815591 on b2g18
Testing completed: none
Risk to taking this patch (and alternatives if risky): No risk, just disabling tests
String or UUID changes made by this patch: none
Attachment #728808 - Flags: approval-mozilla-b2g18?
Attachment #728808 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Depends on: 922228
You need to log in before you can comment on or make changes to this bug.