Fix no-show window issue and refactor cocoa webkit#1271
Conversation
| } | ||
|
|
||
| set_up_window(); | ||
| window_init(); |
There was a problem hiding this comment.
In the constructor, I can easily understand that window_init(window) means that we're going to perform window setup/initialization, potentially with an existing window.
What does it mean when window_init() is called again from on_application_did_finish_launching() without a parameter (= nullptr)?
There was a problem hiding this comment.
It means that by the time on_application_did_finish_launching is called (ie. the delegate is ready), m_window = static_cast<id>(window) was already set, so the window parameter is moot (ie. there is no further reference to window in window_init). Passing nullptr is thus rational.
If the the user DID pass a window into the class constructor -> window_init(window), !m_window will be false on the second call to window_init, so m_window is guarded from being reset.
|
Thanks for the latest commits. The flow is clearer to me. diff --git a/core/include/webview/detail/backends/cocoa_webkit.hh b/core/include/webview/detail/backends/cocoa_webkit.hh
index 63181e15..28d46feb 100755
--- a/core/include/webview/detail/backends/cocoa_webkit.hh
+++ b/core/include/webview/detail/backends/cocoa_webkit.hh
@@ -605,6 +605,13 @@ private:
return;
}
+ // Skip application setup if this isn't the first instance of this class
+ // because the launch event is only sent once.
+ if (!get_and_set_is_first_instance()) {
+ window_init_proceed();
+ return;
+ }
+
m_app_delegate = create_app_delegate();
objc_setAssociatedObject(m_app_delegate, "webview", (id)this,
OBJC_ASSOCIATION_ASSIGN);
@@ -615,11 +622,7 @@ private:
// loop has started in order to perform further initialization.
// We need to return from this constructor so this run loop is only
// temporary.
- // Skip the main loop if this isn't the first instance of this class
- // because the launch event is only sent once.
- if (get_and_set_is_first_instance()) {
- objc::msg_send<void>(m_app, "run"_sel);
- }
+ objc::msg_send<void>(m_app, "run"_sel);
}
void window_init_proceed() {
objc::autoreleasepool arp;
|
|
A note for later: The |
|
Thanks, see latest commit for your suggested change. |
SteffenL
left a comment
There was a problem hiding this comment.
This looks good to me now. Thanks for your work!
And thank-you @SteffenL for your work! See #1272 for a tidy up.
Not sure if I agree... |
|
Thanks!
Don't worry, that isn't something we have to decide on now, and certainly isn't something I expect you to take care of. 😄 I can however give you a few concrete examples where we have application-wide things that we likely could skip if we had Windows:
Linux:
|
This PR: