diff --git a/flutter_inappwebview_windows/windows/CMakeLists.txt b/flutter_inappwebview_windows/windows/CMakeLists.txt index a5cee625e2..444488d7c2 100644 --- a/flutter_inappwebview_windows/windows/CMakeLists.txt +++ b/flutter_inappwebview_windows/windows/CMakeLists.txt @@ -362,7 +362,7 @@ add_executable(${TEST_RUNNER} ) apply_standard_settings(${TEST_RUNNER}) target_include_directories(${TEST_RUNNER} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}") -target_link_libraries(${TEST_RUNNER} PRIVATE gtest_main) +target_link_libraries(${TEST_RUNNER} PRIVATE flutter flutter_wrapper_plugin gtest_main) # Enable automatic test discovery. include(GoogleTest) diff --git a/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.cpp b/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.cpp index 6cae4a9b4f..53b990f925 100644 --- a/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.cpp +++ b/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.cpp @@ -489,6 +489,7 @@ namespace flutter_inappwebview_plugin ); auto callback = std::make_unique(); + callback->owner = aliveToken(); callback->nonNullSuccess = [this, allowRequest, cancelRequest](const NavigationActionPolicy actionPolicy) { if (actionPolicy == NavigationActionPolicy::allow) { @@ -843,6 +844,7 @@ namespace flutter_inappwebview_plugin std::move(windowFeatures)); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, windowId, urlRequest, deferral, args](const std::optional handledByClient) { if (plugin && plugin->inAppWebViewManager && map_contains(plugin->inAppWebViewManager->windowWebViews, windowId)) { @@ -892,6 +894,7 @@ namespace flutter_inappwebview_plugin failedLog(args->get_PermissionKind(&resource)); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> permissionResponse) { failedLog(args->put_State(COREWEBVIEW2_PERMISSION_STATE_DENY)); @@ -959,6 +962,7 @@ namespace flutter_inappwebview_plugin { if (channelDelegate) { auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> response) { failedLog(deferral->Complete()); @@ -984,6 +988,7 @@ namespace flutter_inappwebview_plugin if (settings->useShouldInterceptRequest) { auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> response) { failedLog(deferral->Complete()); @@ -1164,6 +1169,7 @@ namespace flutter_inappwebview_plugin ); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> response) { failedLog(deferral->Complete()); @@ -1271,6 +1277,7 @@ namespace flutter_inappwebview_plugin ); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> response) { failedLog(deferral->Complete()); @@ -1362,6 +1369,7 @@ namespace flutter_inappwebview_plugin ); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> response) { failedLog(deferral->Complete()); @@ -1449,6 +1457,7 @@ namespace flutter_inappwebview_plugin ); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [this, deferral, args](const std::optional> response) { failedLog(deferral->Complete()); @@ -1565,6 +1574,7 @@ namespace flutter_inappwebview_plugin static_cast(isUserInitiated)); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [deferral, args](const std::optional> response) { failedLog(args->put_Cancel(FALSE)); @@ -1684,6 +1694,7 @@ namespace flutter_inappwebview_plugin auto request = std::make_shared(senderOriginValue, notificationControllerId, notificationPtr); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [deferral, args, notificationController](const std::optional> response) { failedLog(args->put_Handled(FALSE)); @@ -1750,6 +1761,7 @@ namespace flutter_inappwebview_plugin SaveAsKindFromOptionalInteger(std::optional{ static_cast(kind) })); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [deferral](const std::optional> response) { if (deferral) { @@ -1826,6 +1838,7 @@ namespace flutter_inappwebview_plugin static_cast(suppressDefaultPolicy)); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [deferral](const std::optional> response) { if (deferral) { @@ -1893,6 +1906,7 @@ namespace flutter_inappwebview_plugin static_cast(handled)); auto callback = std::make_unique(); + callback->owner = aliveToken(); auto defaultBehaviour = [deferral](const std::optional> response) { if (deferral) { @@ -4153,6 +4167,7 @@ namespace flutter_inappwebview_plugin */ auto callback = std::make_unique(); + callback->owner = aliveToken(); callback->defaultBehaviour = [this, callHandlerID](const std::optional response) { std::string json = "null"; @@ -4191,6 +4206,9 @@ namespace flutter_inappwebview_plugin InAppWebView::~InAppWebView() { debugLog("dealloc InAppWebView"); + // Expire before tearing anything down: a Dart reply arriving from here on + // must find a dead token and drop the call. + aliveToken_.reset(); WebViewDropTarget::UnregisterWebView(this); userContentController = nullptr; if (webView) { diff --git a/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.h b/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.h index 3f88445449..d2b32fd9ab 100644 --- a/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.h +++ b/flutter_inappwebview_windows/windows/in_app_webview/in_app_webview.h @@ -2,6 +2,7 @@ #define FLUTTER_INAPPWEBVIEW_PLUGIN_IN_APP_WEBVIEW_H_ #include +#include #include #include #include @@ -253,6 +254,13 @@ namespace flutter_inappwebview_plugin return pageFrameId_; } + // Lifetime token for callbacks that round-trip through Dart: they outlive + // this webview when a tab closes mid-call, and must not touch it afterwards. + std::weak_ptr aliveToken() const + { + return aliveToken_; + } + static bool isSslError(const COREWEBVIEW2_WEB_ERROR_STATUS& webErrorStatus); private: // custom_platform_view @@ -280,6 +288,7 @@ namespace flutter_inappwebview_plugin std::map> webMessageListeners_; std::map> webNotificationControllers_; std::map> printJobControllers_; + std::shared_ptr aliveToken_ = std::make_shared(0); void registerEventHandlers(); void registerSurfaceEventHandlers(); diff --git a/flutter_inappwebview_windows/windows/test/flutter_inappwebview_windows_plugin_test.cpp b/flutter_inappwebview_windows/windows/test/flutter_inappwebview_windows_plugin_test.cpp index 0c2231c1d6..507819e3a1 100644 --- a/flutter_inappwebview_windows/windows/test/flutter_inappwebview_windows_plugin_test.cpp +++ b/flutter_inappwebview_windows/windows/test/flutter_inappwebview_windows_plugin_test.cpp @@ -1,9 +1,68 @@ #include +#include + #include "in_app_webview/webview_visibility_state.h" +#include "types/base_callback_result.h" namespace flutter_inappwebview_plugin::test { +namespace { + +std::unique_ptr> makeCallback(bool* ran) { + auto callback = std::make_unique>(); + callback->decodeResult = [](const flutter::EncodableValue* value) { + return std::make_optional(std::get(*value)); + }; + callback->defaultBehaviour = [ran](const std::optional) { + *ran = true; + }; + callback->error = [ran](const std::string&, const std::string&, + const flutter::EncodableValue*) { *ran = true; }; + return callback; +} + +} // namespace + +TEST(BaseCallbackResult, RunsHandlersWithoutOwner) { + auto ran = false; + auto callback = makeCallback(&ran); + callback->Success(flutter::EncodableValue(true)); + EXPECT_TRUE(ran); +} + +TEST(BaseCallbackResult, RunsHandlersWhileOwnerIsAlive) { + auto ran = false; + auto owner = std::make_shared(0); + auto callback = makeCallback(&ran); + callback->owner = owner; + callback->Success(flutter::EncodableValue(true)); + EXPECT_TRUE(ran); +} + +TEST(BaseCallbackResult, DropsHandlersOnceOwnerIsGone) { + auto ran = false; + auto owner = std::make_shared(0); + auto callback = makeCallback(&ran); + callback->owner = owner; + owner.reset(); + callback->Success(flutter::EncodableValue(true)); + EXPECT_FALSE(ran); +} + +TEST(BaseCallbackResult, DropsErrorAndNotImplementedOnceOwnerIsGone) { + auto ran = false; + auto owner = std::make_shared(0); + auto errorCallback = makeCallback(&ran); + errorCallback->owner = owner; + auto notImplementedCallback = makeCallback(&ran); + notImplementedCallback->owner = owner; + owner.reset(); + errorCallback->Error("code", "message"); + notImplementedCallback->NotImplemented(); + EXPECT_FALSE(ran); +} + TEST(WebViewVisibilityState, StartsVisible) { WebViewVisibilityState state; EXPECT_TRUE(state.shouldBeVisible()); diff --git a/flutter_inappwebview_windows/windows/types/base_callback_result.h b/flutter_inappwebview_windows/windows/types/base_callback_result.h index d156e1e69c..b9c8527564 100644 --- a/flutter_inappwebview_windows/windows/types/base_callback_result.h +++ b/flutter_inappwebview_windows/windows/types/base_callback_result.h @@ -3,8 +3,11 @@ #include #include +#include #include +#include "../utils/log.h" + namespace flutter_inappwebview_plugin { template @@ -18,10 +21,27 @@ namespace flutter_inappwebview_plugin std::function result)> defaultBehaviour = [](const std::optional result) {}; std::function(const flutter::EncodableValue* result)> decodeResult = [](const flutter::EncodableValue* result) { return std::nullopt; }; + // Owner of the objects captured by the handlers above. Dart may reply long + // after that owner is gone (a tab closed while an RPC was in flight), so an + // expired token makes every handler a no-op instead of a use-after-free. + std::optional> owner; + + bool isOwnerGone() const + { + if (!owner.has_value() || !owner.value().expired()) { + return false; + } + debugLog("dropping a Dart reply that outlived its owner"); + return true; + } + BaseCallbackResult() : MethodResultFunctions( [this](const flutter::EncodableValue* val) { + if (isOwnerGone()) { + return; + } std::optional result = decodeResult ? decodeResult(val) : std::nullopt; auto shouldRunDefaultBehaviour = false; if (result.has_value()) { @@ -36,12 +56,18 @@ namespace flutter_inappwebview_plugin }, [this](const std::string& error_code, const std::string& error_message, const flutter::EncodableValue* error_details) { + if (isOwnerGone()) { + return; + } if (error) { error(error_code, error_message, error_details); } }, [this]() { + if (isOwnerGone()) { + return; + } if (defaultBehaviour) { defaultBehaviour(std::nullopt); }