1421197 - use 64-bit for restyle generation counters to avoid overflow issues

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Jeremy Chen ([e-mail])]

We use 32-bit counters while counting the restyle generations [1], but think of getting the generations as 64-bit counters [2]. I believe we do want to use 64-bit for all the generation counters.

I did a bit archaeology and found that this is something originated from bug 1203766 comment 13.



[1] https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/layout/base/RestyleManager.h#266-268

[2] https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/layout/base/nsPresContext.h#1034-1035

Comment 1

[Cameron McCormack (:heycam)]

Ah, yes I filed this one a few months ago too, but never got around to it...

Comment 2

[Jeremy Chen ([e-mail])]

Attachment: Bug 1421197 - use 64-bit for restyle generation counters to avoid overflow issues.

It is expected to use 64-bit for all the restyle generation counters, since the
getter methods all return uint64_t type at present. However, we're using uint32_t
for the actual counter variables, which means the potential overflow issue is not
avoided.

In this patch, we use 64-bit for all the restyle generation counters, so the
overflow issue can be avoided as expected.

Review commit: https://reviewboard.mozilla.org/r/203434/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203434/

Comment 3

[Jeremy Chen ([e-mail])]

Comment on attachment 8932377 [details]
Bug 1421197 - use 64-bit for restyle generation counters to avoid overflow issues.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/203434/diff/1-2/

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8932377 [details]
Bug 1421197 - use 64-bit for restyle generation counters to avoid overflow issues.

https://reviewboard.mozilla.org/r/203434/#review208874

The hover generation doesn't have the same problem of consumers thinking it should be a uint64_t but we're storing it as uint32_t.  The consumers like https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/layout/base/PresShell.cpp#3773-3774 are using uint32_t.  So I don't think we should change mHoverGeneration unless we also change the consumers.  And I think it's probably fine to leave mHoverGeneration as it is.  So r=me with mHoverGeneration unchanged.

Comment 5

[Jeremy Chen ([e-mail])]

(In reply to Cameron McCormack (:heycam) from comment #4)
> Comment on attachment 8932377 [details]
> Bug 1421197 - use 64-bit for restyle generation counters to avoid overflow
> issues.
> 
> https://reviewboard.mozilla.org/r/203434/#review208874
> 
> The hover generation doesn't have the same problem of consumers thinking it
> should be a uint64_t but we're storing it as uint32_t.  The consumers like
> https://searchfox.org/mozilla-central/rev/
> 7a8c667bdd2a4a32746c9862356e199627c0896d/layout/base/PresShell.cpp#3773-3774
> are using uint32_t.  So I don't think we should change mHoverGeneration
> unless we also change the consumers.  And I think it's probably fine to
> leave mHoverGeneration as it is.  So r=me with mHoverGeneration unchanged.

Ok, I'll leave mHoverGeneration unchanged.

Sorry for not removing the review request in this bug, actually, I did uploaded the patch to bug 1376117, but somehow forgot to remove the request here. Set the duplicate flag in the other way around since we're going to land the patch here.

Comment 7

[Jeremy Chen ([e-mail])]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75af66e6a371f30a2b60b592a35bae8921ad6670

Comment 8

[Jeremy Chen ([e-mail])]

Comment on attachment 8932377 [details]
Bug 1421197 - use 64-bit for restyle generation counters to avoid overflow issues.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/203434/diff/2-3/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/660d64624ea9
use 64-bit for restyle generation counters to avoid overflow issues. r=heycam

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/660d64624ea9