Bug 54352 – [GTK] Match more various WebKit API enum values with WebCore enum values

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Joone Hur]

AssertMatchingEnums.cpp was added to assert that various
WebKit API enum values continue matching WebCore defined enum values in the following changeset.
http://trac.webkit.org/changeset/77868

I found more enum values to be asserted, so the attached patch includes them.

[Joone Hur]

Created attachment 82258 [details]
Proposed Patch

[Joone Hur]

Created attachment 82259 [details]
Proposed Patch2

[Martin Robinson]

Comment on attachment 82259 [details]
Proposed Patch2

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

This is awesome, though we cannot break the ABI. Without the last change it's fine though.

> Source/WebKit/gtk/webkit/webkitwebview.h:64
> +    WEBKIT_WEB_VIEW_VIEW_MODE_INVALID,

I think this might be an ABI break. :(

[Joone Hur]

(In reply to comment #3)
> (From update of attachment 82259 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=82259&action=review
> 
> This is awesome, though we cannot break the ABI. Without the last change it's fine though.
> 
> > Source/WebKit/gtk/webkit/webkitwebview.h:64
> > +    WEBKIT_WEB_VIEW_VIEW_MODE_INVALID,
> 
> I think this might be an ABI break. :(

If we keep the ABI, the patch should be updated as follows,
But, I'm not sure if this is neat.

COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_WINDOWED+1, Page::ViewModeWindowed);
COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_FLOATING+1, Page::ViewModeFloating);
COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_FULLSCREEN+1, Page::ViewModeFullscreen);
COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_MAXIMIZED+1, Page::ViewModeMaximized);
COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_MINIMIZED+1, Page::ViewModeMinimized);

[Martin Robinson]

(In reply to comment #4)
> If we keep the ABI, the patch should be updated as follows,
> But, I'm not sure if this is neat.
> 
> COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_WINDOWED+1, Page::ViewModeWindowed);
> COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_FLOATING+1, Page::ViewModeFloating);
> COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_FULLSCREEN+1, Page::ViewModeFullscreen);
> COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_MAXIMIZED+1, Page::ViewModeMaximized);
> COMPILE_ASSERT_MATCHING_ENUM(WEBKIT_WEB_VIEW_VIEW_MODE_MINIMIZED+1, Page::ViewModeMinimized);

Essentially if there is no place where you cast directly between the two enum types, they do not need to be in AssertMatchingEnums.cpp. AssertMatchingEnums.cpp just ensures that a cast between the two types is valid.

[Joone Hur]

Created attachment 82605 [details]
Proposed Patch3

I applied Martin's suggestion to the patch. Thanks!

[Xan Lopez]

Comment on attachment 82605 [details]
Proposed Patch3

r=me

[WebKit Commit Bot]

Comment on attachment 82605 [details]
Proposed Patch3

Clearing flags on attachment: 82605

Committed r78749: <http://trac.webkit.org/changeset/78749>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.