Bug 54352 - [GTK] Match more various WebKit API enum values with WebCore enum values
Summary: [GTK] Match more various WebKit API enum values with WebCore enum values
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-13 02:41 PST by Joone Hur
Modified: 2011-02-16 16:50 PST (History)
4 users (show)

See Also:


Attachments
Proposed Patch (7.56 KB, patch)
2011-02-13 04:13 PST, Joone Hur
no flags Details | Formatted Diff | Diff
Proposed Patch2 (7.64 KB, patch)
2011-02-13 04:19 PST, Joone Hur
mrobinson: review-
Details | Formatted Diff | Diff
Proposed Patch3 (6.49 KB, patch)
2011-02-16 02:33 PST, Joone Hur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joone Hur 2011-02-13 02:41:46 PST
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.
Comment 1 Joone Hur 2011-02-13 04:13:24 PST
Created attachment 82258 [details]
Proposed Patch
Comment 2 Joone Hur 2011-02-13 04:19:54 PST
Created attachment 82259 [details]
Proposed Patch2
Comment 3 Martin Robinson 2011-02-13 09:07:58 PST
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. :(
Comment 4 Joone Hur 2011-02-13 15:30:26 PST
(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);
Comment 5 Martin Robinson 2011-02-15 08:53:03 PST
(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.
Comment 6 Joone Hur 2011-02-16 02:33:31 PST
Created attachment 82605 [details]
Proposed Patch3

I applied Martin's suggestion to the patch. Thanks!
Comment 7 Xan Lopez 2011-02-16 02:54:31 PST
Comment on attachment 82605 [details]
Proposed Patch3

r=me
Comment 8 WebKit Commit Bot 2011-02-16 16:50:42 PST
Comment on attachment 82605 [details]
Proposed Patch3

Clearing flags on attachment: 82605

Committed r78749: <http://trac.webkit.org/changeset/78749>
Comment 9 WebKit Commit Bot 2011-02-16 16:50:48 PST
All reviewed patches have been landed.  Closing bug.