Bug 33317 – Document::minimumLayoutDelay() should return unsigned, rather than int

• Status: ASSIGNED
• Component: WebCore Misc.

[Steve Block]

Document::minimumLayoutDelay() should return unsigned, rather than int. Correspondingly, the variables and helper functions used in this method should be updated to use unsigned. See Bug 32875.

[Steve Block]

Created attachment 46061 [details]
Patch 1 for Bug 33317

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 46061 [details] without any errors.

[Darin Adler]

Comment on attachment 46061 [details]
Patch 1 for Bug 33317

The other changes seem ideal, but I am slightly worried about elapsedTime. Are you sure we won't change behavior here by turning "moving backward in time" cases that might occur due to time changes into "move forward by MAX_UINT" cases?

[Steve Block]

(In reply to comment #3)
> (From update of attachment 46061 [details])
> The other changes seem ideal, but I am slightly worried about elapsedTime. Are
> you sure we won't change behavior here by turning "moving backward in time"
> cases that might occur due to time changes into "move forward by MAX_UINT"
> cases?
Good point. Since the result of elapsedTime is always intended to be non-negative, presumably the right thing to do is to have it return the following?

currentTime() > m_startTime ? currentTime() - m_startTime : 0

This is a behavior change from "moving backward in time" to "no movement in time", but makes it more explicit.

[Darin Adler]

(In reply to comment #4)
> (In reply to comment #3)
> Good point. Since the result of elapsedTime is always intended to be
> non-negative, presumably the right thing to do is to have it return the
> following?
> 
> currentTime() > m_startTime ? currentTime() - m_startTime : 0
> 
> This is a behavior change from "moving backward in time" to "no movement in
> time", but makes it more explicit.

Sure, seems good. The key is probably to find a way to test it.

[David Levin]

Comment on attachment 46061 [details]
Patch 1 for Bug 33317

r- for two reasons:
1. This patch/changelog doesn't mention why this needs to be done only what (and I personally hate unsigned -- been burned in the past by subtle bugs of the nature that Darin mentioned). If there was a "why", perhaps we could discuss alternatives :)

2. Is there a way to address Darin Adler's last comment?

[Steve Block]

> 1. This patch/changelog doesn't mention why this needs to be done only what
The only motivation was Eric's comment on the bug in which I added minimumLayoutDelay. See https://bugs.webkit.org/show_bug.cgi?id=32875#c14 and later comments in that bug.

> 2. Is there a way to address Darin Adler's last comment?
I haven't yet looked into testing.