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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

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

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

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).

Comment 2

[Daniel Holbert [:dholbert]]

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]

Comment 3

[Daniel Holbert [:dholbert]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=47fe610699af

Comment 4

[Daniel Holbert [:dholbert]]

Try run looks good. I noticed that I copypasted the wrong bug number into the commit message, too; fixed that locally.

Comment 5

[Daniel Holbert [:dholbert]]

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).

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac08219f171c

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ac08219f171c