[GLib] Make most public types final #8972

aperezdc · 2023-01-23T14:19:01Z

`f1f3d52`

[GLib] Make most public types final
https://bugs.webkit.org/show_bug.cgi?id=251008

Reviewed by Carlos Garcia Campos.

Add a new WEBKIT_DEFINE_TYPE_WITH_CODE macro, plus two helper macros
with an _IN_2022_API suffix to be used all around the code. These two
macros mark types as final only when ENABLE(2022_GLIB_API) is enabled,
otherwise they behave as the existing ones (leaving the types as
derivable). This will allow in the future to easily search for types
which change between final/non-final, and remove the use of _IN_2022_API
macros when the old API is no longer needed. Also, make sure to require
GLib 2.70 for the new API.

* Source/WTF/wtf/glib/WTFGType.h: Add new macros and switch between
  final/non-final depending on the value of ENABLE(2022_GLIB_API).
* Source/WebKit/Shared/API/glib/WebKitContextMenu.cpp:
* Source/WebKit/Shared/API/glib/WebKitContextMenuItem.cpp:
* Source/WebKit/Shared/API/glib/WebKitURIRequest.cpp:
* Source/WebKit/Shared/API/glib/WebKitURIResponse.cpp:
* Source/WebKit/Shared/API/glib/WebKitUserMessage.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitAuthenticationRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitAutomationSession.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitBackForwardList.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitBackForwardListItem.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitDeviceInfoPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitDownload.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitEditorState.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitFileChooserRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitFindController.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitFormSubmissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitGeolocationPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitMediaKeySystemPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitNavigationPolicyDecision.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitNotification.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitNotificationPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitOptionMenu.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitPointerLockPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitResponsePolicyDecision.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitSecurityManager.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitUserContentFilterStore.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitUserMediaPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataAccessPermissionRequest.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitWindowProperties.cpp:
* Source/WebKit/UIProcess/API/gtk/WebKitColorChooserRequest.cpp:
* Source/WebKit/UIProcess/API/gtk/WebKitPrintOperation.cpp:
* Source/WebKit/UIProcess/API/gtk/WebKitWebInspector.cpp:
* Source/cmake/OptionsGTK.cmake: Require GLib 2.70.0 when
  ENABLE(2022_GLIB_API) is set.
* Source/cmake/OptionsWPE.cmake: Ditto.

Canonical link: https://commits.webkit.org/259270@main

275acb0

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	~~🧪 gtk-wk2~~
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
~~🛠 🧪 jsc~~	✅ 🛠 tv	✅ 🧪 mac-wk2	✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv-sim	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress	✅ 🛠 jsc-mips
✅ 🛠 🧪 merge	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-01-23T14:19:19Z

EWS run on previous version of this PR (hash 534ae73)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 🧪 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc	✅ 🛠 tv	✅ 🧪 mac-wk2	✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress	✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

carlosgcampos · 2023-01-23T14:30:46Z

We need to bump the minimum glib requirements to 2.70 (only for the new api).

aperezdc · 2023-01-23T17:09:56Z

We need to bump the minimum glib requirements to 2.70 (only for the new api).

Good point, I will update the PR. Also no idea what's going on with the timeout in the WPE EWS builder, I'll see if that happens locally or not.

mcatanzaro · 2023-01-23T23:01:40Z

Source/WTF/wtf/glib/WTFGType.h

+#define WEBKIT_DEFINE_FINAL_TYPE_IN_2022_API WEBKIT_DEFINE_TYPE
+#define WEBKIT_DEFINE_FINAL_TYPE_WITH_CODE_IN_2022_API WEBKIT_DEFINE_TYPE_WITH_CODE


This is fine, but I'm not sure it's really much benefit? I would just provide two different definitions of WEBKIT_DEFINE_FINAL_TYPE, where it uses G_TYPE_FLAG_FINAL if it's the new API and just doesn't for the old API. That will avoid adding the _IN_2022_API cruft to all the source files.

Would that conflict with what you've got planned next?

I thought of that first, but then it seemed to me quite obscure that a WEBKIT_DEFINE_FINAL_TYPE macro won't create a final type when ENABLE(2022_GLIB_API) is off—it feels very wrong and will be confusing. So I did the extra indirection with the _IN_2022_API suffix for clarity.

webkit-early-warning-system · 2023-01-23T23:32:32Z

EWS run on previous version of this PR (hash 0662c46)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	~~🛠 wpe~~	⏳ 🛠 🧪 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	⏳ 🧪 api-mac	⏳ 🧪 gtk-wk2
	~~🧪 api-ios~~	⏳ 🧪 mac-wk1	⏳ 🧪 api-gtk
~~🛠 🧪 jsc~~	✅ 🛠 tv	⏳ 🧪 mac-wk2	✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64	⏳ 🛠 tv-sim	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch	⏳ 🧪 mac-wk2-stress	✅ 🛠 jsc-mips
	⏳ 🛠 watch-sim		~~🧪 jsc-mips-tests~~

aperezdc · 2023-01-23T23:49:31Z

We need to bump the minimum glib requirements to 2.70 (only for the new api).

Good point, I will update the PR. Also no idea what's going on with the timeout in the WPE EWS builder, I'll see if that happens locally or not.

The lockup was caused by marking WebKitHitTestResult as final: it turns out that this type has a WebKitWebHitTestResult (in Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitWebHitTestResult.cpp) and the introspection scanner sometimes goes bananas when it sees a subclass of a final type 🙃—PR updated accordingly.

webkit-early-warning-system · 2023-01-23T23:54:30Z

EWS run on previous version of this PR (hash 35ace71)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	❌ 🧪 api-gtk
✅ 🛠 🧪 jsc	✅ 🛠 tv	✅ 🧪 mac-wk2	✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress	✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

mcatanzaro · 2023-01-24T00:49:15Z

Would probably be nicer for WebKitWebHitTestResult to use composition rather than inheritance, but I certainly don't expect you to change that here....

aperezdc · 2023-01-24T08:55:58Z

Would probably be nicer for WebKitWebHitTestResult to use composition rather than inheritance, but I certainly don't expect you to change that here....

Let's do it as a separate issue, I have filed #251075 for that.

webkit-early-warning-system · 2023-01-24T09:12:11Z

EWS run on current version of this PR (hash 275acb0)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	~~🧪 gtk-wk2~~
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
~~🛠 🧪 jsc~~	✅ 🛠 tv	✅ 🧪 mac-wk2	✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv-sim	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress	✅ 🛠 jsc-mips
✅ 🛠 🧪 merge	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

https://bugs.webkit.org/show_bug.cgi?id=251008 Reviewed by Carlos Garcia Campos. Add a new WEBKIT_DEFINE_TYPE_WITH_CODE macro, plus two helper macros with an _IN_2022_API suffix to be used all around the code. These two macros mark types as final only when ENABLE(2022_GLIB_API) is enabled, otherwise they behave as the existing ones (leaving the types as derivable). This will allow in the future to easily search for types which change between final/non-final, and remove the use of _IN_2022_API macros when the old API is no longer needed. Also, make sure to require GLib 2.70 for the new API. * Source/WTF/wtf/glib/WTFGType.h: Add new macros and switch between final/non-final depending on the value of ENABLE(2022_GLIB_API). * Source/WebKit/Shared/API/glib/WebKitContextMenu.cpp: * Source/WebKit/Shared/API/glib/WebKitContextMenuItem.cpp: * Source/WebKit/Shared/API/glib/WebKitURIRequest.cpp: * Source/WebKit/Shared/API/glib/WebKitURIResponse.cpp: * Source/WebKit/Shared/API/glib/WebKitUserMessage.cpp: * Source/WebKit/UIProcess/API/glib/WebKitAuthenticationRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitAutomationSession.cpp: * Source/WebKit/UIProcess/API/glib/WebKitBackForwardList.cpp: * Source/WebKit/UIProcess/API/glib/WebKitBackForwardListItem.cpp: * Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp: * Source/WebKit/UIProcess/API/glib/WebKitDeviceInfoPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitDownload.cpp: * Source/WebKit/UIProcess/API/glib/WebKitEditorState.cpp: * Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp: * Source/WebKit/UIProcess/API/glib/WebKitFileChooserRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitFindController.cpp: * Source/WebKit/UIProcess/API/glib/WebKitFormSubmissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp: * Source/WebKit/UIProcess/API/glib/WebKitGeolocationPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitMediaKeySystemPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitNavigationPolicyDecision.cpp: * Source/WebKit/UIProcess/API/glib/WebKitNotification.cpp: * Source/WebKit/UIProcess/API/glib/WebKitNotificationPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitOptionMenu.cpp: * Source/WebKit/UIProcess/API/glib/WebKitPointerLockPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitResponsePolicyDecision.cpp: * Source/WebKit/UIProcess/API/glib/WebKitSecurityManager.cpp: * Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp: * Source/WebKit/UIProcess/API/glib/WebKitUserContentFilterStore.cpp: * Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp: * Source/WebKit/UIProcess/API/glib/WebKitUserMediaPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp: * Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp: * Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataAccessPermissionRequest.cpp: * Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp: * Source/WebKit/UIProcess/API/glib/WebKitWindowProperties.cpp: * Source/WebKit/UIProcess/API/gtk/WebKitColorChooserRequest.cpp: * Source/WebKit/UIProcess/API/gtk/WebKitPrintOperation.cpp: * Source/WebKit/UIProcess/API/gtk/WebKitWebInspector.cpp: * Source/cmake/OptionsGTK.cmake: Require GLib 2.70.0 when ENABLE(2022_GLIB_API) is set. * Source/cmake/OptionsWPE.cmake: Ditto. Canonical link: https://commits.webkit.org/259270@main

webkit-commit-queue · 2023-01-24T11:13:49Z

Committed 259270@main (f1f3d52): https://commits.webkit.org/259270@main

Reviewed commits have been landed. Closing PR #8972 and removing active labels.

aperezdc requested a review from a team as a code owner January 23, 2023 14:19

aperezdc self-assigned this Jan 23, 2023

aperezdc added the WebKit API For issues and bugs in the Web Kit public embedding APIs label Jan 23, 2023

aperezdc requested review from mcatanzaro and carlosgcampos January 23, 2023 14:20

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 23, 2023

mcatanzaro reviewed Jan 23, 2023

View reviewed changes

aperezdc removed the merging-blocked Applied to prevent a change from being merged label Jan 23, 2023

aperezdc force-pushed the glib-final-types branch from 534ae73 to 0662c46 Compare January 23, 2023 23:28

aperezdc requested a review from mcatanzaro January 23, 2023 23:29

aperezdc force-pushed the glib-final-types branch from 0662c46 to 35ace71 Compare January 23, 2023 23:49

aperezdc force-pushed the glib-final-types branch from 35ace71 to 275acb0 Compare January 24, 2023 09:06

carlosgcampos approved these changes Jan 24, 2023

View reviewed changes

aperezdc added the merge-queue Applied to send a pull request to merge-queue label Jan 24, 2023

webkit-early-warning-system force-pushed the glib-final-types branch from 275acb0 to f1f3d52 Compare January 24, 2023 11:13

webkit-early-warning-system merged commit f1f3d52 into WebKit:main Jan 24, 2023

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 24, 2023

aperezdc deleted the glib-final-types branch January 24, 2023 12:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GLib] Make most public types final #8972

[GLib] Make most public types final #8972

aperezdc commented Jan 23, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Jan 23, 2023 •

edited

carlosgcampos commented Jan 23, 2023

aperezdc commented Jan 23, 2023

mcatanzaro Jan 23, 2023

aperezdc Jan 23, 2023

webkit-early-warning-system commented Jan 23, 2023 •

edited

aperezdc commented Jan 23, 2023

webkit-early-warning-system commented Jan 23, 2023 •

edited

mcatanzaro commented Jan 24, 2023

aperezdc commented Jan 24, 2023

webkit-early-warning-system commented Jan 24, 2023 •

edited

webkit-commit-queue commented Jan 24, 2023

		#define WEBKIT_DEFINE_FINAL_TYPE_IN_2022_API WEBKIT_DEFINE_TYPE
		#define WEBKIT_DEFINE_FINAL_TYPE_WITH_CODE_IN_2022_API WEBKIT_DEFINE_TYPE_WITH_CODE

[GLib] Make most public types final #8972

[GLib] Make most public types final #8972

Conversation

aperezdc commented Jan 23, 2023 • edited by webkit-early-warning-system

f1f3d52

webkit-early-warning-system commented Jan 23, 2023 • edited

carlosgcampos commented Jan 23, 2023

aperezdc commented Jan 23, 2023

mcatanzaro Jan 23, 2023

Choose a reason for hiding this comment

aperezdc Jan 23, 2023

Choose a reason for hiding this comment

webkit-early-warning-system commented Jan 23, 2023 • edited

aperezdc commented Jan 23, 2023

webkit-early-warning-system commented Jan 23, 2023 • edited

mcatanzaro commented Jan 24, 2023

aperezdc commented Jan 24, 2023

webkit-early-warning-system commented Jan 24, 2023 • edited

webkit-commit-queue commented Jan 24, 2023

aperezdc commented Jan 23, 2023 •

edited by webkit-early-warning-system

`f1f3d52`

webkit-early-warning-system commented Jan 23, 2023 •

edited

webkit-early-warning-system commented Jan 23, 2023 •

edited

webkit-early-warning-system commented Jan 23, 2023 •

edited

webkit-early-warning-system commented Jan 24, 2023 •

edited