Avoid building unnecessary nsDisplayBackgroundColors

RESOLVED FIXED in mozilla21

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

18 Branch
mozilla21
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(b2g18 fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
Created attachment 685584 [details] [diff] [review]
fix
Attachment #685584 - Flags: review?(matt.woodrow)
Comment on attachment 685584 [details] [diff] [review]
fix

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+

Comment 5

5 years ago
The backout also fixed Windows mochitest-4 failures of the form:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17441404&tree=Mozilla-Inbound
{
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:
http://mzl.la/VfEIP2
http://mzl.la/VfI84c
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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d8f5013110e
https://hg.mozilla.org/mozilla-central/rev/e941e0df729c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21

Updated

5 years ago
Depends on: 846144
Comment on attachment 685584 [details] [diff] [review]
fix

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing 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]
fix

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...
https://hg.mozilla.org/releases/mozilla-b2g18/rev/7508c5a1026b
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:
gfx/tests/reftest/709477-1.html
layout/reftests/scrolling/opacity-mixed-scrolling-2.html

(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 https://wiki.mozilla.org/Release_Management/B2G_Landing 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?

Updated

5 years ago
Attachment #728808 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+

Updated

4 years ago
Depends on: 922228
You need to log in before you can comment on or make changes to this bug.