Bug 157257 – Change IDL enumerations to be nested in their C++ class instead of at WebCore namespace level

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Darin Adler]

Change IDL enumerations to be nested in their C++ class instead of at WebCore namespace level

[Darin Adler]

Created attachment 277897 [details]
Patch

[Darin Adler]

OK, Chris, Alex, here is the change we discussed.

[Darin Adler]

Build failures on GTK and iOS should be quick to fix; just a couple call sites that need the class prefixes.

[Darin Adler]

Created attachment 277907 [details]
Patch

[Darin Adler]

New patch may fix the GTK and iOS builds.

[Darin Adler]

Created attachment 277908 [details]
Patch

[Chris Dumez]

iOS still does not build:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:404:75: error: no type named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
static inline HTMLMediaElementEnums::VideoFullscreenMode toFullscreenMode(HTMLMediaElement::VideoPresentationMode mode)
                                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                                          HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:407:10: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
    case HTMLMediaElement::VideoPresentationMode::Fullscreen:
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:409:10: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
    case HTMLMediaElement::VideoPresentationMode::PictureInPicture:
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:411:10: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
    case HTMLMediaElement::VideoPresentationMode::Inline:
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:436:8: error: no type named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
static HTMLMediaElement::VideoPresentationMode toPresentationMode(HTMLMediaElement::VideoFullscreenMode mode)
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:439:16: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
        return HTMLMediaElement::VideoPresentationMode::Fullscreen;
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:442:16: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
        return HTMLMediaElement::VideoPresentationMode::PictureInPicture;
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:445:16: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
        return HTMLMediaElement::VideoPresentationMode::Inline;
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:448:12: error: no member named 'VideoPresentationMode' in 'WebCore::HTMLMediaElement'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
    return HTMLMediaElement::VideoPresentationMode::Inline;
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:451:1: error: unknown type name 'VideoPresentationMode'; did you mean 'HTMLVideoElement::VideoPresentationMode'?
VideoPresentationMode HTMLVideoElement::webkitPresentationMode() const
^~~~~~~~~~~~~~~~~~~~~
HTMLVideoElement::VideoPresentationMode
In file included from /Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.cpp:28:
/Volumes/Data/EWS/WebKit/Source/WebCore/html/HTMLVideoElement.h:81:16: note: 'HTMLVideoElement::VideoPresentationMode' declared here
    enum class VideoPresentationMode { Fullscreen, PictureInPicture, Inline };
               ^
10 errors generated.

[Darin Adler]

Created attachment 277909 [details]
Patch

[Chris Dumez]

Comment on attachment 277909 [details]
Patch

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

r=me

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:815
> +sub PrefixIsWordsThatMatchStartOrEndWords

I find this name to bit unclear. If we cannot find a better name, I suggest a comment explaining what this routine does.

[Chris Dumez]

(In reply to comment #9)
> Comment on attachment 277909 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=277909&action=review
> 
> r=me
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:815
> > +sub PrefixIsWordsThatMatchStartOrEndWords
> 
> I find this name to bit unclear. If we cannot find a better name, I suggest
> a comment explaining what this routine does.

BTW, I think the code looks much nicer now after this change.

However, it looks like iOS is still red.

[Darin Adler]

I’ll land this tonight, I guess, after fixing iOS.

[Darin Adler]

Created attachment 277982 [details]
Patch

[Darin Adler]

Will land this as soon as I see the iOS build succeeding on the EWS.

[Darin Adler]

Committed r200361: <http://trac.webkit.org/changeset/200361>