Summary: | Disable experimental web features on platforms without UI to enable them | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||||
Component: | WebCore Misc. | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||||
Severity: | Normal | CC: | achristensen, benjamin, bfulgham, cdumez, changseok, cmarcelo, commit-queue, dino, eric.carlson, esprehn+autocc, ews-watchlist, glenn, jer.noble, jonlee, koivisto, kondapallykalyan, pdr, webkit-bug-importer, youennf | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 233304, 233641 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2021-11-15 14:58:15 PST
Created attachment 444311 [details]
Patch v1
(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. (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. How much does this reduce binary size? Created attachment 444327 [details]
Patch v2
(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. (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>. (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 Created attachment 444449 [details]
Patch v3
(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. (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. 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. (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! Created attachment 444770 [details]
Patch v4
Comment on attachment 444770 [details]
Patch v4
Nice! r=me
Tools/Scripts/svn-apply failed to apply attachment 444770 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Created attachment 445348 [details]
Patch v5 for landing
Comment on attachment 445348 [details]
Patch v5 for landing
Adding cq+ since tvOS and watchOS builds succeeded.
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]. Re-opened since this is blocked by bug 233641 Moving this to RESOLVED/WONTFIX since we need to take a different approach. |