Bug 120617 – Change type of Document::doctype back to a raw pointer

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Darin Adler]

Change type of Document::doctype back to a raw pointer

[Darin Adler]

Created attachment 210327 [details]
Patch

[Darin Adler]

Kling, did you have a good reason to change this to a RefPtr?

[Andreas Kling]

Comment on attachment 210327 [details]
Patch

Sure. r=me

You should change documentTypeString() in markup.cpp back as well.

[Andreas Kling]

(In reply to comment #2)
> Kling, did you have a good reason to change this to a RefPtr?

Not a great one.

I felt that since this function is not performance-critical and its primary purpose is providing web API, we might as well tailor it to that use case. If that makes it a little more awkward to use it internally, all the better.

BUT now that I look closer, it appears that JSC bindings don't actually handle PassRefPtr return values so well, and we end up with an extra ref/deref pair! :(

[Darin Adler]

(In reply to comment #4)
> I felt that since this function is not performance-critical and its primary purpose is providing web API, we might as well tailor it to that use case. If that makes it a little more awkward to use it internally, all the better.

It seems OK to me to put the ref into the bindings rather than into the function in the DOM itself. In my opinion, functions should not return a PassRefPtr if it they don’t need to. For one thing, if there's already a wrapper there is no need to ref the pointer.

> BUT now that I look closer, it appears that JSC bindings don't actually handle PassRefPtr return values so well, and we end up with an extra ref/deref pair! :(

Oh, we should fix that.

[Darin Adler]

Committed r155019: <http://trac.webkit.org/changeset/155019>

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.