-
Notifications
You must be signed in to change notification settings - Fork 54
[flutter_inappwebview] Fix SIGTRAP on TV app teardown #1100
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| #include <app_common.h> | ||
| #include <flutter/standard_method_codec.h> | ||
| #include <flutter_texture_registrar.h> | ||
| #include <glib.h> | ||
| #include <tbm_surface.h> | ||
|
|
||
| #include <atomic> | ||
|
|
@@ -212,6 +213,35 @@ bool WebView::ClearAllCookies() { | |
| return false; | ||
| } | ||
|
|
||
| // static | ||
| void WebView::InitializeEngine() { ewk_init(); } | ||
|
|
||
| // static | ||
| void WebView::ShutdownEngine() { | ||
| // WebView::Dispose() erases from instances_ synchronously, so by normal | ||
| // plugin teardown order (platform views destroyed before the plugin | ||
| // itself) this is already empty. Wait defensively anyway: ewk_shutdown() | ||
| // fatally CHECKs (SIGTRAP) if any Ewk_View is still alive. | ||
| constexpr gint64 kDeadlineUsec = 2 * G_USEC_PER_SEC; | ||
| const gint64 deadline = g_get_monotonic_time() + kDeadlineUsec; | ||
| for (;;) { | ||
| { | ||
| std::lock_guard<std::mutex> lock(instances_mutex_); | ||
| if (instances_.empty()) { | ||
| break; | ||
| } | ||
| } | ||
| if (g_get_monotonic_time() >= deadline) { | ||
| LOG_WARN( | ||
| "ShutdownEngine: WebView instance(s) still alive past the " | ||
| "deadline; calling ewk_shutdown() anyway."); | ||
| break; | ||
|
Comment on lines
+234
to
+238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a platform view is still registered after this deadline—for example, if plugin teardown begins before its platform-view teardown can run—the loop merely logs and then reaches Useful? React with 👍 / 👎. |
||
| } | ||
| g_usleep(1000); | ||
| } | ||
| ewk_shutdown(); | ||
| } | ||
|
|
||
| std::string WebView::GetDefaultUserAgent() { | ||
| std::lock_guard<std::mutex> lock(instances_mutex_); | ||
| for (auto* instance : instances_) { | ||
|
|
@@ -369,8 +399,6 @@ void WebView::Dispose() { | |
| } | ||
|
|
||
| ecore_evas_ = nullptr; | ||
|
|
||
| // ewk_shutdown(); | ||
| } | ||
|
|
||
| void WebView::Offset(double left, double top) { | ||
|
|
@@ -530,15 +558,9 @@ bool WebView::InitWebView() { | |
| chromium_argv); | ||
| }); | ||
|
|
||
| // TODO(jsuya): ewk_init() and ewk_shutdown() are designed to be called only | ||
| // once in a process.(If ewk_init() is called after ewk_shutdown() is | ||
| // called, SIGTRAP is called internally.) ewk_init() initializes the efl | ||
| // modules and web engine's arguments data. The efl modules are initialized | ||
| // by default in OS, and arguments data is also initialized through | ||
| // SetArguments() API, so calling ewk_init() is not necessary. Therefore, | ||
| // temporarily comment out ewk_init() and ewk_shutdown(). It can be reverted | ||
| // depending on updates to chromium-efl. | ||
| // ewk_init(); | ||
| // ewk_init()/ewk_shutdown() are called once per process by | ||
| // WebView::InitializeEngine()/ShutdownEngine(), driven by the plugin's | ||
| // constructor/destructor. | ||
| static Ecore_Evas* shared_ecore_evas = nullptr; | ||
| if (!shared_ecore_evas) { | ||
| shared_ecore_evas = ecore_evas_new("wayland_egl", 0, 0, 1, 1, 0); | ||
|
|
||
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.
When a Flutter engine is destroyed and recreated in the same process, each new plugin instance calls
ewk_init()and the previous instance has already calledewk_shutdown(). This is a per-plugin lifecycle rather than the promised process-wide once-only lifecycle, and the existing EWK constraint described by this change says reinitialization after shutdown triggers SIGTRAP; use process-wide ownership/once semantics instead of pairing the calls with every plugin instance.Useful? React with 👍 / 👎.