|
| 1 | +# WebView Widget — Development Progress |
| 2 | + |
| 3 | +## Phase 1: C Helper Binary — Completed (2026-04-01) |
| 4 | + |
| 5 | +### What was implemented |
| 6 | +- `src/lib/webview_helper/webview_helper.cc` — ~280 LOC C++ helper binary |
| 7 | +- `src/lib/webview_helper/CMakeLists.txt` — CMake build with FetchContent for webview v0.12.0 |
| 8 | +- `src/lib/webview_helper/build.sh` — convenience build script |
| 9 | +- `src/lib/webview_helper/cJSON.c` + `cJSON.h` — vendored JSON parser (v1.7.18) |
| 10 | +- `tests/webview/HelperBinaryTest.php` — 16 passing tests |
| 11 | + |
| 12 | +### Deviations from plan |
| 13 | +- **File renamed from `.c` to `.cc`**: The webview library is header-only C++ (`webview::core` is an INTERFACE target requiring `cxx_std_11`). The helper source must be compiled as C++ for the webview API symbols to be available. cJSON is wrapped with `extern "C"`. |
| 14 | +- **`_exit(0)` instead of `pthread_join`**: After `webview_run()` returns, the reader thread may be blocked on `fgets(stdin)`. Closing stdin from another thread is undefined behavior on Linux. Using `_exit(0)` is the pragmatic solution — all useful work (writing the "closed" event, destroying the webview) is done before exit. |
| 15 | +- **`static_cast<webview_hint_t>(hint)`**: C++ requires explicit int-to-enum conversion. |
| 16 | + |
| 17 | +### Known issues |
| 18 | +- `bind()` command leaks `strdup(name)` memory (the name string passed to `on_bound_call` is never freed). Minimal impact since bindings are typically created once and live for the process lifetime. |
| 19 | +- No malformed JSON test from PHP side (tested at binary level only). |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +## Phase 2: ProcessWebView.php — Completed (2026-04-01) |
| 24 | + |
| 25 | +### What was implemented |
| 26 | +- `src/ProcessWebView.php` — process manager with full IPC lifecycle |
| 27 | +- Non-blocking stdout with buffer accumulation for partial line handling |
| 28 | +- Platform detection for binary path resolution |
| 29 | +- Shutdown function for orphan prevention |
| 30 | +- Windows `stream_select()` fallback |
| 31 | + |
| 32 | +### Deviations from plan |
| 33 | +- Initial config passed via argv (not stdin first-message) — simpler, avoids "first message is special" protocol complication. HTML content sent via `setHtml()` after construction. |
| 34 | + |
| 35 | +### Known issues |
| 36 | +- Windows `stream_set_blocking(false)` on pipes from `proc_open` is unreliable. The `stream_select()` fallback is in place but untested on actual Windows. |
| 37 | + |
| 38 | +--- |
| 39 | + |
| 40 | +## Phase 3: WebView.php Widget — Completed (2026-04-01) |
| 41 | + |
| 42 | +### What was implemented |
| 43 | +- `src/Widget/WebView.php` — full widget API with 23 passing tests |
| 44 | +- Command dispatch (JS→PHP) with error handling and automatic error return to JS |
| 45 | +- Event dispatch (ready, closed, error, pong) |
| 46 | +- Lifecycle callbacks (onReady, onClose, onError) |
| 47 | +- `tests/webview/WebViewWidgetTest.php` — 23 tests |
| 48 | +- `tests/webview/fixtures/test.html` — deterministic test page |
| 49 | + |
| 50 | +### Design decisions |
| 51 | +- Does NOT extend `AbstractWidget` — confirmed as correct. WebView is a separate native window, not a Tcl widget. |
| 52 | +- `bind()` sends IPC to helper (Approach A from plan) — each binding creates a real `webview_bind()` in the helper, so JS can call functions directly by name. |
| 53 | + |
| 54 | +--- |
| 55 | + |
| 56 | +## Phase 4: Application.php Integration — Completed (2026-04-01) |
| 57 | + |
| 58 | +### What was implemented |
| 59 | +- Modified `src/Application.php`: added `$webviews` array, `addWebView()`, `removeWebView()`, `tick()` method |
| 60 | +- WebView polling in main event loop with auto-cleanup of closed instances |
| 61 | +- Adaptive sleep: 20ms when WebViews active, 100ms when idle |
| 62 | +- `quit()` destroys all WebViews before exiting |
| 63 | +- `tests/webview/EventLoopIntegrationTest.php` — 7 tests |
| 64 | +- **No regression**: existing Tk widget tests (WindowTest, ButtonTest) still pass |
| 65 | + |
| 66 | +### Deviations from plan |
| 67 | +- Added `tick()` method for testability — allows running single event loop iterations from tests without blocking. |
| 68 | + |
| 69 | +--- |
| 70 | + |
| 71 | +## Code Review Fixes — Completed (2026-04-01) |
| 72 | + |
| 73 | +### Security: Event name injection in emit command |
| 74 | +- Event names passed to `window.__phpEmit()` were interpolated directly into JavaScript via `snprintf('%s', ...)`, allowing injection through crafted event names containing quotes. |
| 75 | +- **Fix**: Event names are now JSON-encoded via `cJSON_CreateString` + `cJSON_PrintUnformatted`, producing a properly escaped quoted string passed to `__phpEmit()`. |
| 76 | + |
| 77 | +### Memory leak: strdup(name) in bind command |
| 78 | +- Each `bind` command called `strdup(name)` but the allocated string was never freed on `unbind` or process exit. |
| 79 | +- **Fix**: Added a binding name registry (`binding_names[]` array). `bind` registers the pointer, `unbind` frees it, and `binding_registry_free_all()` cleans up at exit. |
| 80 | + |
| 81 | +### Buffer limitation for large HTML payloads |
| 82 | +- The stdin reader used a 64 KiB line buffer (`LINE_BUF_SIZE = 65536`), which would silently truncate large `set_html` payloads. |
| 83 | +- **Fix**: Increased to 1 MiB (`1048576`). |
| 84 | + |
| 85 | +### Code duplication in Application.php |
| 86 | +- `run()` and `tick()` contained identical Tcl polling, callback dispatch, quit-file checking, and WebView polling logic. |
| 87 | +- **Fix**: `run()` now delegates to `tick()` in a loop, with adaptive sleep after each tick. |
| 88 | + |
| 89 | +### MSVC build errors on Windows CI |
| 90 | +- Designated initializers (`.field = value`) require C++20 on MSVC; we target C++14. Replaced with sequential assignment. |
| 91 | +- `_setmode` / `_O_BINARY` need `<io.h>` and `<fcntl.h>` — added missing includes. |
| 92 | +- MSVC multi-config generators place binaries in `Release/` subdirectory, ignoring `RUNTIME_OUTPUT_DIRECTORY`. Added per-config overrides. |
0 commit comments