[webview_flutter_tizen] Add WV API backend - #1142
Conversation
SupportVideoHoleSet and SetBackgroundColor did not match the APIs they bind (ewk_view_set_support_video_hole, ewk_view_bg_color_set), and SetBackgroundColor additionally contradicted its own typedef and collided with the WebView method of that name. Naming every member after its API keeps a second binding table able to mirror this one.
…backend WebView mixes Flutter method-channel work with direct ewk_*/ecore_*/evas_* calls, which leaves no room for a second web engine. Introduce WebViewBackend as the boundary between the two, with EwkWebViewBackend holding every native call WebView makes today and WebViewBackendFactory owning the choice of implementation. Nothing routes through this yet; WebView still drives EWK directly.
WebView now asks the factory for a backend and talks to it through the interface, keeping the method channel, the texture and the buffer pool while the backend keeps the engine. The ewk_* code it used to carry is gone; it lives in EwkWebViewBackend from the previous commit. No behavior change: the same EWK calls run in the same order.
The WV headers are not part of the plugin's build sysroot, so mirror the public types from chromium-efl/tizen_src/wv/integration/public/ (C ABI) and resolve every function with dlsym(), exactly like EwkInternalApiBinding. The wv_* dispatcher ships in the same library as the ewk_* one. No callers yet.
A second WebViewBackend built on the WV C API. wv_set_arguments() must run before wv_init(), unlike the EWK path which sets arguments per view, so the argv is registered in GlobalInitialize(). Two details differ from the EWK backend and are load-bearing: wv_view_script_message_cb carries no user_data, so JS-channel messages are routed back through a view registry; and Offset() is deliberately a no-op because WV feeds input with a view-local origin already. Not reachable yet; the factory still only builds the EWK backend.
Without --enable-wv-standalone, wv_init() forwards every wv_* call to its ewk_* counterpart and wv_view_create() is a thin shell over ewk_view_add(). Wrapper mode reaches that engine through the same backend, so which mode to ask for is fixed at construction: the argv is shortened to drop the switch, and the view needs offscreen rendering enabled explicitly, which is sound only here because the handle really is an Evas_Object.
…rsion EWK through Tizen 10.0, WV wrapper mode on 10.1, WV standalone from 11.0. The answer is cached so InitializeEngine(), Create() and ShutdownEngine() cannot disagree within a process. WV never falls back to EWK silently: if its API layer does not initialize, WebView creation fails instead of quietly running a different engine.
…ix WV engine init handling * Rename BackendKind::kWvWrapper to kEwkWrapper (label-only, no behavior change) and reorder the enum to kEwk, kEwkWrapper, kWvStandalone. * Propagate wv_init() failure from GlobalInitialize() (now returns bool) instead of silently marking the engine initialized. * Check g_wv_engine_initialized in Create() so a failed engine init can't still construct a WvWebViewBackend. * Drop the wrapper-mode-only ewk_view_offscreen_rendering_enabled_set() workaround and the EWK binding it required — confirmed unnecessary on current chromium-efl images, and clean up the now-unused standalone_ instance state that only existed for it. * Cache EwkInternalApiBinding::Initialize()'s result like WvInternalApiBinding already does, instead of re-resolving every symbol on every call.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d18683194
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
JSUYA
left a comment
JSUYA
left a comment
There was a problem hiding this comment.
Please delete all unnecessary comments written throughout the code.
| if (!WvInternalApiBinding::GetInstance().view.ScriptExecute( | ||
| view_, javascript.c_str(), &WvWebViewBackend::OnEvaluateJavaScript, | ||
| callback_ptr)) { | ||
| LOG_WARN("wv_view_script_execute failed."); | ||
| (*callback_ptr)(nullptr); | ||
| delete callback_ptr; |
There was a problem hiding this comment.
If ScriptExecute() returns null, (*callback_ptr)(nullptr); is called, which finally calls raw_result->Success();. Please verify this along with the EWK logic.
There was a problem hiding this comment.
Thanks for the review.
WV already guards it (checks the return value, force-completes the callback with nullptr on failure). EWK didn't: ewk_view_script_execute also returns Eina_Bool and skips the callback on failure (chromium-efl ewk_view.h), so the old code would leave the Dart Future hanging forever. Fixed EWK to match WV's pattern.
runJavaScriptReturningResult() has no documented failure contract in the platform interface, and Android's own implementation does the exact same thing: evaluateJavascript() returning null becomes '', never an Error. None of the Dart-side callers here catch a PlatformException from this call either, so surfacing Error() would just be an uncaught exception, not a handled failure.
| // Standalone also consumes the headers parameter as an Eina_Hash* | ||
| // (wv_view_private.cc:192), so the EWK header-building code is reused. | ||
| Eina_Hash* wv_headers = eina_hash_new( | ||
| [](const void* key) -> unsigned int { | ||
| return key ? strlen(static_cast<const char*>(key)) + 1 : 0; | ||
| }, | ||
| [](const void* key1, int key1_length, const void* key2, | ||
| int key2_length) -> int { | ||
| return strcmp(static_cast<const char*>(key1), | ||
| static_cast<const char*>(key2)); | ||
| }, | ||
| EINA_KEY_HASH(eina_hash_superfast), [](void* data) { free(data); }, 10); | ||
| for (const auto& header : headers) { | ||
| eina_hash_add(wv_headers, header.first.c_str(), | ||
| strdup(header.second.c_str())); | ||
| } |
There was a problem hiding this comment.
I think you can create common functions by creating a mixin class and using multiple inheritance.
template <typename Backend>
class WebViewBackendMixin {} ...
class WvWebViewBackend : public WebViewBackend,
public WebViewBackendMixin<WvWebViewBackend> {
or maybe, you can make common method within WebViewBackend class.
| "--no-zygote", "--enable-wv-standalone", | ||
| }; | ||
| int argc = sizeof(argv) / sizeof(argv[0]); | ||
| if (!standalone) { |
There was a problem hiding this comment.
standalone is always true in WvWebViewBackend::GlobalInitialize().
Therefore, the bool standalone parameter is not needed.
There was a problem hiding this comment.
WebViewBackendFactory passes kind == BackendKind::kWvStandalone, which is false for kEwkWrapper (Tizen 10.1). The parameter is needed to distinguish the two modes.
SendKey() dropped Shift/Ctrl/Alt modifier state, breaking KeyboardEvent.shiftKey/ctrlKey/altKey in web content. Fix each backend using its own dependency: EwkWebViewBackend replays the raw Ecore modifiers onto the view's Evas canvas since Evas_Event_Key_Down/Up need real Evas_Modifier*/Evas_Lock* handles; WvWebViewBackend, which has no Ecore dependency, converts the raw bitmask straight into wv_modifier_e.
…kend PR - Consolidate the initialized_/initialize_result_ bool pair into a single std::optional<bool> in both EWK and WV internal API bindings. - Round width/height before casting to int in EWK and WV Create()/Resize(), instead of truncating. - Pass proper bool literals to TouchEventsEnabledSet() instead of 1/0. - Drop the unsupported engine_policy handling from WvWebViewBackend::Create() and the now-unnecessary comments/LOG_INFOs around WV engine init. - Bail out of GlobalInitialize() without calling wv_init() if wv_set_arguments() fails, and log failures as errors instead of warnings. - Select the default backend from TIZEN_API_VERSION (getenv), matching flutter_tizen's own apiVersion getter, instead of system_info_get_platform_string(). - Guard ewk_view_script_execute()'s return value the same way the WV backend already does: force-complete the callback on dispatch failure instead of leaving it (and the Dart Future) hanging forever. - Extract the PendingTeardown/FlushPendingTeardowns machinery, which was duplicated verbatim between the EWK and WV backends, into a shared PendingTeardownRegistry<Handle> template in pending_teardown.h.
I've addressed all the review comments above, thanks! |
2 participants
Add a WV API backend behind the existing
WebViewBackendinterface, alongside EWK. Backend is auto-selected by Tizen platform version: EWK through 10.0, EWK wrapper mode on 10.1, WV standalone from 11.0.