diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..b99c7f3 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,36 @@ +# Native lifecycle tests + +Run on macOS with a built Defold SDK (including the host's `script`, `lua`, +`dlib`, `profile_null`, and `ddf` libraries): + +```sh +python3 tests/run_native_tests.py --defold-home /path/to/defold/tmp/dynamo_home +``` + +The tests compile the production callback code and use Defold's real Lua and +callback helpers. They exercise callback arguments, navigation return values, +deleted script instances, stale generations after view-slot reuse, destruction/replacement during callbacks with forced +garbage collection, error handling, stack restoration, and reference cleanup. +AddressSanitizer and UndefinedBehaviorSanitizer instrument the test and extension +code. Pass `--sanitizer thread` to use ThreadSanitizer instead of AddressSanitizer. +A second executable exercises the production Darwin queue and delegates, +including invalid IDs, slot reuse within one drained batch, cancellation of +pending navigation decisions, concurrent queue draining, a producer racing shutdown, and late events after +finalization/reinitialization. These tests use platform/script-boundary test +doubles and create no browser windows; they do not replace testing the native +WebView lifecycle on Android and Apple devices. + +## Device regression project + +Copy `tests/lifecycle` to a temporary directory, then copy this checkout's +`webview` directory into that project. Open its `game.project` in Defold and +bundle it for Android, iOS, or macOS. It has no remote dependencies. + +The project destroys an unopened view, repeatedly destroys/recreates views in +the same slot while JavaScript emits events, alternates visible and hidden views, +and destroys a view from inside its callback. It must print +`WebView lifecycle regression passed` and exit without crashes, assertion errors, +callbacks after destruction, or events delivered to a replacement view. + +Run Android debug and release bundles, including armeabi-v7a, for issue #40. +The host runner checks this script's Lua syntax but does not execute the device test. diff --git a/tests/lifecycle/game.input_binding b/tests/lifecycle/game.input_binding new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/lifecycle/game.input_binding @@ -0,0 +1 @@ + diff --git a/tests/lifecycle/game.project b/tests/lifecycle/game.project new file mode 100644 index 0000000..9ca2bfd --- /dev/null +++ b/tests/lifecycle/game.project @@ -0,0 +1,21 @@ +[project] +title = WebView lifecycle regression + +[bootstrap] +main_collection = /main.collectionc + +[input] +game_binding = /game.input_bindingc + +[display] +width = 320 +height = 480 + +[script] +shared_state = 1 + +[android] +package = com.defold.webview.lifecycle + +[ios] +bundle_identifier = com.defold.webview.lifecycle diff --git a/tests/lifecycle/main.collection b/tests/lifecycle/main.collection new file mode 100644 index 0000000..4ad753d --- /dev/null +++ b/tests/lifecycle/main.collection @@ -0,0 +1,8 @@ +name: "main" +embedded_instances { + id: "main" + data: "components {\n" + " id: \"script\"\n" + " component: \"/main.script\"\n" + "}\n" +} diff --git a/tests/lifecycle/main.script b/tests/lifecycle/main.script new file mode 100644 index 0000000..f6bebad --- /dev/null +++ b/tests/lifecycle/main.script @@ -0,0 +1,62 @@ +-- Bundle this project with the local webview directory to exercise native teardown. +local function destroy(record) + if not record.dead then + record.dead = true + assert(webview.destroy(record.id) == 0) + end +end + +local function create(self, close_in_callback) + self.serial = self.serial + 1 + local record = { token = "webview-lifetime-" .. self.serial } + record.id = webview.create(function(_, id, request, event, data) + assert(not record.dead, "callback delivered after destruction") + if data.result then + local token = data.result:match("webview%-lifetime%-%d+") + assert(not token or token == record.token, "event delivered to a replacement view") + end + if close_in_callback then + record.id = id + destroy(record) + self.closed_in_callback = true + end + return true + end) + assert(record.id >= 0) + local html = "WebView lifecycle test" + webview.open_raw(record.id, html, { hidden = self.serial % 2 == 0, transparent = false }) + if not record.dead then + webview.eval(record.id, "'" .. record.token .. "'") + end + return record +end + +function init(self) + assert(webview, "run on Android, iOS, or macOS with the local extension") + -- A view that was never attached to a window must also be safe to destroy. + local unseen = webview.create(function() error("unexpected callback for unopened view") end) + assert(unseen >= 0 and webview.destroy(unseen) == 0) + + self.serial = 0 + self.current = create(self, false) + self.loop = timer.delay(0.1, true, function() + destroy(self.current) + if self.serial < 20 then + self.current = create(self, false) + else + timer.cancel(self.loop) + self.current = create(self, true) + timer.delay(5, false, function() + assert(self.closed_in_callback, "no callback received before timeout") + print("WebView lifecycle regression passed") + sys.exit(0) + end) + end + end) +end + +function final(self) + if self.current then destroy(self.current) end +end diff --git a/tests/run_native_tests.py b/tests/run_native_tests.py new file mode 100644 index 0000000..e6592b1 --- /dev/null +++ b/tests/run_native_tests.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python3 +"""Run host regression tests against a built Defold SDK on macOS.""" +import argparse +import os +from pathlib import Path +import platform +import subprocess +import tempfile + +parser = argparse.ArgumentParser(description=__doc__) +parser.add_argument("--defold-home", default=os.environ.get("WEBVIEW_DEFOLD_HOME"), + help="Defold build/install directory containing sdk/include and lib/") +parser.add_argument("--sanitizer", choices=["address", "thread"], default="address") +args = parser.parse_args() +if not args.defold_home: + parser.error("provide --defold-home or WEBVIEW_DEFOLD_HOME") +if platform.system() != "Darwin": + parser.error("this runner currently supports macOS hosts") +sdk = Path(args.defold_home).resolve() +root = Path(__file__).resolve().parents[1] +host = "arm64-macos" if platform.machine() == "arm64" else "x86_64-macos" +with tempfile.TemporaryDirectory(prefix="webview-tests-") as directory: + common_flags = [ + "clang++", "-std=c++11", "-g", "-fsanitize=" + args.sanitizer + ",undefined", + "-fno-omit-frame-pointer", "-Wl,-dead_strip", "-Wno-nontrivial-memcall", + "-DDM_PLATFORM_OSX", '-DDLIB_LOG_DOMAIN="WEBVIEW_TEST"', + "-I" + str(sdk / "sdk/include"), "-I" + str(sdk / "include"), + "-I" + str(root / "webview/src"), + "-L" + str(sdk / "lib" / host), "-lscript", "-llua", "-ldlib", + "-lprofile_null", "-lddf", "-framework", "CoreFoundation", + ] + for name, sources, flags in [ + ("test_callbacks", ["tests/test_callbacks.cpp", "webview/src/webview_common.cpp"], []), + ("test_darwin", ["tests/test_darwin.mm"], ["-framework", "WebKit", "-framework", "AppKit"]), + ]: + executable = Path(directory) / name + subprocess.run(common_flags + [str(root / source) for source in sources] + + flags + ["-o", str(executable)], check=True) + arguments = [str(root / "tests/lifecycle/main.script")] if name == "test_callbacks" else [] + subprocess.run([str(executable)] + arguments, check=True) diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp new file mode 100644 index 0000000..01c8a61 --- /dev/null +++ b/tests/test_callbacks.cpp @@ -0,0 +1,211 @@ +// Exercises the production callback code with Defold's real Lua and script libraries. +#include +#include +#include +#include "webview_common.h" +extern "C" { +#include +} + +// Engine helpers used to create a script-instance fixture and check reference accounting. +namespace dmScript { +int GetLuaRefCount(); +uint32_t RegisterUserType(lua_State* L, const char* name, const luaL_reg methods[], const luaL_reg meta[]); +} + +static dmWebView::WebViewInfo g_Info; +static int g_Allowed; +static int g_Cancelled; +static int g_ContextRef; +static bool g_InstanceValid = true; + +namespace dmWebView { +int Platform_ContinueOpen(lua_State*, int, int request, const char*) { ++g_Allowed; return request; } +int Platform_CancelOpen(lua_State*, int, int request, const char*) { ++g_Cancelled; return request; } +} + +static int InstanceValid(lua_State* L) { lua_pushboolean(L, g_InstanceValid); return 1; } +static int ContextRef(lua_State* L) { lua_pushinteger(L, g_ContextRef); return 1; } +static int ScriptId(lua_State* L) { lua_pushinteger(L, 1); return 1; } + +static void RunLua(lua_State* L, const char* source) +{ + int result = luaL_dostring(L, source); + if (result) fprintf(stderr, "%s\n", lua_tostring(L, -1)); + assert(result == 0); +} + +static void SetCallback(lua_State* L, const char* body) +{ + std::string source = "return function(self, view, request, event, data) "; + source += body; + source += " end"; + RunLua(L, source.c_str()); + dmWebView::CreateWebViewInfo(&g_Info, L, -1); + assert(g_Info.m_Callback); + lua_pop(L, 1); +} + +static int Destroy(lua_State*) { dmWebView::ClearWebViewInfo(&g_Info); return 0; } +static int Replace(lua_State* L) +{ + dmWebView::ClearWebViewInfo(&g_Info); + SetCallback(L, "replacement_calls = replacement_calls + 1"); + return 0; +} + +static void Dispatch(lua_State* L, dmWebView::CallbackResult type = dmWebView::CALLBACK_RESULT_URL_LOADING, + uint64_t generation = 0) +{ + dmWebView::CallbackInfo event; + event.m_Info = &g_Info; + event.m_Generation = generation ? generation : g_Info.m_Generation; + event.m_Type = type; + event.m_WebViewID = 2; + event.m_RequestID = 7; + event.m_Url = "https://example.invalid/"; + event.m_Result = "tick"; + + // A pre-existing stack entry must not be mistaken for the callback result. + int top = lua_gettop(L); + lua_pushboolean(L, 0); + dmScript::GetInstance(L); + int previous = luaL_ref(L, LUA_REGISTRYINDEX); + lua_pushnil(L); + dmScript::SetInstance(L); + dmWebView::RunCallback(&event); + assert(lua_gettop(L) == top + 1); + dmScript::GetInstance(L); + assert(lua_isnil(L, -1)); + lua_pop(L, 2); + lua_rawgeti(L, LUA_REGISTRYINDEX, previous); + dmScript::SetInstance(L); + luaL_unref(L, LUA_REGISTRYINDEX, previous); +} + +// Verifies the public callback arguments and nil/true/false navigation decisions. +static void TestNavigation(lua_State* L) +{ + const char* returns[] = { "return nil", "return true", "return false" }; + for (int i = 0; i < 3; ++i) + { + std::string body = "assert(self ~= nil and view == 2 and request == 7 and event == -3); " + "assert(data.url == 'https://example.invalid/' and data.result == 'tick'); "; + body += returns[i]; + SetCallback(L, body.c_str()); + g_Allowed = g_Cancelled = 0; + Dispatch(L); + assert(g_Allowed == (i < 2)); + assert(g_Cancelled == (i == 2)); + dmWebView::ClearWebViewInfo(&g_Info); + } +} + +// Verifies that missing callbacks and deleted script instances never enter Lua. +static void TestInvalidCallbacks(lua_State* L) +{ + Dispatch(L); + RunLua(L, "invalid_calls = 0"); + SetCallback(L, "invalid_calls = invalid_calls + 1"); + g_InstanceValid = false; + Dispatch(L, dmWebView::CALLBACK_RESULT_EVAL_OK); + g_InstanceValid = true; + dmWebView::ClearWebViewInfo(&g_Info); + Dispatch(L); + dmWebView::ClearWebViewInfo(&g_Info); + RunLua(L, "assert(invalid_calls == 0)"); +} + +// Verifies teardown remains safe when Lua destroys/replaces the view and forces GC. +static void TestReentrantDestruction(lua_State* L) +{ + const char* actions[] = { "destroy_view()", "replace_view()" }; + for (int i = 0; i < 2; ++i) + { + std::string body = actions[i]; + body += "; collectgarbage('collect'); return true"; + SetCallback(L, body.c_str()); + g_Allowed = g_Cancelled = 0; + Dispatch(L); + assert(g_Allowed == 0 && g_Cancelled == 0); + if (i == 1) + { + Dispatch(L, dmWebView::CALLBACK_RESULT_EVAL_OK); + RunLua(L, "assert(replacement_calls == 1)"); + } + dmWebView::ClearWebViewInfo(&g_Info); + } +} + +// Verifies Lua errors leave a balanced stack and cancel pending navigation. +static void TestCallbackError(lua_State* L) +{ + SetCallback(L, "error('expected callback test error')"); + g_Allowed = g_Cancelled = 0; + Dispatch(L); + assert(g_Allowed == 0 && g_Cancelled == 1); + dmWebView::ClearWebViewInfo(&g_Info); +} + +// Verifies old events cannot reach a new callback after the same slot is reused. +static void TestStaleGeneration(lua_State* L) +{ + RunLua(L, "stale_calls = 0"); + SetCallback(L, "stale_calls = stale_calls + 1"); + uint64_t old_generation = g_Info.m_Generation; + dmWebView::ClearWebViewInfo(&g_Info); + Dispatch(L, dmWebView::CALLBACK_RESULT_EVAL_OK, old_generation); + SetCallback(L, "stale_calls = stale_calls + 1"); + assert(old_generation != g_Info.m_Generation); + // Covers both events queued before destruction and late events from the old producer. + Dispatch(L, dmWebView::CALLBACK_RESULT_EVAL_OK, old_generation); + Dispatch(L, dmWebView::CALLBACK_RESULT_URL_LOADING, old_generation); + RunLua(L, "assert(stale_calls == 0)"); + Dispatch(L, dmWebView::CALLBACK_RESULT_EVAL_OK); + RunLua(L, "assert(stale_calls == 1)"); + dmWebView::ClearWebViewInfo(&g_Info); +} + +int main(int argc, char** argv) +{ + lua_State* L = luaL_newstate(); + luaL_openlibs(L); + lua_pushlightuserdata(L, L); + lua_setglobal(L, "__script_main_thread"); + const luaL_reg methods[] = { { 0, 0 } }; + const luaL_reg meta[] = { + { "__is_valid", InstanceValid }, + { "__get_instance_context_table_ref", ContextRef }, + { "__get_unique_script_id", ScriptId }, + { 0, 0 } + }; + dmScript::RegisterUserType(L, "WebViewTestInstance", methods, meta); + lua_newtable(L); + g_ContextRef = luaL_ref(L, LUA_REGISTRYINDEX); + lua_newuserdata(L, 1); + luaL_getmetatable(L, "WebViewTestInstance"); + lua_setmetatable(L, -2); + dmScript::SetInstance(L); + lua_register(L, "destroy_view", Destroy); + lua_register(L, "replace_view", Replace); + RunLua(L, "replacement_calls = 0"); + + int references = dmScript::GetLuaRefCount(); + TestNavigation(L); + TestInvalidCallbacks(L); + TestReentrantDestruction(L); + TestCallbackError(L); + TestStaleGeneration(L); + assert(dmScript::GetLuaRefCount() == references); + assert(lua_gettop(L) == 0); + for (int i = 1; i < argc; ++i) + { + int result = luaL_loadfile(L, argv[i]); + if (result) fprintf(stderr, "%s\n", lua_tostring(L, -1)); + assert(result == 0); + lua_pop(L, 1); + } + lua_close(L); + puts("Callback lifecycle tests passed (real Defold Lua/script libraries)."); + return 0; +} diff --git a/tests/test_darwin.mm b/tests/test_darwin.mm new file mode 100644 index 0000000..3a35ab5 --- /dev/null +++ b/tests/test_darwin.mm @@ -0,0 +1,189 @@ +// Exercise the production event queue and delegate without creating a browser window. +#include +#include +#include +#include +#include +#include "../webview/src/webview_darwin.mm" + +static int g_Delivered; +static std::function g_OnCallback; + +namespace dmGraphics { +id GetNativeOSXNSWindow() { return nil; } +id GetNativeOSXNSView() { return nil; } +} + +namespace dmWebView { +void ClearWebViewInfo(WebViewInfo* info) { info->m_Callback = 0; } +void RunCallback(CallbackInfo* event) +{ + assert(event->m_WebViewID == 0); + assert(event->m_Generation == g_WebView.m_Info[0].m_Generation); + assert(strcmp(event->m_Result, "payload") == 0); + ++g_Delivered; + if (g_OnCallback) g_OnCallback(); +} +} + +static uint64_t NewView() +{ + static uint64_t generation = 0; + g_WebView.m_Info[0].m_Callback = (dmScript::LuaCallbackInfo*)1; + g_WebView.m_Info[0].m_Generation = ++generation; + return generation; +} + +static void QueueEvent(uint64_t generation, int view_id = 0) +{ + Command command; + command.m_Type = CMD_EVAL_OK; + command.m_Generation = generation; + command.m_WebViewID = view_id; + command.m_Url = strdup("https://example.invalid/"); + command.m_Data = strdup("payload"); + QueueCommand(&command); +} + +// Verifies the queue rejects destroyed/reused slots and malformed IDs before dispatch. +static void TestQueuedEvents() +{ + uint64_t old_generation = NewView(); + QueueEvent(old_generation); + dmWebView::ClearWebViewInfo(&g_WebView.m_Info[0]); + dmWebView::Platform_Update(0); + assert(g_Delivered == 0); + + uint64_t generation = NewView(); + QueueEvent(old_generation); + QueueEvent(generation, -1); + QueueEvent(generation, dmWebView::MAX_NUM_WEBVIEWS); + QueueEvent(generation); + dmWebView::Platform_Update(0); + assert(g_Delivered == 1); + + // The first callback invalidates commands already swapped into the update's local batch. + g_Delivered = 0; + g_OnCallback = [] { NewView(); }; + QueueEvent(generation); + QueueEvent(generation); + dmWebView::Platform_Update(0); + assert(g_Delivered == 1); + g_OnCallback = {}; +} + +// Verifies stale direct delegates are ignored and their navigation decision is cancelled. +static void TestStaleDelegate() +{ + uint64_t old_generation = NewView(); + WebViewDelegate* delegate = [[WebViewDelegate alloc] init]; + delegate->m_WebViewID = 0; + delegate->m_Generation = old_generation; + NewView(); + g_Delivered = 0; + // Stale delegates must return before accessing these browser/action arguments. + WKWebView* unused_view = nil; + WKNavigationAction* unused_action = nil; + NSError* unused_error = nil; + [delegate webView:unused_view didFinishNavigation:nil]; + [delegate webView:unused_view didFailNavigation:nil withError:unused_error]; + assert(g_Delivered == 0); + + __block int decisions = 0; + [delegate webView:unused_view decidePolicyForNavigationAction:unused_action decisionHandler:^(WKNavigationActionPolicy policy) { + assert(policy == WKNavigationActionPolicyCancel); + ++decisions; + }]; + assert(decisions == 1); + [delegate release]; +} + +// Verifies destroying a view resolves its pending decision exactly once. +static void TestPendingDecision() +{ + uint64_t generation = NewView(); + WebViewDelegate* delegate = [[WebViewDelegate alloc] init]; + delegate->m_WebViewID = 0; + delegate->m_Generation = generation; + delegate->m_PendingUrl = [@"https://example.invalid/" copy]; + __block int decisions = 0; + delegate->m_DecisionHandler = [^(WKNavigationActionPolicy policy) { + assert(policy == WKNavigationActionPolicyCancel); + ++decisions; + } copy]; + g_WebView.m_WebViewDelegates[0] = delegate; + assert(dmWebView::Platform_Destroy(0, 0) == 0); + assert(decisions == 1); + assert(!g_WebView.m_Info[0].m_Callback && !g_WebView.m_WebViewDelegates[0]); +} + +// Verifies queue reads and writes remain synchronized while update drains events. +static void TestConcurrentUpdate() +{ + uint64_t generation = NewView(); + std::atomic finished(false); + g_Delivered = 0; + std::thread producer([&] { + for (int i = 0; i < 1000; ++i) QueueEvent(generation); + finished.store(true); + }); + while (!finished.load()) dmWebView::Platform_Update(0); + producer.join(); + dmWebView::Platform_Update(0); + assert(g_Delivered == 1000); +} + +// Verifies pending and concurrently arriving events are discarded during shutdown. +static void TestShutdown() +{ + uint64_t generation = NewView(); + QueueEvent(generation); + std::atomic start(false); + std::thread producer([&] { + while (!start.load()) std::this_thread::yield(); + for (int i = 0; i < 1000; ++i) QueueEvent(generation); + }); + start.store(true); + dmWebView::Platform_Finalize(0); + producer.join(); + assert(g_WebView.m_CmdQueue.Empty()); + assert(!g_WebView.m_AcceptCommands); + + dmMutex::HMutex mutex = g_WebView.m_Mutex; + dmWebView::Platform_AppFinalize(0); + QueueEvent(generation); + assert(g_WebView.m_Mutex == mutex); + assert(g_WebView.m_CmdQueue.Empty()); + + // Reopening the queue must still reject results from the previous lifetime. + dmWebView::Platform_AppInitialize(0); + dmWebView::Platform_Initialize(0); + assert(g_WebView.m_Mutex == mutex); + uint64_t current = NewView(); + g_Delivered = 0; + QueueEvent(generation); + QueueEvent(current); + dmWebView::Platform_Update(0); + assert(g_Delivered == 1); + dmWebView::ClearWebViewInfo(&g_WebView.m_Info[0]); +} + +int main() +{ + @autoreleasepool + { + QueueEvent(1); + assert(g_WebView.m_CmdQueue.Empty()); + dmWebView::Platform_AppInitialize(0); + dmWebView::Platform_Initialize(0); + TestQueuedEvents(); + TestStaleDelegate(); + TestPendingDecision(); + TestConcurrentUpdate(); + TestShutdown(); + dmWebView::Platform_Finalize(0); + dmWebView::Platform_AppFinalize(0); + } + puts("Darwin event queue and delegate lifecycle tests passed."); + return 0; +} diff --git a/webview/src/java/com/defold/webview/WebViewJNI.java b/webview/src/java/com/defold/webview/WebViewJNI.java index 9ec127d..acc92b5 100644 --- a/webview/src/java/com/defold/webview/WebViewJNI.java +++ b/webview/src/java/com/defold/webview/WebViewJNI.java @@ -46,19 +46,22 @@ public class WebViewJNI { public static final String JS_NAMESPACE = "defold"; private Activity activity; - private static WebViewInfo[] infos; + private final WebViewInfo[] infos; private boolean immersiveMode = false; private boolean displayCutout = false; - public native void onPageLoading(String url, int webview_id, int request_id); - public native void onPageFinished(String url, int webview_id, int request_id); - public native void onReceivedError(String url, int webview_id, int request_id, String errorMessage); - public native void onEvalFinished(String result, int webview_id, int request_id); - public native void onEvalFailed(String errorMessage, int webview_id, int request_id); + public native void onPageLoading(String url, int webview_id, long generation, int request_id); + public native void onPageFinished(String url, int webview_id, long generation, int request_id); + public native void onReceivedError(String url, int webview_id, long generation, int request_id, String errorMessage); + public native void onEvalFinished(String result, int webview_id, long generation, int request_id); + public native void onEvalFailed(String errorMessage, int webview_id, long generation, int request_id); private static class CustomWebViewClient extends WebViewClient { public Activity activity; public int webviewID; + private final long generation; + // JavaScript interface calls arrive on a WebView background thread. + private volatile boolean destroyed; public int requestID; private String continueLoadingUrl; private WebViewJNI webviewJNI; @@ -70,11 +73,12 @@ private static class CustomWebViewClient extends WebViewClient { // This guard variable helps to avoid propagating the onPageFinished calls in that case private boolean hasError; - public CustomWebViewClient(Activity activity, WebViewJNI webviewJNI, int webview_id) { + public CustomWebViewClient(Activity activity, WebViewJNI webviewJNI, int webview_id, long generation) { super(); this.activity = activity; this.webviewJNI = webviewJNI; this.webviewID = webview_id; + this.generation = generation; PACKAGE_NAME = activity.getApplicationContext().getPackageName(); reset(-1); } @@ -88,6 +92,10 @@ public void addHeader(final String header, final String value) { } } + public void destroy() { + destroyed = true; + } + public void clearHeaders() { extraHeaders.clear(); } @@ -122,6 +130,7 @@ private boolean shouldContinueLoadingUrl(String url) { @Override public WebResourceResponse shouldInterceptRequest(WebView view, WebResourceRequest request) { + if (destroyed) return null; final String method = request.getMethod(); final String url = request.getUrl().toString(); String ext = MimeTypeMap.getFileExtensionFromUrl(url); @@ -152,6 +161,7 @@ public WebResourceResponse shouldInterceptRequest(WebView view, WebResourceReque @Override public boolean shouldOverrideUrlLoading(WebView view, String url) { + if (destroyed) return true; if( url.startsWith(PACKAGE_NAME) ) { // Try to find an app that can open the url scheme, @@ -174,23 +184,25 @@ public boolean shouldOverrideUrlLoading(WebView view, String url) { } // block the page from loading and ask the client if it should load // or not - webviewJNI.onPageLoading(url, webviewID, requestID); + webviewJNI.onPageLoading(url, webviewID, generation, requestID); return true; } @Override public void onPageFinished(WebView view, String url) { + if (destroyed) return; // NOTE! this callback will be called TWICE for errors, see comment above // NOTE! this callback will be called once when initially blocked in // shouldOverrideUrlLoading and then once more if allowed to load if (!this.hasError && shouldContinueLoadingUrl(url)) { continueLoadingUrl = null; - webviewJNI.onPageFinished(url, webviewID, requestID); + webviewJNI.onPageFinished(url, webviewID, generation, requestID); } } @Override public void onReceivedError(WebView view, int errorCode, String description, String failingUrl) { + if (destroyed) return; if (errorCode == WebViewClient.ERROR_UNSUPPORTED_SCHEME) { // Try to find an app that can open the url scheme, // otherwise continue as usual error. @@ -205,24 +217,29 @@ public void onReceivedError(WebView view, int errorCode, String description, Str if (!this.hasError) { this.hasError = true; - webviewJNI.onReceivedError(failingUrl, webviewID, requestID, description); + webviewJNI.onReceivedError(failingUrl, webviewID, generation, requestID, description); } } @JavascriptInterface public void returnResultToJava(String result) { - webviewJNI.onEvalFinished(result, webviewID, requestID); + if (!destroyed) { + webviewJNI.onEvalFinished(result, webviewID, generation, requestID); + } } } private static class CustomWebChromeClient extends WebChromeClient { private WebViewJNI webviewJNI; private int webviewID; + private final long generation; + private volatile boolean destroyed; private int requestID; - public CustomWebChromeClient(WebViewJNI webviewJNI, int webview_id) { + public CustomWebChromeClient(WebViewJNI webviewJNI, int webview_id, long generation) { this.webviewJNI = webviewJNI; this.webviewID = webview_id; + this.generation = generation; reset(-1); } @@ -231,11 +248,16 @@ public void reset(int request_id) this.requestID = request_id; } + public void destroy() { + destroyed = true; + } + @Override public boolean onConsoleMessage(ConsoleMessage msg) { + if (destroyed) return false; if( msg.messageLevel() == ConsoleMessage.MessageLevel.ERROR ) { - webviewJNI.onEvalFailed(String.format("js:%d: %s", msg.lineNumber(), msg.message()), webviewID, requestID); + webviewJNI.onEvalFailed(String.format("js:%d: %s", msg.lineNumber(), msg.message()), webviewID, generation, requestID); return true; } return false; @@ -260,7 +282,7 @@ public WebViewJNI(Activity activity, int maxnumviews, boolean immersiveMode, boo this.displayCutout = displayCutout; } - private WebViewInfo createView(Activity activity, int webview_id) + private WebViewInfo createView(Activity activity, int webview_id, long generation) { WebViewInfo info = new WebViewInfo(); info.webviewID = webview_id; @@ -284,10 +306,10 @@ private WebViewInfo createView(Activity activity, int webview_id) info.webview.requestFocusFromTouch(); - info.webviewChromeClient = new CustomWebChromeClient(WebViewJNI.this, webview_id); + info.webviewChromeClient = new CustomWebChromeClient(WebViewJNI.this, webview_id, generation); info.webview.setWebChromeClient(info.webviewChromeClient); - info.webviewClient = new CustomWebViewClient(activity, WebViewJNI.this, webview_id); + info.webviewClient = new CustomWebViewClient(activity, WebViewJNI.this, webview_id, generation); info.webview.setWebViewClient(info.webviewClient); WebSettings webSettings = info.webview.getSettings(); @@ -377,11 +399,11 @@ private void setPositionInternal(WebViewInfo info, int x, int y, int width, int } } - public void create(final int webview_id) { + public void create(final int webview_id, final long generation) { this.activity.runOnUiThread(new Runnable() { @Override public void run() { - WebViewJNI.this.infos[webview_id] = createView(WebViewJNI.this.activity, webview_id); + WebViewJNI.this.infos[webview_id] = createView(WebViewJNI.this.activity, webview_id, generation); } }); } @@ -390,17 +412,31 @@ public void destroy(final int webview_id) { this.activity.runOnUiThread(new Runnable() { @Override public void run() { - if( WebViewJNI.this.infos[webview_id] != null ) - { - if( WebViewJNI.this.infos[webview_id].layout != null ) - { - WindowManager wm = activity.getWindowManager(); - wm.removeView(WebViewJNI.this.infos[webview_id].layout); - } - WebViewJNI.this.infos[webview_id].layout = null; - WebViewJNI.this.infos[webview_id].webview = null; - WebViewJNI.this.infos[webview_id] = null; + WebViewInfo info = WebViewJNI.this.infos[webview_id]; + if (info == null) return; + WebViewJNI.this.infos[webview_id] = null; + + info.webviewClient.destroy(); + info.webviewChromeClient.destroy(); + WebView view = info.webview; + view.removeJavascriptInterface(JS_NAMESPACE); + view.setWebChromeClient(null); + view.setWebViewClient(null); + view.stopLoading(); + + ViewGroup parent = (ViewGroup)view.getParent(); + if (parent != null) { + parent.removeView(view); + } + // A newly created or hidden view may never have been added to the window. + if (info.layout.getParent() != null) { + activity.getWindowManager().removeView(info.layout); } + view.destroy(); + info.webview = null; + info.layout = null; + info.webviewClient = null; + info.webviewChromeClient = null; } }); } diff --git a/webview/src/webview_android.cpp b/webview/src/webview_android.cpp index 0e03a8c..ca7c229 100644 --- a/webview/src/webview_android.cpp +++ b/webview/src/webview_android.cpp @@ -32,6 +32,7 @@ struct WebViewCommand memset(this, 0, sizeof(WebViewCommand)); } CommandType m_Type; + uint64_t m_Generation; int m_WebViewID; int m_RequestID; void* m_Data; @@ -71,15 +72,22 @@ struct WebViewExtensionState jmethodID m_AddHeader; jmethodID m_ClearHeaders; dmMutex::HMutex m_Mutex; + bool m_AcceptCommands; dmArray m_CmdQueue; }; WebViewExtensionState g_WebView; +static void FreeCommand(const WebViewCommand& cmd) +{ + free((void*)cmd.m_Url); + free(cmd.m_Data); +} + namespace dmWebView { -#define CHECK_WEBVIEW_AND_RETURN() if( webview_id >= MAX_NUM_WEBVIEWS || webview_id < 0 ) { dmLogError("%s: Invalid webview_id: %d", __FUNCTION__, webview_id); return -1; } +#define CHECK_WEBVIEW_AND_RETURN() if( webview_id >= MAX_NUM_WEBVIEWS || webview_id < 0 || !g_WebView.m_Used[webview_id] ) { dmLogError("%s: Invalid webview_id: %d", __FUNCTION__, webview_id); return -1; } int Platform_Create(lua_State* L, dmWebView::WebViewInfo* _info) { @@ -105,7 +113,7 @@ int Platform_Create(lua_State* L, dmWebView::WebViewInfo* _info) dmAndroid::ThreadAttacher threadAttacher; JNIEnv* env = threadAttacher.GetEnv(); - env->CallVoidMethod(g_WebView.m_WebViewJNI, g_WebView.m_Create, webview_id); + env->CallVoidMethod(g_WebView.m_WebViewJNI, g_WebView.m_Create, webview_id, (jlong)_info->m_Generation); return webview_id; } @@ -113,18 +121,17 @@ int Platform_Create(lua_State* L, dmWebView::WebViewInfo* _info) static int DestroyWebView(int webview_id) { CHECK_WEBVIEW_AND_RETURN(); + ClearWebViewInfo(&g_WebView.m_Info[webview_id]); + g_WebView.m_Used[webview_id] = false; dmAndroid::ThreadAttacher threadAttacher; JNIEnv* env = threadAttacher.GetEnv(); env->CallVoidMethod(g_WebView.m_WebViewJNI, g_WebView.m_Destroy, webview_id); - ClearWebViewInfo(&g_WebView.m_Info[webview_id]); - g_WebView.m_Used[webview_id] = false; return 0; } int Platform_Destroy(lua_State* L, int webview_id) { - DestroyWebView(webview_id); - return 0; + return DestroyWebView(webview_id); } int Platform_Open(lua_State* L, int webview_id, const char* url, dmWebView::RequestInfo* options) @@ -254,7 +261,17 @@ static char* CopyString(JNIEnv* env, jstring s) static void QueueCommand(WebViewCommand* cmd) { + if (!g_WebView.m_Mutex) + { + FreeCommand(*cmd); + return; + } DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + if (!g_WebView.m_AcceptCommands) + { + FreeCommand(*cmd); + return; + } if (g_WebView.m_CmdQueue.Full()) { g_WebView.m_CmdQueue.OffsetCapacity(8); @@ -262,59 +279,78 @@ static void QueueCommand(WebViewCommand* cmd) g_WebView.m_CmdQueue.Push(*cmd); } +static void FinalizeCommands() +{ + if (!g_WebView.m_Mutex) + return; + + dmArray pending; + { + DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + g_WebView.m_AcceptCommands = false; + pending.Swap(g_WebView.m_CmdQueue); + } + for (uint32_t i = 0; i != pending.Size(); ++i) + FreeCommand(pending[i]); +} #ifdef __cplusplus extern "C" { #endif -JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onPageFinished(JNIEnv* env, jobject, jstring url, jint webview_id, jint request_id) +JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onPageFinished(JNIEnv* env, jobject, jstring url, jint webview_id, jlong generation, jint request_id) { WebViewCommand cmd; cmd.m_Type = CMD_LOAD_OK; cmd.m_WebViewID = webview_id; + cmd.m_Generation = (uint64_t)generation; cmd.m_RequestID = request_id; cmd.m_Url = CopyString(env, url); QueueCommand(&cmd); } -JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onReceivedError(JNIEnv* env, jobject, jstring url, jint webview_id, jint request_id, jstring errorMessage) +JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onReceivedError(JNIEnv* env, jobject, jstring url, jint webview_id, jlong generation, jint request_id, jstring errorMessage) { WebViewCommand cmd; cmd.m_Type = CMD_LOAD_ERROR; cmd.m_WebViewID = webview_id; + cmd.m_Generation = (uint64_t)generation; cmd.m_RequestID = request_id; cmd.m_Url = CopyString(env, url); cmd.m_Data = CopyString(env, errorMessage); QueueCommand(&cmd); } -JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onEvalFinished(JNIEnv* env, jobject, jstring result, jint webview_id, jint request_id) +JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onEvalFinished(JNIEnv* env, jobject, jstring result, jint webview_id, jlong generation, jint request_id) { WebViewCommand cmd; cmd.m_Type = CMD_EVAL_OK; cmd.m_WebViewID = webview_id; + cmd.m_Generation = (uint64_t)generation; cmd.m_RequestID = request_id; cmd.m_Url = 0; cmd.m_Data = CopyString(env, result); QueueCommand(&cmd); } -JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onEvalFailed(JNIEnv* env, jobject, jstring error, jint webview_id, jint request_id) +JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onEvalFailed(JNIEnv* env, jobject, jstring error, jint webview_id, jlong generation, jint request_id) { WebViewCommand cmd; cmd.m_Type = CMD_EVAL_ERROR; cmd.m_WebViewID = webview_id; + cmd.m_Generation = (uint64_t)generation; cmd.m_RequestID = request_id; cmd.m_Url = 0; cmd.m_Data = CopyString(env, error); QueueCommand(&cmd); } -JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onPageLoading(JNIEnv* env, jobject, jstring url, jint webview_id, jint request_id) +JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onPageLoading(JNIEnv* env, jobject, jstring url, jint webview_id, jlong generation, jint request_id) { WebViewCommand cmd; cmd.m_Type = CMD_LOADING; cmd.m_WebViewID = webview_id; + cmd.m_Generation = (uint64_t)generation; cmd.m_RequestID = request_id; cmd.m_Url = CopyString(env, url); QueueCommand(&cmd); @@ -326,22 +362,27 @@ JNIEXPORT void JNICALL Java_com_defold_webview_WebViewJNI_onPageLoading(JNIEnv* dmExtension::Result Platform_Update(dmExtension::Params* params) { - if (g_WebView.m_CmdQueue.Empty()) - { - return dmExtension::RESULT_OK; // avoid a lock (~300us on iPhone 4s) - } - dmArray tmp; { DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + if (!g_WebView.m_AcceptCommands || g_WebView.m_CmdQueue.Empty()) + return dmExtension::RESULT_OK; tmp.Swap(g_WebView.m_CmdQueue); } for (uint32_t i=0; i != tmp.Size(); ++i) { const WebViewCommand& cmd = tmp[i]; + if (cmd.m_WebViewID < 0 || cmd.m_WebViewID >= MAX_NUM_WEBVIEWS || + !g_WebView.m_Used[cmd.m_WebViewID] || + cmd.m_Generation != g_WebView.m_Info[cmd.m_WebViewID].m_Generation) + { + FreeCommand(cmd); + continue; + } dmWebView::CallbackInfo cbinfo; + cbinfo.m_Generation = cmd.m_Generation; switch (cmd.m_Type) { case CMD_LOADING: @@ -397,26 +438,25 @@ dmExtension::Result Platform_Update(dmExtension::Params* params) default: assert(false); } - if (cmd.m_Url) { - free((void*)cmd.m_Url); - } - if (cmd.m_Data) { - free(cmd.m_Data); - } + FreeCommand(cmd); } return dmExtension::RESULT_OK; } dmExtension::Result Platform_AppInitialize(dmExtension::AppParams* params) { - g_WebView.m_Mutex = dmMutex::New(); - g_WebView.m_CmdQueue.SetCapacity(8); + if (!g_WebView.m_Mutex) + g_WebView.m_Mutex = dmMutex::New(); + { + DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + g_WebView.m_CmdQueue.SetCapacity(8); + } dmAndroid::ThreadAttacher threadAttacher; JNIEnv* env = threadAttacher.GetEnv(); jclass webview_class = dmAndroid::LoadClass(env, "com.defold.webview.WebViewJNI"); - g_WebView.m_Create = env->GetMethodID(webview_class, "create", "(I)V"); + g_WebView.m_Create = env->GetMethodID(webview_class, "create", "(IJ)V"); g_WebView.m_Destroy = env->GetMethodID(webview_class, "destroy", "(I)V"); g_WebView.m_Load = env->GetMethodID(webview_class, "load", "(Ljava/lang/String;IIII)V"); g_WebView.m_LoadRaw = env->GetMethodID(webview_class, "loadRaw", "(Ljava/lang/String;IIII)V"); @@ -439,23 +479,28 @@ dmExtension::Result Platform_AppInitialize(dmExtension::AppParams* params) dmExtension::Result Platform_Initialize(dmExtension::Params* params) { + DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + g_WebView.m_AcceptCommands = true; return dmExtension::RESULT_OK; } dmExtension::Result Platform_AppFinalize(dmExtension::AppParams* params) { + FinalizeCommands(); dmAndroid::ThreadAttacher threadAttacher; JNIEnv* env = threadAttacher.GetEnv(); env->DeleteGlobalRef(g_WebView.m_WebViewJNI); g_WebView.m_WebViewJNI = NULL; - dmMutex::Delete(g_WebView.m_Mutex); + // Java callbacks already in flight can outlive finalization. Keep the mutex + // until process teardown so they can safely observe the closed queue. return dmExtension::RESULT_OK; } dmExtension::Result Platform_Finalize(dmExtension::Params* params) { + FinalizeCommands(); for( int i = 0; i < dmWebView::MAX_NUM_WEBVIEWS; ++i ) { if (g_WebView.m_Used[i]) { @@ -463,15 +508,6 @@ dmExtension::Result Platform_Finalize(dmExtension::Params* params) } } - DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); - for (uint32_t i=0; i != g_WebView.m_CmdQueue.Size(); ++i) - { - const WebViewCommand& cmd = g_WebView.m_CmdQueue[i]; - if (cmd.m_Url) { - free((void*)cmd.m_Url); - } - } - g_WebView.m_CmdQueue.SetSize(0); return dmExtension::RESULT_OK; } diff --git a/webview/src/webview_common.cpp b/webview/src/webview_common.cpp index 31ffc3e..563f7c5 100644 --- a/webview/src/webview_common.cpp +++ b/webview/src/webview_common.cpp @@ -1,6 +1,7 @@ #if defined(DM_PLATFORM_ANDROID) || defined(DM_PLATFORM_IOS) || defined (DM_PLATFORM_OSX) #include +#include #include #include "webview_common.h" @@ -8,28 +9,49 @@ namespace dmWebView { +static uint32_t g_CallbackInvocationDepth = 0; +static dmArray g_DeferredCallbacks; +// Only accessed on the script thread. Preserve this across extension restarts. +static uint64_t g_NextGeneration = 0; + +void CreateWebViewInfo(WebViewInfo* info, lua_State* L, int callback_index) +{ + info->m_Callback = dmScript::CreateCallback(L, callback_index); + if (++g_NextGeneration == 0) + ++g_NextGeneration; + info->m_Generation = g_NextGeneration; +} + +static void FlushDeferredCallbacks() +{ + if (g_CallbackInvocationDepth != 0) + return; + + for (uint32_t i = 0; i != g_DeferredCallbacks.Size(); ++i) + dmScript::DestroyCallback(g_DeferredCallbacks[i]); + g_DeferredCallbacks.SetSize(0); +} + void RunCallback(CallbackInfo* cbinfo) { - if (cbinfo->m_Info->m_Callback == LUA_NOREF) + if (cbinfo->m_Generation != cbinfo->m_Info->m_Generation) + return; + + dmScript::LuaCallbackInfo* callback = cbinfo->m_Info->m_Callback; + if (!dmScript::IsCallbackValid(callback)) { - dmLogError("No callback set"); + if (callback && cbinfo->m_Type == CALLBACK_RESULT_URL_LOADING) + Platform_CancelOpen(0, cbinfo->m_WebViewID, cbinfo->m_RequestID, cbinfo->m_Url); + return; } - lua_State* L = cbinfo->m_Info->m_L; + lua_State* L = dmScript::GetCallbackLuaContext(callback); int top = lua_gettop(L); - lua_rawgeti(L, LUA_REGISTRYINDEX, cbinfo->m_Info->m_Callback); - // Setup self - lua_rawgeti(L, LUA_REGISTRYINDEX, cbinfo->m_Info->m_Self); - lua_pushvalue(L, -1); - - dmScript::SetInstance(L); - - if (!dmScript::IsInstanceValid(L)) + if (!dmScript::SetupCallback(callback)) { - dmLogError("Could not run WebView callback because the instance has been deleted."); - lua_pop(L, 2); - assert(top == lua_gettop(L)); + if (cbinfo->m_Type == CALLBACK_RESULT_URL_LOADING) + Platform_CancelOpen(L, cbinfo->m_WebViewID, cbinfo->m_RequestID, cbinfo->m_Url); return; } @@ -56,35 +78,48 @@ void RunCallback(CallbackInfo* cbinfo) lua_rawset(L, -3); - int ret = lua_pcall(L, 5, 1, 0); - if (ret != 0) { - dmLogError("Error running WebView callback: %s", lua_tostring(L,-1)); - lua_pop(L, 1); - } else { - if (cbinfo->m_Type == dmWebView::CALLBACK_RESULT_URL_LOADING) { - // returning either nothing (nil) or a boolean true should continue - // to load the url - if (lua_isnil(L, 1) || lua_toboolean(L, 1) == 1) { - Platform_ContinueOpen(L, cbinfo->m_WebViewID, cbinfo->m_RequestID, cbinfo->m_Url); - } - else { - Platform_CancelOpen(L, cbinfo->m_WebViewID, cbinfo->m_RequestID, cbinfo->m_Url); - } - } + // Lua may destroy this view (and create another in the same slot). Keep the + // original callback rooted until its stack and script context are restored. + ++g_CallbackInvocationDepth; + int ret = dmScript::PCall(L, 5, 1); + bool allow_navigation = false; + if (ret == 0) + { + allow_navigation = lua_isnil(L, -1) || lua_toboolean(L, -1); lua_pop(L, 1); } + dmScript::TeardownCallback(callback); assert(top == lua_gettop(L)); + + if (cbinfo->m_Type == CALLBACK_RESULT_URL_LOADING && + cbinfo->m_Generation == cbinfo->m_Info->m_Generation && + cbinfo->m_Info->m_Callback == callback && dmScript::IsCallbackValid(callback)) + { + if (ret == 0 && allow_navigation) + Platform_ContinueOpen(L, cbinfo->m_WebViewID, cbinfo->m_RequestID, cbinfo->m_Url); + else + Platform_CancelOpen(L, cbinfo->m_WebViewID, cbinfo->m_RequestID, cbinfo->m_Url); + } + + --g_CallbackInvocationDepth; + FlushDeferredCallbacks(); } void ClearWebViewInfo(WebViewInfo* info) { - if( info->m_Callback != LUA_NOREF ) - dmScript::Unref(info->m_L, LUA_REGISTRYINDEX, info->m_Callback); - if( info->m_Self != LUA_NOREF ) - dmScript::Unref(info->m_L, LUA_REGISTRYINDEX, info->m_Self); - info->m_L = 0; - info->m_Callback = LUA_NOREF; - info->m_Self = LUA_NOREF; + dmScript::LuaCallbackInfo* callback = info->m_Callback; + info->m_Callback = 0; + if (!callback) + return; + + if (g_CallbackInvocationDepth == 0) + { + dmScript::DestroyCallback(callback); + return; + } + if (g_DeferredCallbacks.Full()) + g_DeferredCallbacks.OffsetCapacity(4); + g_DeferredCallbacks.Push(callback); } /** Creates a web view @@ -95,24 +130,12 @@ static int Create(lua_State* L) { int top = lua_gettop(L); - int index = 1; - if (lua_type(L, 1) == LUA_TTABLE) - { - index = index +1; - } - luaL_checktype(L, 1, LUA_TFUNCTION); - - - luaL_checktype(L, 1, LUA_TFUNCTION); - lua_pushvalue(L, 1); - WebViewInfo info; - info.m_Callback = dmScript::Ref(L, LUA_REGISTRYINDEX); - dmScript::GetInstance(L); - info.m_Self = dmScript::Ref(L, LUA_REGISTRYINDEX); - info.m_L = dmScript::GetMainThread(L); + CreateWebViewInfo(&info, L, 1); - int webview_id = Platform_Create(L, &info); + int webview_id = info.m_Callback ? Platform_Create(L, &info) : -1; + if (webview_id < 0) + ClearWebViewInfo(&info); lua_pushnumber(L, webview_id); assert(top + 1 == lua_gettop(L)); diff --git a/webview/src/webview_common.h b/webview/src/webview_common.h index 65299fd..9c65792 100644 --- a/webview/src/webview_common.h +++ b/webview/src/webview_common.h @@ -20,11 +20,10 @@ enum CallbackResult struct WebViewInfo { - lua_State* m_L; - int m_Self; - int m_Callback; + dmScript::LuaCallbackInfo* m_Callback; + uint64_t m_Generation; - WebViewInfo() : m_L(0), m_Self(LUA_NOREF), m_Callback(LUA_NOREF) {} + WebViewInfo() : m_Callback(0), m_Generation(0) {} }; struct RequestInfo @@ -38,6 +37,7 @@ struct RequestInfo struct CallbackInfo { WebViewInfo* m_Info; + uint64_t m_Generation; int m_WebViewID; int m_RequestID; CallbackResult m_Type; @@ -46,6 +46,7 @@ struct CallbackInfo CallbackInfo() : m_Info(0) + , m_Generation(0) , m_WebViewID(0) , m_RequestID(0) , m_Type(CALLBACK_RESULT_URL_OK) @@ -56,6 +57,7 @@ struct CallbackInfo void RunCallback(CallbackInfo* cbinfo); +void CreateWebViewInfo(WebViewInfo* info, lua_State* L, int callback_index); void ClearWebViewInfo(WebViewInfo* info); int Platform_Create(lua_State* L, dmWebView::WebViewInfo* info); diff --git a/webview/src/webview_darwin.mm b/webview/src/webview_darwin.mm index d031377..259687a 100644 --- a/webview/src/webview_darwin.mm +++ b/webview/src/webview_darwin.mm @@ -30,6 +30,7 @@ memset(this, 0, sizeof(*this)); } CommandType m_Type; + uint64_t m_Generation; int m_WebViewID; int m_RequestID; void* m_Data; @@ -42,17 +43,19 @@ @interface WebViewDelegate : NSViewController #endif { @public int m_WebViewID; + @public uint64_t m_Generation; @public int m_RequestID; @public NSString *m_PendingUrl; @public bool m_HeadersSet; @public NSMutableDictionary* m_Headers; @public void (^m_DecisionHandler)(WKNavigationActionPolicy); } +- (void)resolveNavigation:(WKNavigationActionPolicy)policy; @end struct WebViewExtensionState { - WebViewExtensionState() + WebViewExtensionState() : m_Mutex(0), m_AcceptCommands(false) { Clear(); } @@ -72,24 +75,76 @@ void Clear() { WKWebView* m_WebViews[dmWebView::MAX_NUM_WEBVIEWS]; WebViewDelegate* m_WebViewDelegates[dmWebView::MAX_NUM_WEBVIEWS]; dmMutex::HMutex m_Mutex; + bool m_AcceptCommands; dmArray m_CmdQueue; }; WebViewExtensionState g_WebView; +static bool IsWebViewCurrent(int webview_id, uint64_t generation) +{ + return webview_id >= 0 && webview_id < dmWebView::MAX_NUM_WEBVIEWS && + g_WebView.m_Info[webview_id].m_Callback && + g_WebView.m_Info[webview_id].m_Generation == generation; +} + +static void FreeCommand(const Command& cmd) +{ + free((void*)cmd.m_Url); + free(cmd.m_Data); +} @implementation WebViewDelegate +- (BOOL)isCurrentView:(WKWebView*)view +{ + return IsWebViewCurrent(m_WebViewID, m_Generation) && + g_WebView.m_WebViews[m_WebViewID] == view; +} + +- (void)resolveNavigation:(WKNavigationActionPolicy)policy +{ + void (^handler)(WKNavigationActionPolicy) = m_DecisionHandler; + m_DecisionHandler = nil; + [m_PendingUrl release]; + m_PendingUrl = nil; + m_HeadersSet = false; + if (handler) + { + handler(policy); + [handler release]; + } +} + +- (void)dealloc +{ + [m_DecisionHandler release]; + [m_PendingUrl release]; + [m_Headers release]; + [super dealloc]; +} + - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler { + // Lua can destroy this delegate's view during any callback below. + [[self retain] autorelease]; + if (![self isCurrentView:webView]) + { + decisionHandler(WKNavigationActionPolicyCancel); + return; + } // cancel existing decision handler // if this happens it means that we have navigated to a new page // before the previous callback completed // if we don't make a decision we'll get an error if (m_DecisionHandler) { - m_DecisionHandler(WKNavigationActionPolicyCancel); - m_DecisionHandler = NULL; + [self resolveNavigation:WKNavigationActionPolicyCancel]; + } + if (![self isCurrentView:webView]) + { + decisionHandler(WKNavigationActionPolicyCancel); + return; } // if headers have not been set we first copy the request @@ -110,18 +165,20 @@ - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigati m_HeadersSet = true; // load the new request - WKWebView* webview = g_WebView.m_WebViews[m_WebViewID]; - [webview loadRequest:newRequest]; + if ([self isCurrentView:webView]) + [webView loadRequest:newRequest]; + [newRequest release]; } else { // headers have been set and we proceed to ask the user if // the navigation should be allowed or not NSString *url = navigationAction.request.URL.absoluteString; - m_PendingUrl = url; - m_DecisionHandler = decisionHandler; + m_PendingUrl = [url copy]; + m_DecisionHandler = [decisionHandler copy]; dmWebView::CallbackInfo cbinfo; + cbinfo.m_Generation = m_Generation; cbinfo.m_Info = &g_WebView.m_Info[m_WebViewID]; cbinfo.m_WebViewID = m_WebViewID; cbinfo.m_RequestID = m_RequestID; @@ -134,7 +191,11 @@ - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigati - (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation { + if (![self isCurrentView:webView]) + return; + [[self retain] autorelease]; dmWebView::CallbackInfo cbinfo; + cbinfo.m_Generation = m_Generation; cbinfo.m_Info = &g_WebView.m_Info[m_WebViewID]; cbinfo.m_WebViewID = m_WebViewID; cbinfo.m_RequestID = m_RequestID; @@ -146,7 +207,11 @@ - (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigat - (void)webView:(WKWebView *)webView didFailNavigation:(WKNavigation *)navigation withError:(NSError *)error { + if (![self isCurrentView:webView]) + return; + [[self retain] autorelease]; dmWebView::CallbackInfo cbinfo; + cbinfo.m_Generation = m_Generation; cbinfo.m_Info = &g_WebView.m_Info[m_WebViewID]; cbinfo.m_WebViewID = m_WebViewID; cbinfo.m_RequestID = m_RequestID; @@ -168,7 +233,17 @@ - (void)webView:(WKWebView *)webView didFailNavigation:(WKNavigation *)navigatio static void QueueCommand(Command* cmd) { + if (!g_WebView.m_Mutex) + { + FreeCommand(*cmd); + return; + } DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + if (!g_WebView.m_AcceptCommands) + { + FreeCommand(*cmd); + return; + } if (g_WebView.m_CmdQueue.Full()) { g_WebView.m_CmdQueue.OffsetCapacity(8); @@ -176,6 +251,20 @@ static void QueueCommand(Command* cmd) g_WebView.m_CmdQueue.Push(*cmd); } +static void FinalizeCommands() +{ + if (!g_WebView.m_Mutex) + return; + + dmArray pending; + { + DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + g_WebView.m_AcceptCommands = false; + pending.Swap(g_WebView.m_CmdQueue); + } + for (uint32_t i = 0; i != pending.Size(); ++i) + FreeCommand(pending[i]); +} namespace dmWebView { @@ -185,7 +274,7 @@ int Platform_Create(lua_State* L, dmWebView::WebViewInfo* _info) int webview_id = -1; for( int i = 0; i < dmWebView::MAX_NUM_WEBVIEWS; ++i ) { - if( g_WebView.m_Info[i].m_L == 0 ) + if( g_WebView.m_Info[i].m_Callback == 0 ) { webview_id = i; break; @@ -210,13 +299,15 @@ int Platform_Create(lua_State* L, dmWebView::WebViewInfo* _info) WKWebViewConfiguration *configuration = [[WKWebViewConfiguration alloc] init]; WKWebView *view = [[WKWebView alloc] initWithFrame:gameFrame configuration:configuration]; #endif - WebViewDelegate* navigationDelegate = [WebViewDelegate alloc]; + WebViewDelegate* navigationDelegate = [[WebViewDelegate alloc] init]; navigationDelegate->m_WebViewID = webview_id; + navigationDelegate->m_Generation = _info->m_Generation; navigationDelegate->m_RequestID = 0; navigationDelegate->m_PendingUrl = NULL; navigationDelegate->m_DecisionHandler = NULL; navigationDelegate->m_Headers = [[NSMutableDictionary alloc] init]; view.navigationDelegate = navigationDelegate; + [configuration release]; g_WebView.m_WebViews[webview_id] = view; g_WebView.m_WebViewDelegates[webview_id] = navigationDelegate; @@ -231,12 +322,18 @@ int Platform_Create(lua_State* L, dmWebView::WebViewInfo* _info) return webview_id; } -#define CHECK_WEBVIEW_AND_RETURN() if( webview_id >= dmWebView::MAX_NUM_WEBVIEWS || webview_id < 0 ) { dmLogError("%s: Invalid webview_id: %d", __FUNCTION__, webview_id); return -1; } +#define CHECK_WEBVIEW_AND_RETURN() if( webview_id >= dmWebView::MAX_NUM_WEBVIEWS || webview_id < 0 || !g_WebView.m_Info[webview_id].m_Callback ) { dmLogError("%s: Invalid webview_id: %d", __FUNCTION__, webview_id); return -1; } static void DestroyWebView(int webview_id) { ClearWebViewInfo(&g_WebView.m_Info[webview_id]); + WebViewDelegate* delegate = g_WebView.m_WebViewDelegates[webview_id]; WKWebView *view = g_WebView.m_WebViews[webview_id]; + g_WebView.m_WebViews[webview_id] = nil; + g_WebView.m_WebViewDelegates[webview_id] = nil; + view.navigationDelegate = nil; + [delegate resolveNavigation:WKNavigationActionPolicyCancel]; + [view stopLoading]; #if defined(DM_PLATFORM_OSX) NSWindow *window = dmGraphics::GetNativeOSXNSWindow(); if ([window firstResponder] == view) { @@ -245,7 +342,7 @@ static void DestroyWebView(int webview_id) #endif [view removeFromSuperview]; [view release]; - g_WebView.m_WebViews[webview_id] = NULL; + [delegate release]; } int Platform_ClearHeaders(lua_State* L, int webview_id) @@ -298,24 +395,26 @@ int Platform_SetTransparent(lua_State* L, int webview_id, int transparent) int Platform_Open(lua_State* L, int webview_id, const char* url, dmWebView::RequestInfo* options) { CHECK_WEBVIEW_AND_RETURN(); + int request_id = ++g_WebView.m_WebViewDelegates[webview_id]->m_RequestID; Platform_SetVisibleInternal(g_WebView.m_WebViews[webview_id], !options->m_Hidden); Platform_SetTransparentInternal(g_WebView.m_WebViews[webview_id], options->m_Transparent); NSURL* ns_url = [NSURL URLWithString: [NSString stringWithUTF8String: url]]; NSURLRequest* request = [NSURLRequest requestWithURL: ns_url]; [g_WebView.m_WebViews[webview_id] loadRequest:request]; - return ++g_WebView.m_WebViewDelegates[webview_id]->m_RequestID; + return request_id; } int Platform_OpenRaw(lua_State* L, int webview_id, const char* html, dmWebView::RequestInfo* options) { CHECK_WEBVIEW_AND_RETURN(); + int request_id = ++g_WebView.m_WebViewDelegates[webview_id]->m_RequestID; Platform_SetVisibleInternal(g_WebView.m_WebViews[webview_id], !options->m_Hidden); Platform_SetTransparentInternal(g_WebView.m_WebViews[webview_id], options->m_Transparent); NSString* ns_html = [NSString stringWithUTF8String: html]; [g_WebView.m_WebViews[webview_id] loadHTMLString:ns_html baseURL:nil]; - return ++g_WebView.m_WebViewDelegates[webview_id]->m_RequestID; + return request_id; } int Platform_ContinueOpen(lua_State* L, int webview_id, int request_id, const char* url) @@ -323,9 +422,7 @@ int Platform_ContinueOpen(lua_State* L, int webview_id, int request_id, const ch CHECK_WEBVIEW_AND_RETURN(); WebViewDelegate* delegate = g_WebView.m_WebViewDelegates[webview_id]; if ([delegate->m_PendingUrl isEqualToString:[NSString stringWithUTF8String: url]]) { - delegate->m_DecisionHandler(WKNavigationActionPolicyAllow); - delegate->m_DecisionHandler = NULL; - delegate->m_HeadersSet = false; + [delegate resolveNavigation:WKNavigationActionPolicyAllow]; } return request_id; } @@ -335,9 +432,7 @@ int Platform_CancelOpen(lua_State* L, int webview_id, int request_id, const char CHECK_WEBVIEW_AND_RETURN(); WebViewDelegate* delegate = g_WebView.m_WebViewDelegates[webview_id]; if ([delegate->m_PendingUrl isEqualToString:[NSString stringWithUTF8String: url]]) { - delegate->m_DecisionHandler(WKNavigationActionPolicyCancel); - delegate->m_DecisionHandler = NULL; - delegate->m_HeadersSet = false; + [delegate resolveNavigation:WKNavigationActionPolicyCancel]; } return request_id; } @@ -346,6 +441,7 @@ int Platform_Eval(lua_State* L, int webview_id, const char* code) { CHECK_WEBVIEW_AND_RETURN(); int request_id = ++g_WebView.m_WebViewDelegates[webview_id]->m_RequestID; + const uint64_t generation = g_WebView.m_Info[webview_id].m_Generation; [g_WebView.m_WebViews[webview_id] evaluateJavaScript:[NSString stringWithUTF8String: code] completionHandler:^(NSObject *resultObject, NSError *error) { @@ -353,9 +449,10 @@ int Platform_Eval(lua_State* L, int webview_id, const char* code) Command cmd; cmd.m_Type = (result != nil) ? CMD_EVAL_OK : CMD_EVAL_ERROR; cmd.m_WebViewID = webview_id; + cmd.m_Generation = generation; cmd.m_RequestID = request_id; cmd.m_Url = 0; - cmd.m_Data = (void*) ((result != nil) ? CopyString(result) : "Error string unavailable on iOS"); + cmd.m_Data = (void*) ((result != nil) ? CopyString(result) : strdup("Error string unavailable on iOS")); QueueCommand(&cmd); }]; @@ -402,26 +499,34 @@ int Platform_SetPosition(lua_State* L, int webview_id, int x, int y, int width, dmExtension::Result Platform_AppInitialize(dmExtension::AppParams* params) { - g_WebView.Clear(); - g_WebView.m_Mutex = dmMutex::New(); - g_WebView.m_CmdQueue.SetCapacity(8); + if (!g_WebView.m_Mutex) + g_WebView.m_Mutex = dmMutex::New(); + { + DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + g_WebView.m_CmdQueue.SetCapacity(8); + } return dmExtension::RESULT_OK; } dmExtension::Result Platform_AppFinalize(dmExtension::AppParams* params) { - dmMutex::Delete(g_WebView.m_Mutex); + FinalizeCommands(); + // WebKit completions can outlive finalization. Keep the mutex alive so late + // producers can safely observe the closed queue. return dmExtension::RESULT_OK; } dmExtension::Result Platform_Initialize(dmExtension::Params* params) { + DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + g_WebView.m_AcceptCommands = true; return dmExtension::RESULT_OK; } dmExtension::Result Platform_Finalize(dmExtension::Params* params) { + FinalizeCommands(); for( int i = 0; i < dmWebView::MAX_NUM_WEBVIEWS; ++i ) { if (g_WebView.m_WebViews[i]) { @@ -429,36 +534,30 @@ int Platform_SetPosition(lua_State* L, int webview_id, int x, int y, int width, } } - DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); - for (uint32_t i=0; i != g_WebView.m_CmdQueue.Size(); ++i) - { - const Command& cmd = g_WebView.m_CmdQueue[i]; - if (cmd.m_Url) { - free((void*)cmd.m_Url); - } - } - g_WebView.m_CmdQueue.SetSize(0); return dmExtension::RESULT_OK; } dmExtension::Result Platform_Update(dmExtension::Params* params) { - if (g_WebView.m_CmdQueue.Empty()) - { - return dmExtension::RESULT_OK; // avoid a lock (~300us on iPhone 4s) - } - dmArray tmp; { DM_MUTEX_SCOPED_LOCK(g_WebView.m_Mutex); + if (!g_WebView.m_AcceptCommands || g_WebView.m_CmdQueue.Empty()) + return dmExtension::RESULT_OK; tmp.Swap(g_WebView.m_CmdQueue); } for (uint32_t i=0; i != tmp.Size(); ++i) { const Command& cmd = tmp[i]; + if (!IsWebViewCurrent(cmd.m_WebViewID, cmd.m_Generation)) + { + FreeCommand(cmd); + continue; + } dmWebView::CallbackInfo cbinfo; + cbinfo.m_Generation = cmd.m_Generation; switch (cmd.m_Type) { case CMD_EVAL_OK: @@ -484,12 +583,7 @@ int Platform_SetPosition(lua_State* L, int webview_id, int x, int y, int width, default: assert(false); } - if (cmd.m_Url) { - free((void*)cmd.m_Url); - } - if (cmd.m_Data) { - free(cmd.m_Data); - } + FreeCommand(cmd); } return dmExtension::RESULT_OK; }