Bug 157257 - Change IDL enumerations to be nested in their C++ class instead of at WebCore namespace level
Summary: Change IDL enumerations to be nested in their C++ class instead of at WebCore...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-02 00:36 PDT by Darin Adler
Modified: 2016-05-02 22:46 PDT (History)
2 users (show)

See Also:

Attachments
Patch (183.90 KB, patch)
2016-05-02 01:01 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (184.62 KB, patch)
2016-05-02 08:26 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (186.93 KB, patch)
2016-05-02 08:47 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (186.94 KB, patch)
2016-05-02 09:00 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (189.23 KB, patch)
2016-05-02 22:20 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2016-05-02 00:36:48 PDT 
Change IDL enumerations to be nested in their C++ class instead of at WebCore namespace level
Comment 1 Darin Adler 2016-05-02 01:01:12 PDT 
Created attachment 277897 [details]
Patch
Comment 2 Darin Adler 2016-05-02 01:01:50 PDT 
OK, Chris, Alex, here is the change we discussed.
Comment 3 Darin Adler 2016-05-02 07:19:02 PDT 
Build failures on GTK and iOS should be quick to fix; just a couple call sites that need the class prefixes.
Comment 4 Darin Adler 2016-05-02 08:26:18 PDT 
Created attachment 277907 [details]
Patch
Comment 5 Darin Adler 2016-05-02 08:27:06 PDT 
New patch may fix the GTK and iOS builds.
Comment 6 Darin Adler 2016-05-02 08:47:11 PDT 
Created attachment 277908 [details]
Patch
Comment 7 Chris Dumez 2016-05-02 08:59:42 PDT 
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.
Comment 8 Darin Adler 2016-05-02 09:00:59 PDT 
Created attachment 277909 [details]
Patch
Comment 9 Chris Dumez 2016-05-02 09:21:43 PDT 
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.
Comment 10 Chris Dumez 2016-05-02 09:22:43 PDT 
(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.
Comment 11 Darin Adler 2016-05-02 09:44:15 PDT 
I’ll land this tonight, I guess, after fixing iOS.
Comment 12 Darin Adler 2016-05-02 22:20:40 PDT 
Created attachment 277982 [details]
Patch
Comment 13 Darin Adler 2016-05-02 22:35:23 PDT 
Will land this as soon as I see the iOS build succeeding on the EWS.
Comment 14 Darin Adler 2016-05-02 22:46:59 PDT 
Committed r200361: <http://trac.webkit.org/changeset/200361>