Bug 113473 – Rename toScriptElement to differentiate it from other to* functions and better describe its function

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Philip Rogers]

ScriptElement::toScriptElement is different from other to* functions (e.g., Element::toElement, etc) in that it returns 0 if the cast is unsuccessful. These functions have security implications and need to be clear and consistent.
http://trac.webkit.org/browser/trunk/Source/WebCore/dom/ScriptElement.cpp#L417

We should rename toScriptElement to toScriptElementIfPossible (or similar) both to clarify the function behavior and to differentiate it from other to* functions.

[Geoffrey Garen]

In JSC, we use jsCast<T> and jsDynamicCast<T>, to mimic static_cast<T> and dynamic_cast<T>.

Ryosuke and I were discussing using domCast<T> or coreCast<T>, along with domDynamicCast<T> or coreDynamicCast<T> for the DOM. This would allow for specialized implementations while keeping the interface consistent at all call sites.

[Philip Rogers]

(In reply to comment #1)
> In JSC, we use jsCast<T> and jsDynamicCast<T>, to mimic static_cast<T> and dynamic_cast<T>.
> 
> Ryosuke and I were discussing using domCast<T> or coreCast<T>, along with domDynamicCast<T> or coreDynamicCast<T> for the DOM. This would allow for specialized implementations while keeping the interface consistent at all call sites.

I think Abhishek is working on something similar to domCast<T> to help reduce the security issues involved in our many static_casts. My understanding is that we will only return 0 for null casts, so the toScriptElement function is really an odd one out.

I was not aware of jsDynamicCast before, but I like it. Abhishek, is there a bug that's tracking this work?

[Philip Rogers]

Created attachment 195478 [details]
Patch that just handles the rename

[WebKit Review Bot]

Comment on attachment 195478 [details]
Patch that just handles the rename

Clearing flags on attachment: 195478

Committed r147057: <http://trac.webkit.org/changeset/147057>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.