Bug 157257

Summary:

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

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

Bindings

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, cdumez

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Darin Adler

Reported 2016-05-02 00:36:48 PDT

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

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 attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2016-05-02 01:01:12 PDT

Created attachment 277897 [details] Patch

Darin Adler

Comment 2 2016-05-02 01:01:50 PDT

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

Darin Adler

Comment 3 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.

Darin Adler

Comment 4 2016-05-02 08:26:18 PDT

Created attachment 277907 [details] Patch

Darin Adler

Comment 5 2016-05-02 08:27:06 PDT

New patch may fix the GTK and iOS builds.

Darin Adler

Comment 6 2016-05-02 08:47:11 PDT

Created attachment 277908 [details] Patch

Chris Dumez

Comment 7 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.

Darin Adler

Comment 8 2016-05-02 09:00:59 PDT

Created attachment 277909 [details] Patch

Chris Dumez

Comment 9 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.

Chris Dumez

Comment 10 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.

Darin Adler

Comment 11 2016-05-02 09:44:15 PDT

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

Darin Adler

Comment 12 2016-05-02 22:20:40 PDT

Created attachment 277982 [details] Patch

Darin Adler

Comment 13 2016-05-02 22:35:23 PDT

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

Darin Adler

Comment 14 2016-05-02 22:46:59 PDT

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

Note You need to log in before you can comment on or make changes to this bug.