Closed Bug 183604 Opened 22 years ago Closed 22 years ago

The DOM code should re-use the "OS" timer for inteval timers

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.3beta

People

(Reporter: jst, Assigned: jst)

Details

(Whiteboard: [HAVE FIX])

Attachments

(3 files, 1 obsolete file)

Re-initialize the existing timer for inteval timeouts
(deleted), patch
Details | Diff | Splinter Review
diff -w
(deleted), patch
caillon
: review+
peterv
: superreview+
Details | Diff | Splinter Review
Final complete patch with the minor tweak to isInteval's initialization.
(deleted), patch
Details | Diff | Splinter Review
Now that the timer code supports re-initializeing existing timers we should take advantage of that in the DOM timeout code. Patch coming up.
Attachment #108274 - Flags: superreview?(bzbarsky)
Attachment #108274 - Flags: review?(caillon)
Comment on attachment 108274 [details] [diff] [review] Don't re-create timers for inteval timeouts, re-use the existing one. Man, a diff -w of some sorts would have really been nice here. >+ // Reschedule timeout. Ignore any errors since nobody who >+ // listens them will care anyways. listens _to_ them r=caillon
Attachment #108274 - Flags: review?(caillon) → review+
Comment on attachment 108274 [details] [diff] [review] Don't re-create timers for inteval timeouts, re-use the existing one. sr=bzbarsky with that comment change.
Attachment #108274 - Flags: superreview?(bzbarsky) → superreview+
Comment changed, I looked at a diff -w of this before attaching the diff, but it didn't actually help much so I ended up not attaching it. Thanks for the reviews.
Status: NEW → ASSIGNED
Priority: -- → P3
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.3beta
Comment on attachment 108274 [details] [diff] [review] Don't re-create timers for inteval timeouts, re-use the existing one. Bah. That didn't work, I don't know what I was thinking... New patch coming up...
Attachment #108274 - Attachment is obsolete: true
This fixes this bug, for real this time. This also eliminates a bunch of silly assumptions about who owns what based on the existance of a OS timer or not. This should be far more maintainable from here on. diff -w coming up of this one.
Attached patch diff -w (deleted) — — Splinter Review
Attachment #108963 - Flags: superreview?(bzbarsky)
Attachment #108963 - Flags: review?(caillon)
Comment on attachment 108963 [details] [diff] [review] diff -w I thought I marked this already with r=caillon. Anyway, I think that bz is on vacation now, so unless you wanna wait like a month or so, maybe peterv should sr this.
Attachment #108963 - Flags: review?(caillon) → review+
Attachment #108963 - Flags: superreview?(bzbarsky) → superreview?(peterv)
Comment on attachment 108963 [details] [diff] [review] diff -w sr=peterv with the minor change you wanted to do for isInterval.
Attachment #108963 - Flags: superreview?(peterv) → superreview+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: