[webkit-dev] WEB_TIMING enabled on all ports - let's remove the flag?

[Maciej Stachowiak]

> On Aug 1, 2017, at 6:55 PM, Adrian Perez de Castro <aperez at igalia.com> wrote:
> 
> Hello,
> 
> On Tue, 01 Aug 2017 18:11:53 -0400, Maciej Stachowiak <mjs at apple.com <mailto:mjs at apple.com>> wrote:
> 
>>> On Aug 1, 2017, at 5:55 PM, Konstantin Tokarev <annulen at yandex.ru> wrote:
>>> 
>>> 02.08.2017, 00:49, "Sam Weinig" <weinig at apple.com>:
>>>>> On Aug 1, 2017, at 6:57 AM, Dean Jackson <dino at apple.com> wrote:
>>>>> 
>>>>>> On 24 Jul 2017, at 22:44, Brian Burg <bburg at apple.com> wrote:
>>>>>> 
>>>>>> Hi WebKittens,
>>>>>> 
>>>>>> In WebKit, the various web-exposed timing APIs–Resource Timing, User Timing, and Navigation Timing are guarded by the ENABLE_WEB_TIMING feature flag.
>>>>>> 
>>>>>> It seems that ENABLE_WEB_TIMING is turned on for Xcode and CMake build systems by default, and we have not experienced any fallout from shipping these features in Safari Technology Preview. I think it’s time to remove the feature flag. Are there any objections? Is there a port in-tree that’s compiling without this feature?
>>>>>> 
>>>>>> If I don’t hear anything, the flag’s removal will be tracked in <https://bugs.webkit.org/show_bug.cgi?id=174795>.
>>>>> 
>>>>> In general I think we should be more enthusiastic about removing feature flags that are guarding core parts of the Web platform. Web Timing is a great example.
> 
> In general I agree that build-time feature flags should go away once all ports
> can have the feature enabled by default.
> 
>>>>> Some others I see:
>>>>> 
>>>>> ENABLE_CANVAS_PATH
>>>>> ENABLE_CSS_COMPOSITING
>>>>> ENABLE_CSS_SELECTORS_LEVEL4
>>>>> ENABLE_FETCH_API
>>>>> ENABLE_GEOLOCATION
>>>>> ENABLE_INDEXED_DATABASE
>>>>> ENABLE_STREAMS_API
>>>>> ENABLE_CSS_SCROLL_SNAP
>>>>> ENABLE_WEBGL
>>>>> ENABLE_WEB_AUDIO
>>>>> ENABLE_WEB_SOCKETS
>>>> 
>>>> I think WebGL is still not supported on Windows in WebKitLegacy, so we may need to keep that one.
>>>> 
>>>> I’d like to add ENABLE_VIDEO to that list, given how core it is to the platform, and how many other features depend on it.
>>> 
>>> I think it's a bad idea to remove ENABLE_VIDEO. Not only because there are lots of non-browser applications that need HTML renderer (document viewers, mail clients, instant messengers, RSS readers, etc.) which don't need video, but also because it greatly raises the bar for implementing new ports (e.g. recently announced Android port didn't implement video at that time)
>> 
>> I think all of the clients you mentioned actually do need video (or at least benefit from it). In any case, most clients like that don't usually bundle their own WebKit. They use a shared copy. Usually a copy that is also used by a web browser. So if they really want to disable video, they need a runtime flag, not a compile-time flag.
> 
> Many embedded applications (embedded == not a browser, and the device does
> not have a general-purpose one) ship their own build of WebKit, almost always
> tailored to fit.
> 
> A good example of an use-case that benefits from supporting ENABLE_VIDEO=OFF
> are signage panels (typically some kind of info screens in a public space),
> which most of the time show imagery and textual content, but no video or audio
> at all, running on small embedded computers where on-disk size and memory
> usage matter. For this kind of application it also makes sense to support
> ENABLE_WEB_AUDIO=OFF, as the hardware usually does not even have speakers
> nor any other kind of sound output.
> 
> Moreover, for the WebKitGTK+ disabling both ENABLE_VIDEO and ENABLE_WEB_AUDIO
> does not need GStreamer at all, which further reduces disk and memory usage.
> For example Buildroot includes a WebKitGTK+ recipe which can disable both [1]
> precisely for this reason. So I would also keep ENABLE_WEB_AUDIO.

That sound somewhat more compelling to me than apps for desktop platforms that bundle their own WebKit.

Regards,
Maciej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20170801/12906ae1/attachment.html>