CSS counter values seem non-deterministic once all counter uses have been removed

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Jesse Ruderman, Assigned: dbaron)

Tracking

({testcase})

Trunk
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
Created attachment 251732 [details]
testcase

The numbers in this testcase keep changing.  The effect is especially noticeable in debug builds.

Updated

11 years ago
OS: Mac OS X → All
Hardware: Macintosh → All
(Assignee)

Comment 1

11 years ago
Created attachment 251739 [details] [diff] [review]
patch

The problem was that I forgot to update mSize in DestroyNodesFor.
Attachment #251739 - Flags: superreview?(roc)
Attachment #251739 - Flags: review?(roc)
(Assignee)

Updated

11 years ago
Blocks: 3247
Status: NEW → ASSIGNED
Whiteboard: [patch]
(Assignee)

Comment 2

11 years ago
Comment on attachment 251739 [details] [diff] [review]
patch

er, no, Remove does this, except for the one case where I don't call Remove
Attachment #251739 - Flags: superreview?(roc)
Attachment #251739 - Flags: review?(roc)
(Assignee)

Comment 3

11 years ago
Created attachment 251740 [details] [diff] [review]
patch

OK, this just fixes the one case that was broken.  (So the bug was only present if you had counters, removed *all* of them, and then added more -- size would end up off by one.)

This simplifies code by doing pointer comparison on dangling pointers to see if we just removed the last entry.  A little hairy, but it should be ok.  Remove does some extra fixup we don't need in that case, but I'd rather just call Remove everywhere we do a remove.
Attachment #251739 - Attachment is obsolete: true
Attachment #251740 - Flags: superreview?(roc)
Attachment #251740 - Flags: review?(roc)
Comment on attachment 251740 [details] [diff] [review]
patch

OK, but really, I think you should just do the comparison early on non-dangling pointers and save the result in a boolean.
Attachment #251740 - Flags: superreview?(roc)
Attachment #251740 - Flags: superreview+
Attachment #251740 - Flags: review?(roc)
Attachment #251740 - Flags: review+
(Assignee)

Comment 5

11 years ago
Created attachment 251744 [details] [diff] [review]
patch

Good idea.
Attachment #251740 - Attachment is obsolete: true
(Assignee)

Comment 6

11 years ago
Checked in to trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Summary: CSS counter values seem non-deterministic → CSS counter values seem non-deterministic once all counter uses have been removed
Flags: in-testsuite?
(Reporter)

Comment 7

11 years ago
Created attachment 257210 [details] [diff] [review]
reftest

In old builds, this fails in a different way than the original testcase.  Instead of appearing random, the last number is 4 instead of 2.  In new builds, it passes.
Attachment #257210 - Flags: review?(dbaron)
(Assignee)

Comment 8

11 years ago
Comment on attachment 257210 [details] [diff] [review]
reftest

r=dbaron
Attachment #257210 - Flags: review?(dbaron) → review+
(Reporter)

Comment 9

11 years ago
reftest checked in.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.