Bug 243663

Summary: [GLib] Most public types should be final; most class structs should be private
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: 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    

Description Michael Catanzaro 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?
Comment 1 Michael Catanzaro 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.
Comment 2 Adrian Perez 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).
Comment 3 Adrian Perez 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.
Comment 4 Adrian Perez 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
Comment 5 Adrian Perez 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.
Comment 6 Michael Catanzaro 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.
Comment 7 Adrian Perez 2023-02-09 15:47:20 PST
Pull request: https://github.com/WebKit/WebKit/pull/9895
Comment 8 EWS 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.
Comment 9 Michael Catanzaro 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....