Bug 243663
Summary: | [GLib] Most public types should be final; most class structs should be private | ||
---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
Component: | WebKitGTK | Assignee: | Adrian Perez <aperez> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | aperez, bugs-noreply, mcatanzaro |
Priority: | P2 | ||
Version: | WebKit Nightly Build | ||
Hardware: | PC | ||
OS: | Linux | ||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=251075 https://bugs.webkit.org/show_bug.cgi?id=252162 https://bugs.webkit.org/show_bug.cgi?id=252383 |
||
Bug Depends on: | 251008 | ||
Bug Blocks: | 210100 |
Michael Catanzaro
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
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
(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
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
(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
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
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
Pull request: https://github.com/WebKit/WebKit/pull/9895
EWS
Committed 260190@main (234799572ddb): <https://commits.webkit.org/260190@main>
Reviewed commits have been landed. Closing PR #9895 and removing active labels.
Michael Catanzaro
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....