Closed Bug 1227377 Opened 4 years ago Closed 4 years ago

Change ImageRequest::mRequests to use nsIDocument as a key, instead of nsISupports

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

struct ImageValue has a hash table as a member-var, "mRequests".

For some reason, the key to this hash table is nsPtrHashKey<nsISupports>, but we always seem to use a nsIDocument* as the actual key.  And in the destructor, we static_cast the key to be a nsIDocument* (and assert that this is valid).

Seems like we should just make the key based on nsIDocument, rather than nsISupports.

I'm guessing that this is the way it is because this hash table was originally added (in bug 697230) with the following type:
>  nsInterfaceHashtable<nsISupportsHashKey, imgIRequest> mRequests; 
https://hg.mozilla.org/mozilla-central/rev/5c730c1f2274#l16.80

That type (nsInterfaceHashtable/nsISupportsHashKey) presumably didn't have a nsIDocument-specific variant (not sure).  Anyway, nowadays this hash table just has the following type:
> nsRefPtrHashtable<nsPtrHashKey<nsISupports>, imgRequestProxy> mRequests;
https://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSValue.h?rev=3c1cb9e4546c#150

...and it seems like we can swap in nsIDocument there & remove our now-unnecessary static-casts & assertions, for better type-safety.
https://hg.mozilla.org/mozilla-central/rev/5c730c1f2274#l16.80
Assignee: nobody → dholbert
Attached patch fix v1Splinter Review
Tagging khuey to review, since he added this hash table in bug 697230.

Note that this patch layers on top of attachment 8691150 [details] [diff] [review] (njn's bug 1187144 part 4).
Attachment #8691194 - Flags: review?(khuey)
I haven't tested this patch beyond
 (1) verifying that it builds successfully.
 (2) verifying that all users of this hash-table (ImageLoader.cpp in particular) seem to be passing in nsIDocument* pointers [which really must be the case, or else it wouldn't have built successfully]
Try run looks good. I noticed that I copypasted the wrong bug number into the commit message, too; fixed that locally.
Thanks! I'll land this once njn's bug 1187144 has landed (since this layers on top of that, and I don't want cause bitrot for him).
Flags: needinfo?(dholbert)
Flags: needinfo?(dholbert) → in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/ac08219f171c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.