Closed Bug 803438 Opened 7 years ago Closed 7 years ago

Convert the preflight cache to use mozilla/LinkedList.h rather than prclist.h

Categories

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

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: Waldo, Assigned: Waldo)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
prclist.h is grody macros, LinkedList.h is the new hotness (and still type-safe, and moreover full of delicious sanity-checking assertions).  And prclist.h includes prtypes.h, which is on ehsan's hit list.  :-)

I'm reasonably certain, given locally running some of content/ mochitests, that this should be good, but I pushed to try just to be safe:

https://tbpl.mozilla.org/?tree=Try&rev=1b1890873105

I don't really plan on doing conversions beyond this one, but one good example seems worthwhile so that each individual transition can be a good first (or maybe second) bug.   Also it seems worthwhile in order to consider whether the LinkedList interface is as good/complete as it could be.  Just doing this one conversion points out one, maybe two, maybe three, maybe even four issues worth considering.  This is the basic patch, and I'll file a followup to discuss those other issues separately.
Attachment #673104 - Flags: review?(justin.lebar+bug)
Blocks: 803439
Blocks: 803441
Comment on attachment 673104 [details] [diff] [review]
Patch

Looks good. My only complaint is the 4 lines of context.  git bz [1] will ensure you always get 8 lines.

[1] https://github.com/jlebar/moz-git-tools
Attachment #673104 - Flags: review?(justin.lebar+bug) → review+
But that would require using git.  And [diff] unified = 8 would work just as well there, if I didn't intentionally have unified = 3.  :-P

https://hg.mozilla.org/integration/mozilla-inbound/rev/499ca9ee03bc
Target Milestone: --- → mozilla19
> But that would require using git

Ah, well, if you're on hg, you have no excuse.  :)

> And [diff] unified = 8 would work just as well there, if I didn't intentionally have unified = 3.  
> :-P

I'm not sure what you mean, but I don't believe you can make git diff default to -U8.  You have to create an alias, e.g.

    [alias]
    dif = diff -U8 --patience -M -C
Er, sorry, that might have been clearer without "there" -- the syntax bits were referring to ~/.hgrc.
...wait.  Did I just miss a chance to snark about hg being better than git?  I am falling down on the job!  I should be ashamed to show my face in public.  ;-)
Comment on attachment 673104 [details] [diff] [review]
Patch

Review of attachment 673104 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsCrossSiteListenerProxy.cpp
@@ +219,4 @@
>      // If that didn't remove anything then kick out the least recently used
>      // entry.
>      if (mTable.Count() == PREFLIGHT_CACHE_SIZE) {
> +      CacheEntry* lruEntry = static_cast<CacheEntry*>(mList.popLast());

Do you still need that cast?
https://hg.mozilla.org/mozilla-central/rev/499ca9ee03bc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.