542632 - Protect nsGenericHTMLElement::GetHrefURIForAnchors

Status: RESOLVED FIXED

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

Description

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

We should protect it so consumers no longer access it.  This also makes the dns prefetch code use the cached URI.

Comment 1

[Boris Zbarsky [:bzbarsky]]

>+      Link *link = static_cast<Link *>(static_cast<nsISupports *>(content.get()));

Why is this cast ok?

Comment 2

[Shawn Wilsher :sdwilsh]

Because we only ever add Link elements to this queue, which are stored as weak references.  We QI the weak reference to nsIContent, so we can test if it is in the document (and exists).  We already know it's a Link for sure (we added it), so we have to cast down to nsISupports since that is what Link inherits from, and then cast to Link.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> so we have to cast down to nsISupports since that is what Link inherits from,
> and then cast to Link

But the nsISupports that nsIContent inherits from (and casts to) is not the same as the nsISupports Link inherits from, is it?

I would fully expect this code to either crash or behave really oddly if actually executed.

Comment 4

[Shawn Wilsher :sdwilsh]

Bah, that's right.  What's the right way to get to Link in this case?  Do I have to add an IID for it to QI to?

Comment 5

[Boris Zbarsky [:bzbarsky]]

That seems like the simplest way to go...

Comment 6

[Shawn Wilsher :sdwilsh]

Darn, OK.  New patch soon.

Comment 7

[Shawn Wilsher :sdwilsh]

I'm actually not sure how to do this QI and not destroy the other QI's.  Thoughts?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Destroy in what sense?

Comment 9

[Shawn Wilsher :sdwilsh]

Nevermind.  I wasn't thinking.  The classes that implement Link do the QI implementation.

Comment 10

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

I tried making that far harder than it had to be...

Comment 11

[Shawn Wilsher :sdwilsh]

Comment on attachment 426369 [details] [diff] [review]
v1.1

Build finished, but this patch isn't right.  Arg (startup crash dereferencing a null pointer)

Comment 12

[Shawn Wilsher :sdwilsh]

Attachment: v1.2

OK, this one actually runs.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 426386 [details] [diff] [review]
v1.2

>+++ b/content/html/content/src/nsHTMLAnchorElement.cpp
>   NS_HTML_CONTENT_INTERFACE_TABLE4(nsHTMLAnchorElement,

Needs to become TABLE5.

r=bzbarsky

Comment 14

[Shawn Wilsher :sdwilsh]

(In reply to comment #13)
> Needs to become TABLE5.
Fixed locally.

Comment 15

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/2777f7baaaad