Skip to content

Commit cd16624

Browse files
committed
Fix native crash and dialog-type assertion in WebView dialog tests
The instrumentation suite crashed the app process with SIGABRT during the WebViewDialogOverrideTest teardown (a deferred g_object_unref on the WebKit main loop while ActivityScenario destroyed the activity). The trigger is the new focus wiring: onFocusChanged now calls wpe_view_focus_in, which keeps the page active and scheduling engine work, while WebViewTestActivity never released the WebView, so the leaked active page raced the activity teardown. - WebViewTestActivity: release the WebView in onDestroy() so the focused/active page is torn down deterministically per ActivityScenario recreation. - WebView.destroy(): drop the platformView reference once the native WPEView has been invalidated, so a late onFocusChanged during teardown cannot touch it. - nativeScriptDialogConfirm: only call webkit_script_dialog_confirm_set_confirmed for CONFIRM/BEFORE_UNLOAD dialogs (and set prompt text for PROMPT). Calling it for an alert/prompt tripped a g_return_if_fail CRITICAL in WebKit.
1 parent 0f595bc commit cd16624

3 files changed

Lines changed: 31 additions & 3 deletions

File tree

wpeview/src/androidTest/java/org/wpewebkit/wpeview/WebViewTestActivity.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,17 @@ public void onCreate(Bundle savedInstanceState) {
2525
setContentView(linearLayout);
2626
}
2727

28+
@Override
29+
public void onDestroy() {
30+
// ActivityScenario recreates this activity for every test. Release the WebView so the
31+
// (now focused, and therefore active) page is torn down deterministically instead of
32+
// leaking and racing the activity teardown on the WebKit main loop.
33+
if (webView != null) {
34+
webView.destroy();
35+
webView = null;
36+
}
37+
super.onDestroy();
38+
}
39+
2840
public WebView getWebView() { return webView; }
2941
}

wpeview/src/main/cpp/capi/WebKitWebView.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,22 @@ void JNIWebKitWebViewCache::nativeEvaluateJavascript(
397397
void JNIWebKitWebViewCache::nativeScriptDialogConfirm(JNIEnv*, jobject, jlong dialogPtr, jboolean confirm, jstring text)
398398
{
399399
auto* dialog = reinterpret_cast<WebKitScriptDialog*>(dialogPtr); // NOLINT(performance-no-int-to-ptr)
400-
if (webkit_script_dialog_get_dialog_type(dialog) == WEBKIT_SCRIPT_DIALOG_PROMPT && text != nullptr)
401-
webkit_script_dialog_prompt_set_text(dialog, JNI::String(text).getContent().get());
402-
webkit_script_dialog_confirm_set_confirmed(dialog, static_cast<gboolean>(confirm));
400+
// Each dialog type accepts only its own result setter; calling the wrong one trips a
401+
// g_return_if_fail in WebKit. Alerts carry no result and are answered purely by closing.
402+
switch (webkit_script_dialog_get_dialog_type(dialog)) {
403+
case WEBKIT_SCRIPT_DIALOG_CONFIRM:
404+
case WEBKIT_SCRIPT_DIALOG_BEFORE_UNLOAD_CONFIRM:
405+
webkit_script_dialog_confirm_set_confirmed(dialog, static_cast<gboolean>(confirm));
406+
break;
407+
case WEBKIT_SCRIPT_DIALOG_PROMPT:
408+
// A confirmed prompt returns the entered text; a cancelled prompt leaves the text
409+
// unset so JavaScript prompt() yields null.
410+
if (confirm && text != nullptr)
411+
webkit_script_dialog_prompt_set_text(dialog, JNI::String(text).getContent().get());
412+
break;
413+
case WEBKIT_SCRIPT_DIALOG_ALERT:
414+
break;
415+
}
403416
}
404417

405418
void JNIWebKitWebViewCache::nativeScriptDialogClose(JNIEnv*, jobject, jlong dialogPtr)

wpeview/src/main/java/org/wpewebkit/wpeview/WebView.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ public void destroy() {
218218
webKitWebView.destroy();
219219
webKitWebView = null;
220220
}
221+
// webKitWebView.destroy() invalidates the underlying native WPEView, so drop our reference
222+
// too: any later widget callback (e.g. onFocusChanged during teardown) must not touch it.
223+
platformView = null;
221224
if (ownsContext && wpeContext != null) {
222225
wpeContext.destroy();
223226
wpeContext = null;

0 commit comments

Comments
 (0)