-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make WebView use C++ API instead of C API #48
Make WebView use C++ API instead of C API #48
Conversation
Also removes what little existed of Haiku's C implementation of WebView's API. Haiku's API itself doesn't support C, so it is doubtful that we want a C API for WebKit.
Haiku's API usually includes relevant information along with the messages it sends, so we may as well too.
@@ -39,6 +39,7 @@ class DrawingAreaProxy; | |||
class WebViewBase; | |||
|
|||
class PageClientImpl: public PageClient { | |||
WTF_MAKE_FAST_ALLOCATED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I remember correctly (it's been a while since I last looked into this), the classes in UIProcess/API are meant to be used directly by the "UI process", that is, they should be usable outside of WebKit and behave like "normal" classes.
WTF_MAKE_FAST_ALLOCATED may prevent creating them with the normal "new" operator from outside the WebKit tree (I'm not sure if that's really the case, I only vaguely remember hitting this problem, sorry).
It may be OK if the class is created on WebKit side and then passed on to the browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... I can't really figure out what exactly the problem was. It seems that using WTF_MAKE_FAST_ALLOCATED even works when directly used on MiniBrowser/haiku/BrowserWindow.h! Looking at the GTK port's API doesn't provide much of a clue. They use it in some places but not others, and I can't think of the rationale behind it.
All of the classes I annotated shouldn't be used outside of WebKit, so at least we have that protection.
|
||
void NavigationClient::didCommitNavigation(WebPageProxy& page, API::Navigation* navigation, API::Object* userData) | ||
{ | ||
BMessage message(DID_COMMIT_NAVIGATION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If all this is going to do is send BMessage to the page view, it doesn't need to be in the API directory. This directory should be reserved for the public API, that is, classes that the browser needs to directly interact with.
At least this is how it is handled by WebKitLegacy, but I think the idea is the same here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk, the other ports seem to do similar things. Also, APINavigationClient.h is in the API folder.
I suppose the idea is to put things in the API folder if they form part of the API the client needs to know to use WebKit. Obviously, things used directly by the browser fall under this. But I would argue that the exact format and name of DID_COMMIT_NAVIGATION also counts as part of the API.
672f2fc
to
12572f6
Compare
Also removes what little existed of Haiku's C implementation of WebView's API. Haiku's API itself doesn't support C, so it is doubtful that we want a C API for WebKit.