WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
243663
[GLib] Most public types should be final; most class structs should be private
https://bugs.webkit.org/show_bug.cgi?id=243663
Summary
[GLib] Most public types should be final; most class structs should be private
Michael Catanzaro
Reported
2022-08-08 05:36:59 PDT
In the new API, most public API types should be final (non-derivable) and their class structs should be private (move from header file to source file). Only classes that we know need to be subclassed should be derivable. That includes WebKitWebView for sure. Maybe nothing else?
Attachments
Add attachment
proposed patch, testcase, etc.
Michael Catanzaro
Comment 1
2022-12-05 06:53:54 PST
More specifically, we can use G_DECLARE_FINAL_TYPE for most types, and G_DECLARE_DERIVABLE_TYPE for the few that need to be derivable. This will make the header files more complicated for now, because it's an ABI change, so we have to keep all the boilerplate for the older API versions. But when we eventually remove the older API versions, then we can remove the boilerplate, and the headers will be cleaner. So it's worth it.
Adrian Perez
Comment 2
2023-01-23 05:44:14 PST
(In reply to Michael Catanzaro from
comment #0
)
> In the new API, most public API types should be final (non-derivable) and > their class structs should be private (move from header file to source > file). Only classes that we know need to be subclassed should be derivable. > That includes WebKitWebView for sure. Maybe nothing else?
At least we also want to leave WebKitInputMethodContext as derivable:
https://webkitgtk.org/reference/webkit2gtk/unstable/class.InputMethodContext.html
(Unless we think the API needs to be different going forward, that is).
Adrian Perez
Comment 3
2023-01-23 05:54:35 PST
I'm doubtful about whether to make WebKitPrintOperation final or not... In one hand, we do expect users of the API to create an instance, which is a hint that they might want to subclass the type to extend it; on the other hand I cannot imagine why one would want to extend this particular class in any way 🤔️ -- for now I am going to mark it as final, let me know if there's some reason to leave it as non-final.
Adrian Perez
Comment 4
2023-01-23 06:09:09 PST
(In reply to Adrian Perez from
comment #3
)
> I'm doubtful about whether to make WebKitPrintOperation final or not... > In one hand, we do expect users of the API to create an instance, which > is a hint that they might want to subclass the type to extend it; on the > other hand I cannot imagine why one would want to extend this particular > class in any way 🤔️ -- for now I am going to mark it as final, let me > know if there's some reason to leave it as non-final.
The same reasoning goes for the following: - WebKitSettings - WebKitURIRequest - WebKitUserContentFilterStore - WebKitUserContentManager - WebKitUserMessage - WebKitWebsiteDataManager - WebKitWebsitePolicies I am not so sure about the following, maybe we want them derivable... but I myself lean towards making them final, too: - WebKitWebContext
Adrian Perez
Comment 5
2023-01-23 06:12:01 PST
FTR, I will be splitting the work in two patches: one to mark types as final, then a follow-up to move their private structs.
Michael Catanzaro
Comment 6
2023-01-23 07:51:43 PST
Sounds like a good plan. I agree we should lean towards making classes final when we're not certain. Final classes are simpler. We can make final classes derivable later on if desired without breaking API/ABI. And composition is generally better than inheritance anyway, so discouraging inheritance is good.
Adrian Perez
Comment 7
2023-02-09 15:47:20 PST
Pull request:
https://github.com/WebKit/WebKit/pull/9895
EWS
Comment 8
2023-02-13 03:47:50 PST
Committed
260190@main
(234799572ddb): <
https://commits.webkit.org/260190@main
> Reviewed commits have been landed. Closing PR #9895 and removing active labels.
Michael Catanzaro
Comment 9
2023-02-14 14:14:41 PST
Hey Adrian, can you please remind me what the follow-up work here is? I remember you had something left to do after this landed, but don't remember what it was....
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug