Bug 208830 - Remove obsolete feature flags
Summary: Remove obsolete feature flags
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-09 13:42 PDT by Don Olmstead
Modified: 2020-03-09 19:32 PDT (History)
24 users (show)

See Also:


Attachments
WIP Patch (71.59 KB, patch)
2020-03-09 13:43 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (70.26 KB, patch)
2020-03-09 14:51 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (77.54 KB, patch)
2020-03-09 15:18 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (72.73 KB, patch)
2020-03-09 17:21 PDT, Don Olmstead
achristensen: review-
Details | Formatted Diff | Diff
Patch (74.00 KB, patch)
2020-03-09 17:36 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2020-03-09 13:42:30 PDT
Remove any obsolete ENABLE flags from the build system. Find any misused ENABLE flags.
Comment 1 Don Olmstead 2020-03-09 13:43:08 PDT Comment hidden (obsolete)
Comment 2 Don Olmstead 2020-03-09 14:51:54 PDT Comment hidden (obsolete)
Comment 3 Jonathan Bedard 2020-03-09 15:09:51 PDT
(In reply to Don Olmstead from comment #0)
> Remove any obsolete ENABLE flags from the build system. Find any misused
> ENABLE flags.

Could we make the bug title a bit more descriptive? Something like 'Remove obsolete feature flags'. Cleanup feature defines could refer to a few different things.
Comment 4 Don Olmstead 2020-03-09 15:18:19 PDT Comment hidden (obsolete)
Comment 5 Don Olmstead 2020-03-09 17:21:11 PDT
Created attachment 393095 [details]
Patch

Don't try and sort anything in FeatureDefines.xcconfig.
Comment 6 Alex Christensen 2020-03-09 17:27:44 PDT
Comment on attachment 393095 [details]
Patch

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

> Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig:-314
> -ENABLE_POINTER_EVENTS = $(ENABLE_POINTER_EVENTS_$(WK_PLATFORM_NAME));

This is very much still used
Comment 7 Don Olmstead 2020-03-09 17:30:04 PDT
(In reply to Alex Christensen from comment #6)
> Comment on attachment 393095 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=393095&action=review
> 
> > Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig:-314
> > -ENABLE_POINTER_EVENTS = $(ENABLE_POINTER_EVENTS_$(WK_PLATFORM_NAME));
> 
> This is very much still used

https://trac.webkit.org/changeset/258148/webkit
Comment 8 Don Olmstead 2020-03-09 17:36:16 PDT
Created attachment 393099 [details]
Patch

Reuploading to make sure the changelog is clear about ENABLE_POINTER_EVENTS being removed in a patch today.
Comment 9 WebKit Commit Bot 2020-03-09 19:10:56 PDT
Comment on attachment 393099 [details]
Patch

Clearing flags on attachment: 393099

Committed r258181: <https://trac.webkit.org/changeset/258181>
Comment 10 WebKit Commit Bot 2020-03-09 19:10:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2020-03-09 19:11:17 PDT
<rdar://problem/60255677>
Comment 12 Darin Adler 2020-03-09 19:20:55 PDT
Comment on attachment 393099 [details]
Patch

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

> Source/cmake/WebKitFeatures.cmake:228
> +    WEBKIT_OPTION_DEFINE(ENABLE_WIRELESS_PLAYBACK_TARGET "Togle wireless playback target support" PRIVATE OFF)

"Togle"!
Comment 13 Darin Adler 2020-03-09 19:22:12 PDT
Comment on attachment 393099 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:424
> -#if ENABLE(WIRELESS_TARGET_PLAYBACK)
> +#if ENABLE(WIRELESS_PLAYBACK_TARGET)
>      configuration->_allowsAirPlayForMediaPlayback = self->_allowsAirPlayForMediaPlayback;
>  #endif

Love that you fixed this. Puzzled that there is no test coverage!
Comment 14 Don Olmstead 2020-03-09 19:32:55 PDT
(In reply to Darin Adler from comment #13)
> Comment on attachment 393099 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=393099&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:424
> > -#if ENABLE(WIRELESS_TARGET_PLAYBACK)
> > +#if ENABLE(WIRELESS_PLAYBACK_TARGET)
> >      configuration->_allowsAirPlayForMediaPlayback = self->_allowsAirPlayForMediaPlayback;
> >  #endif
> 
> Love that you fixed this. Puzzled that there is no test coverage!

Yea there were a few I was surprised to find.

Thank the script at https://bugs.webkit.org/show_bug.cgi?id=208726 which found these issues. Hoping to get it integrated tomorrow and also start generating some of these things.