Bug 178824 – Add downcast support for FullscreenClient.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Jeremy Jones]

Add downcast support for FullscreenClient.

[Jeremy Jones]

Created attachment 324878 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 324878 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/FullscreenClient.h:44
> +    static int type;
> +    bool isType(void* target) const override { return target == &type || API::FullscreenClient::isType(target);};

This is really weird. Why the generic types (int and void*) and not enums and classes?

[Jeremy Jones]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 324878 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324878&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/FullscreenClient.h:44
> > +    static int type;
> > +    bool isType(void* target) const override { return target == &type || API::FullscreenClient::isType(target);};
> 
> This is really weird. Why the generic types (int and void*) and not enums
> and classes?

Are you suggesting something like:

    typedef int Type;
    typedef void* TypeRef;
    ...
    static Type type;
    bool isType(TypeRef target);


Or are you suggesting API::FullscreenClient have something like:

    enum FullscreenClientType {
        APIFullscreenClientType,
        WebKitFullscreenClientType
    };
    ...
    bool isType(FullscreenClientType type);

In the latter case, it seems weird to me for the base class to be aware of all derived classes.

[Simon Fraser (smfr)]

> Are you suggesting something like:
> 
>     typedef int Type;
>     typedef void* TypeRef;
>     ...
>     static Type type;
>     bool isType(TypeRef target);
> 
> 
> Or are you suggesting API::FullscreenClient have something like:
> 
>     enum FullscreenClientType {
>         APIFullscreenClientType,
>         WebKitFullscreenClientType
>     };
>     ...
>     bool isType(FullscreenClientType type);
> 
> In the latter case, it seems weird to me for the base class to be aware of
> all derived classes.

The latter. We do this all over the place.

[Jeremy Jones]

Created attachment 326167 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 326167 [details]
Patch

Clearing flags on attachment: 326167

Committed r224558: <https://trac.webkit.org/changeset/224558>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35568682>