Closed
Bug 803665
Opened 12 years ago
Closed 12 years ago
convert nsGlobalWindow's timeout list to use mozilla::LinkedList
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: froydnj, Assigned: froydnj)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
16.52 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
12.45 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
895 bytes,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
975 bytes,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #673384 -
Flags: review?(bzbarsky)
Comment 2•12 years ago
|
||
Comment on attachment 673384 [details] [diff] [review] convert nsGlobalWindow's timeout list to use mozilla::LinkedList r=me, though seems like we could nix the Next/Prev/FirstTimeout/LastTimeout stuff if we wanted to now that they have sane implementations.
Attachment #673384 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #2) > r=me, though seems like we could nix the Next/Prev/FirstTimeout/LastTimeout > stuff if we wanted to now that they have sane implementations. Agreed. I thought it would be easier to do that in a followup patch.
Comment 4•12 years ago
|
||
Sure. Followup is great.
Assignee | ||
Comment 5•12 years ago
|
||
The LinkedList<T> methods work just fine; no need to have wrappers around them.
Attachment #673913 -
Flags: review?(bzbarsky)
Comment 6•12 years ago
|
||
Comment on attachment 673913 [details] [diff] [review] Bug 803665 - part 2 - dispense with nsGlobalWindow::{First,Last}Timeout and nsTimeout::{Next,Previous}; r=bz r=me
Attachment #673913 -
Flags: review?(bzbarsky) → review+
Comment 7•12 years ago
|
||
Yay! One step closer to sanity for nsTimeout.
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/24d8e0cb73f2 https://hg.mozilla.org/integration/mozilla-inbound/rev/fb673c95d56b
Assignee | ||
Comment 9•12 years ago
|
||
Hunting down some segfaults from failing tests pointed to nsTimeout's bogus constructor. By using memset(), it breaks LinkedListElement's invariants. Let's not do that. mccr8 has this bit in his patches for bug 774874, too.
Attachment #677448 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 10•12 years ago
|
||
This small patch corrects a conceptual mismatch between prclist and LinkedList: when we're walking backwards through the list, getting to the end of the list is represented by a sentinel node in prclist and nullptr in LinkedList. So we have to check for that condition specially. Since we're walking backwards through the list, .insertFront makes sense to me, and tests seem to work, but asking for review as a sanity check. This will get folded into an earlier patch in any event.
Attachment #677451 -
Flags: review?(bzbarsky)
Comment 11•12 years ago
|
||
Comment on attachment 677448 [details] [diff] [review] part 0 - make nsTimeout properly initialize its fields r=me
Attachment #677448 -
Flags: review?(bzbarsky) → review+
Comment 12•12 years ago
|
||
Comment on attachment 677451 [details] [diff] [review] small fix for inserting timeouts, to be folded Indeed. Sorry I didn't catch this. :(
Attachment #677451 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #12) > Comment on attachment 677451 [details] [diff] [review] > small fix for inserting timeouts, to be folded > > Indeed. Sorry I didn't catch this. :( No worries! My job to actually run tests, not yours. Re-landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/e2e32c3c7c89 https://hg.mozilla.org/integration/mozilla-inbound/rev/b6bd66fc8988 https://hg.mozilla.org/integration/mozilla-inbound/rev/b3a6bc0d9df5
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e2e32c3c7c89 https://hg.mozilla.org/mozilla-central/rev/b6bd66fc8988 https://hg.mozilla.org/mozilla-central/rev/b3a6bc0d9df5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment 15•12 years ago
|
||
Unfortunately the inbound push this landed in broke Linux PGO, so push backed out (see bug 809756). https://hg.mozilla.org/mozilla-central/rev/82c59210f11f
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla19 → ---
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7f38df5f2448 https://hg.mozilla.org/mozilla-central/rev/00534980e998 https://hg.mozilla.org/mozilla-central/rev/6a9e7aeb184f
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•