Closed
Bug 1296678
Opened 8 years ago
Closed 8 years ago
Uninitialised value use in nsDisplayBackgroundImage::IsNonEmptyFixedImage
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: jseward, Unassigned)
References
Details
Attachments
(2 files)
2.62 KB,
text/plain
|
Details | |
1.12 KB,
patch
|
botond
:
review+
ritu
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
DISPLAY=:1.0 G_SLICE=always-malloc ./mach mochitest --valgrind=vTRUNK \ --valgrind-args=--num-transtab-sectors=24,--px-default=allregs-at-mem-access,--px-file-backed=unwindregs-at-mem-access,--fair-sched=yes,--fullpath-after=/MOZ/,--trace-children=yes,--show-mismatched-frees=no,--track-origins=yes \ browser/base/content/test/general/browser_bug422590.js \ 2>&1 | tee spew-14-05-mc Produces uninitialised value errors as shown in the attachment. This happens for several tests in Mochitests. It appears that nsCSSRendering::PrepareImageLayer can return without assigning any value to its out-parameter |bool* aOutIsTransformedFixed| and that is the cause of the problem. Possibly related to bug 735857.
Reporter | ||
Comment 1•8 years ago
|
||
Ensure *aOutIsTransformedFixed is given a value on all return paths.
Reporter | ||
Updated•8 years ago
|
Attachment #8782968 -
Flags: review?(botond)
Updated•8 years ago
|
Attachment #8782968 -
Flags: review?(botond) → review+
Comment 2•8 years ago
|
||
(In reply to Julian Seward [:jseward] from comment #0) > It appears that nsCSSRendering::PrepareImageLayer can return without > assigning any value to its out-parameter |bool* aOutIsTransformedFixed| > and that is the cause of the problem. > > Possibly related to bug 735857. Yeah, this was introduced in bug 735857.
Blocks: 735857
Reporter | ||
Comment 3•8 years ago
|
||
Note that bug 735857 was uplifted to 48.
Comment 4•8 years ago
|
||
(In reply to Julian Seward [:jseward] from comment #3) > Note that bug 735857 was uplifted to 48. I had a look at the code to see what the consequences of the uninitialized value could be. It seems like a background-attachment:fixed image could incorrectly be treated as background-attachment:scroll in the following combination of circumstances: - The uninitialized variable |isTransformedFixed| in the nsDisplayBackgroundImage constructor happens to have the initial value of |true|. - The early-return path in PrepareImageLayer() is hit, which means "There's no image or it's not ready to be painted." - The background image is not, in fact, inside a transform. The treatment would be corrected on a subsequent paint when the image becomes ready to be painted. I believe this isn't important enough to try getting it into a 48 point-release. It does make sense to uplift it as far as 49, as the patch is very low-risk.
Updated•8 years ago
|
Pushed by jseward@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/137be83b273c Uninitialised value use in nsDisplayBackgroundImage::IsNonEmptyFixedImage. r=botond.
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/137be83b273c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment 7•8 years ago
|
||
Comment on attachment 8782968 [details] [diff] [review] bug1296678-1.cset Requesting uplift as per comment 4: Approval Request Comment [Feature/regressing bug #]: Bug 735857 [User impact if declined]: A background image which is background-attachment:scroll could be treated as background-attachment:fixed by our layout system, but only while the image isn't loaded yet. Markus and I do not believe this can result in any user-visible misbehaviour. However, since the fix is so trivial, I am requesting uplift anyways. [Describe test coverage new/current, TreeHerder]: On m-c since 2016-08-26 [Risks and why]: Very low. The patch just ensures that a variable isn't left uninitialized during an early-return path of a function. [String/UUID change made/needed]: None
Attachment #8782968 -
Flags: approval-mozilla-beta?
Attachment #8782968 -
Flags: approval-mozilla-aurora?
Comment on attachment 8782968 [details] [diff] [review] bug1296678-1.cset Fixes a mochitest failure, Aurora50+
Attachment #8782968 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 9•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/42711ffd066b
Comment 10•8 years ago
|
||
Comment on attachment 8782968 [details] [diff] [review] bug1296678-1.cset Regression from 48, seems pretty safe. Taking this for beta 9.
Attachment #8782968 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/bc17999985e5
You need to log in
before you can comment on or make changes to this bug.
Description
•