Bug 233152 – Disable experimental web features on platforms without UI to enable them

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[David Kilzer (:ddkilzer)]

Disable experimental web features on platforms without UI to enable them.

Both tvOS and watchOS include WebKit, but they don't have UI to enable or disable experimental web features, so this just ends up being dead code on customer hardware/

The initial version of this patch disables these experimental features (chosen from macros used to enable features in Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml):

ENABLE_CORE_IMAGE_ACCELERATED_FILTER_RENDER
ENABLE_CSS_PAINTING_API
ENABLE_CSS_TYPED_OM
ENABLE_MODEL_ELEMENT
ENABLE_SEPARATED_MODEL
HAVE_CELESTIAL
HAVE_CFNETWORK_ALTERNATIVE_SERVICE
HAVE_AVSAMPLEBUFFERVIDEOOUTPUT

[Radar WebKit Bug Importer]

<rdar://problem/85430643>

[David Kilzer (:ddkilzer)]

Created attachment 444311 [details]
Patch v1

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #2)
> Created attachment 444311 [details]
> Patch v1

Curious...this patch built a couple days ago on watchOS just fine.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #3)
> (In reply to David Kilzer (:ddkilzer) from comment #2)
> > Created attachment 444311 [details]
> > Patch v1
> 
> Curious...this patch built a couple days ago on watchOS just fine.

I'm about 80% certain that the issue is that Source/WebCore/DerivedSources-input.xcfilelist doesn't include dependences for Platform.h or the headers it includes (as part of Source/WebCore/DerivedSources.make), so the IDLs are not rebuilt on incremental builds when only a header file changes.

I'm trying to figure out how to make Tools/Scripts/generate-xcfilelists include Platform.h header dependencies, but it might be a while.  Lots of new-to-me Python code that that wasn't easy to read at first glance.

[Alex Christensen]

How much does this reduce binary size?

[David Kilzer (:ddkilzer)]

Created attachment 444327 [details]
Patch v2

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #6)
> Created attachment 444327 [details]
> Patch v2

Manually update Source/WebCore/DerivedSources-input.xcfilelist to include header files from Source/WTF as input dependencies to see if that fixes the tv/watch builds.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #7)
> (In reply to David Kilzer (:ddkilzer) from comment #6)
> > Created attachment 444327 [details]
> > Patch v2
> 
> Manually update Source/WebCore/DerivedSources-input.xcfilelist to include
> header files from Source/WTF as input dependencies to see if that fixes the
> tv/watch builds.

Need to check what path is being used for WTF headers when pre-processing <wtf/Platform.h>.

[David Kilzer (:ddkilzer)]

(In reply to Alex Christensen from comment #5)
> How much does this reduce binary size?

Savings in WebCore.framework/WebCore alone are about 1.2% to 1.3% of the total size:

- armv7k:  (59215528 - 58461164) / 1024 = 737 Kb
- arm64_32:  (69203584 - 68297120) / 1024 = 885 Kb
- arm64e:  (72847152 - 71949768) / 1024 = 876 Kb

[David Kilzer (:ddkilzer)]

Created attachment 444449 [details]
Patch v3

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #10)
> Created attachment 444449 [details]
> Patch v3

Add debugging output for Source/WebCore/DerivedSources.make to see which Platform.h headers are used.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #11)
> (In reply to David Kilzer (:ddkilzer) from comment #10)
> > Created attachment 444449 [details]
> > Patch v3
> 
> Add debugging output for Source/WebCore/DerivedSources.make to see which
> Platform.h headers are used.

And I probably worked around the missing dependency by modifying DerivedSources.make.

[David Kilzer (:ddkilzer)]

Just realized this morning that both the targets in DerivedSources.make and DerivedSources-input.xcfilelist need dependencies on the Platform.h headers for this to work correctly.

Also, I’m curious why Keith Rollin didn’t just add a target to DerivedSources.make to regenerate DerivedSources-input.xcfilelist (instead of writing Python scripts) as the Makefile has all the lists of files needed to do so.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #13)
> Also, I’m curious why Keith Rollin didn’t just add a target to
> DerivedSources.make to regenerate DerivedSources-input.xcfilelist (instead
> of writing Python scripts) as the Makefile has all the lists of files needed
> to do so.

Answer: The Python script is doing something like this, as fixing a FIXME about missing header dependencies updates DerivedSources-input.xcfilelist without any Python code changes.  Magical!

[David Kilzer (:ddkilzer)]

Created attachment 444770 [details]
Patch v4

[Brent Fulgham]

Comment on attachment 444770 [details]
Patch v4

Nice! r=me

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 444770 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[David Kilzer (:ddkilzer)]

Created attachment 445348 [details]
Patch v5 for landing

[David Kilzer (:ddkilzer)]

Comment on attachment 445348 [details]
Patch v5 for landing

Adding cq+ since tvOS and watchOS builds succeeded.

[EWS]

Committed r286272 (244633@main): <https://commits.webkit.org/244633@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445348 [details].

[WebKit Commit Bot]

Re-opened since this is blocked by bug 233641

[David Kilzer (:ddkilzer)]

Moving this to RESOLVED/WONTFIX since we need to take a different approach.