Bug 112225 – Replace static_casts with to* functions for document types

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

[Abhishek Arya]

Replace static_casts with to* functions for document types

[Abhishek Arya]

Created attachment 192866 [details]
Patch

[Abhishek Arya]

Created attachment 192867 [details]
Patch

[Early Warning System Bot]

Comment on attachment 192867 [details]
Patch

Attachment 192867 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17160215

[Early Warning System Bot]

Comment on attachment 192867 [details]
Patch

Attachment 192867 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17010936

[EFL EWS Bot]

Comment on attachment 192867 [details]
Patch

Attachment 192867 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17080442

[Abhishek Arya]

Created attachment 192868 [details]
Patch

[Early Warning System Bot]

Comment on attachment 192868 [details]
Patch

Attachment 192868 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17060722

[Early Warning System Bot]

Comment on attachment 192868 [details]
Patch

Attachment 192868 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17060725

[Build Bot]

Comment on attachment 192868 [details]
Patch

Attachment 192868 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17208050

New failing tests:
userscripts/user-script-plugin-document.html
http/tests/security/contentSecurityPolicy/object-src-url-allowed.html
plugins/return-error-from-new-stream-callback-in-full-frame-plugin.html
plugins/plugin-document-back-forward.html
fast/frames/iframe-plugin-load-remove-document-crash.html
plugins/iframe-plugin-bgcolor.html
http/tests/security/contentSecurityPolicy/object-src-url-blocked.html
fast/loader/reload-zero-byte-plugin.html

[Abhishek Arya]

Created attachment 192875 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 192875 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192875&action=review

Please be sure to run tests by a debug build yourself before landing it.

> Source/WebCore/html/ImageDocument.h:88
> +// This will catch anyone doing an unnecessary cast.
> +void toImageDocument(const ImageDocument*);

I'm not sure if this is really useful.

> Source/WebCore/html/MediaDocument.h:71
> +// This will catch anyone doing an unnecessary cast.
> +void toMediaDocument(const MediaDocument*);

Ditto.

[Abhishek Arya]

(In reply to comment #11)
> (From update of attachment 192875 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192875&action=review
> 
> Please be sure to run tests by a debug build yourself before landing it.

Sure.

> 
> > Source/WebCore/html/ImageDocument.h:88
> > +// This will catch anyone doing an unnecessary cast.
> > +void toImageDocument(const ImageDocument*);
> 
> I'm not sure if this is really useful.
> 

actually it helped twice yesterday since compile will fail if something is already the same type and we try to call static_cast on it.

> > Source/WebCore/html/MediaDocument.h:71
> > +// This will catch anyone doing an unnecessary cast.
> > +void toMediaDocument(const MediaDocument*);
> 
> Ditto.

[Philip Rogers]

Comment on attachment 192875 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192875&action=review

> Source/WebCore/Modules/notifications/Notification.cpp:261
>      ASSERT_WITH_SECURITY_IMPLICATION(context->isDocument());

This is no longer necessary (here, and elsewhere). Do you mind removing these extra assertions?

[Abhishek Arya]

(In reply to comment #13)
> (From update of attachment 192875 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192875&action=review
> 
> > Source/WebCore/Modules/notifications/Notification.cpp:261
> >      ASSERT_WITH_SECURITY_IMPLICATION(context->isDocument());
> 
> This is no longer necessary (here, and elsewhere). Do you mind removing these extra assertions?

Yes, forgot some of those, will remove before landing.

[Abhishek Arya]

Created attachment 192968 [details]
Patch

[WebKit Review Bot]

Comment on attachment 192968 [details]
Patch

Clearing flags on attachment: 192968

Committed r145745: <http://trac.webkit.org/changeset/145745>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.