diff --git a/packages/webview_flutter/CHANGELOG.md b/packages/webview_flutter/CHANGELOG.md index aa5a97dde..d4ccd8037 100644 --- a/packages/webview_flutter/CHANGELOG.md +++ b/packages/webview_flutter/CHANGELOG.md @@ -1,3 +1,13 @@ +## 0.10.1 + +* Implement `clearLocalStorage`. +* Implement `onHttpError` for the navigation delegate. +* Fix races and use-after-frees on WebView disposal, including a buffer-pool + use-after-free on the raster thread. +* Replace the Ecore main loop API with GLib. +* Drain all pending WebView teardowns before ewk_shutdown(), fixing a native + crash on app exit and enabling simultaneous use of multiple WebViews. + ## 0.10.0 * Update minimum supported SDK version to Flutter 3.32/Dart 3.8. diff --git a/packages/webview_flutter/README.md b/packages/webview_flutter/README.md index 3dd1b9a1d..6dc6ef0dc 100644 --- a/packages/webview_flutter/README.md +++ b/packages/webview_flutter/README.md @@ -23,7 +23,7 @@ This package is not an _endorsed_ implementation of `webview_flutter`. Therefore ```yaml dependencies: webview_flutter: ^4.13.1 - webview_flutter_tizen: ^0.10.0 + webview_flutter_tizen: ^0.10.1 ``` ## Example diff --git a/packages/webview_flutter/example/integration_test/webview_flutter_test.dart b/packages/webview_flutter/example/integration_test/webview_flutter_test.dart index d861f9994..5c15848f6 100644 --- a/packages/webview_flutter/example/integration_test/webview_flutter_test.dart +++ b/packages/webview_flutter/example/integration_test/webview_flutter_test.dart @@ -44,12 +44,10 @@ Future main() async { final Completer pageFinished = Completer(); final WebViewController controller = WebViewController(); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), - ), + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), ); - unawaited(controller.loadRequest(Uri.parse(primaryUrl))); + await controller.loadRequest(Uri.parse(primaryUrl)); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -63,13 +61,11 @@ Future main() async { final Completer pageFinished = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), ); - unawaited(controller.loadRequest(Uri.parse(primaryUrl))); + await controller.loadRequest(Uri.parse(primaryUrl)); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -89,16 +85,14 @@ Future main() async { final StreamController pageLoads = StreamController(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (String url) => pageLoads.add(url)), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (String url) => pageLoads.add(url)), ); await tester.pumpWidget(WebViewWidget(controller: controller)); - unawaited(controller.loadRequest(Uri.parse(headersUrl), headers: headers)); + await controller.loadRequest(Uri.parse(headersUrl), headers: headers); await pageLoads.stream.firstWhere((String url) => url == headersUrl); @@ -113,11 +107,9 @@ Future main() async { testWidgets('JavascriptChannel', (WidgetTester tester) async { final Completer pageFinished = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), ); final Completer channelCompleter = Completer(); @@ -178,14 +170,12 @@ Future main() async { final Completer pageFinished = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageFinished.complete()), ); - unawaited(controller.setUserAgent('Custom_User_Agent1')); - unawaited(controller.loadRequest(Uri.parse('about:blank'))); + await controller.setUserAgent('Custom_User_Agent1'); + await controller.loadRequest(Uri.parse('about:blank')); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -210,16 +200,12 @@ Future main() async { final Completer pageLoaded = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()), ); - unawaited( - controller.loadRequest( - Uri.parse('data:text/html;charset=utf-8;base64,$getTitleTestBase64'), - ), + await controller.loadRequest( + Uri.parse('data:text/html;charset=utf-8;base64,$getTitleTestBase64'), ); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -265,18 +251,12 @@ Future main() async { final Completer pageLoaded = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()), ); - unawaited( - controller.loadRequest( - Uri.parse( - 'data:text/html;charset=utf-8;base64,$scrollTestPageBase64', - ), - ), + await controller.loadRequest( + Uri.parse('data:text/html;charset=utf-8;base64,$scrollTestPageBase64'), ); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -296,14 +276,30 @@ Future main() async { expect(scrollPos.dx, isNot(X_SCROLL)); expect(scrollPos.dy, isNot(Y_SCROLL)); + // The scroll position settles asynchronously, so poll until it reaches + // the expected value (with a timeout) instead of reading it once. This + // keeps the test stable on slower software-GL rendering (e.g. emulators). + Future pollScrollPosition(int expectedX, int expectedY) async { + Offset pos = await controller.getScrollPosition(); + for ( + int i = 0; + i < 20 && (pos.dx != expectedX || pos.dy != expectedY); + i++ + ) { + await Future.delayed(const Duration(milliseconds: 100)); + pos = await controller.getScrollPosition(); + } + return pos; + } + await controller.scrollTo(X_SCROLL, Y_SCROLL); - scrollPos = await controller.getScrollPosition(); + scrollPos = await pollScrollPosition(X_SCROLL, Y_SCROLL); expect(scrollPos.dx, X_SCROLL); expect(scrollPos.dy, Y_SCROLL); // Check scrollBy() (on top of scrollTo()) await controller.scrollBy(X_SCROLL, Y_SCROLL); - scrollPos = await controller.getScrollPosition(); + scrollPos = await pollScrollPosition(X_SCROLL * 2, Y_SCROLL * 2); expect(scrollPos.dx, X_SCROLL * 2); expect(scrollPos.dy, Y_SCROLL * 2); }); @@ -319,23 +315,21 @@ Future main() async { Completer pageLoaded = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate( - onPageFinished: (_) => pageLoaded.complete(), - onNavigationRequest: (NavigationRequest navigationRequest) { - return (navigationRequest.url.contains('youtube.com')) - ? NavigationDecision.prevent - : NavigationDecision.navigate; - }, - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate( + onPageFinished: (_) => pageLoaded.complete(), + onNavigationRequest: (NavigationRequest navigationRequest) { + return (navigationRequest.url.contains('youtube.com')) + ? NavigationDecision.prevent + : NavigationDecision.navigate; + }, ), ); await tester.pumpWidget(WebViewWidget(controller: controller)); - unawaited(controller.loadRequest(Uri.parse(blankPageEncoded))); + await controller.loadRequest(Uri.parse(blankPageEncoded)); await pageLoaded.future; // Wait for initial page load. @@ -352,19 +346,15 @@ Future main() async { Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate( - onWebResourceError: (WebResourceError error) { - errorCompleter.complete(error); - }, - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate( + onWebResourceError: (WebResourceError error) { + errorCompleter.complete(error); + }, ), ); - unawaited( - controller.loadRequest(Uri.parse('https://www.notawebsite..com')), - ); + await controller.loadRequest(Uri.parse('https://www.notawebsite..com')); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -380,21 +370,17 @@ Future main() async { final Completer pageFinishCompleter = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate( - onPageFinished: (_) => pageFinishCompleter.complete(), - onWebResourceError: (WebResourceError error) { - errorCompleter.complete(error); - }, - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate( + onPageFinished: (_) => pageFinishCompleter.complete(), + onWebResourceError: (WebResourceError error) { + errorCompleter.complete(error); + }, ), ); - unawaited( - controller.loadRequest( - Uri.parse('data:text/html;charset=utf-8;base64,PCFET0NUWVBFIGh0bWw+'), - ), + await controller.loadRequest( + Uri.parse('data:text/html;charset=utf-8;base64,PCFET0NUWVBFIGh0bWw+'), ); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -407,23 +393,21 @@ Future main() async { Completer pageLoaded = Completer(); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate( - onPageFinished: (_) => pageLoaded.complete(), - onNavigationRequest: (NavigationRequest navigationRequest) { - return (navigationRequest.url.contains('youtube.com')) - ? NavigationDecision.prevent - : NavigationDecision.navigate; - }, - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate( + onPageFinished: (_) => pageLoaded.complete(), + onNavigationRequest: (NavigationRequest navigationRequest) { + return (navigationRequest.url.contains('youtube.com')) + ? NavigationDecision.prevent + : NavigationDecision.navigate; + }, ), ); await tester.pumpWidget(WebViewWidget(controller: controller)); - unawaited(controller.loadRequest(Uri.parse(blankPageEncoded))); + await controller.loadRequest(Uri.parse(blankPageEncoded)); await pageLoaded.future; // Wait for initial page load. @@ -443,30 +427,90 @@ Future main() async { expect(currentUrl, isNot(contains('youtube.com'))); }); - testWidgets('supports asynchronous decisions', (WidgetTester tester) async { - Completer pageLoaded = Completer(); + testWidgets('onHttpError', (WidgetTester tester) async { + final Completer errorCompleter = + Completer(); + + final WebViewController controller = WebViewController(); + unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); + unawaited( + controller.setNavigationDelegate( + NavigationDelegate( + onHttpError: (HttpResponseError error) { + errorCompleter.complete(error); + }, + ), + ), + ); + + unawaited(controller.loadRequest(Uri.parse('$prefixUrl/favicon.ico'))); + + await tester.pumpWidget(WebViewWidget(controller: controller)); + + final HttpResponseError error = await errorCompleter.future; + + expect(error, isNotNull); + expect(error.response?.statusCode, 404); + }); + + testWidgets('onHttpError is not called when no HTTP error is received', ( + WidgetTester tester, + ) async { + const String testPage = ''' + + + + + + '''; + + final Completer errorCompleter = + Completer(); + final Completer pageFinishCompleter = Completer(); final WebViewController controller = WebViewController(); unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); unawaited( controller.setNavigationDelegate( NavigationDelegate( - onPageFinished: (_) => pageLoaded.complete(), - onNavigationRequest: (NavigationRequest navigationRequest) async { - NavigationDecision decision = NavigationDecision.prevent; - decision = await Future.delayed( - const Duration(milliseconds: 10), - () => NavigationDecision.navigate, - ); - return decision; + onPageFinished: (_) => pageFinishCompleter.complete(), + onHttpError: (HttpResponseError error) { + errorCompleter.complete(error); }, ), ), ); + unawaited(controller.loadHtmlString(testPage)); + await tester.pumpWidget(WebViewWidget(controller: controller)); - unawaited(controller.loadRequest(Uri.parse(blankPageEncoded))); + expect(errorCompleter.future, doesNotComplete); + await pageFinishCompleter.future; + }); + + testWidgets('supports asynchronous decisions', (WidgetTester tester) async { + Completer pageLoaded = Completer(); + + final WebViewController controller = WebViewController(); + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate( + onPageFinished: (_) => pageLoaded.complete(), + onNavigationRequest: (NavigationRequest navigationRequest) async { + NavigationDecision decision = NavigationDecision.prevent; + decision = await Future.delayed( + const Duration(milliseconds: 10), + () => NavigationDecision.navigate, + ); + return decision; + }, + ), + ); + + await tester.pumpWidget(WebViewWidget(controller: controller)); + + await controller.loadRequest(Uri.parse(blankPageEncoded)); await pageLoaded.future; // Wait for initial page load. @@ -483,13 +527,11 @@ Future main() async { final WebViewController controller = WebViewController(); final Completer urlChangeCompleter = Completer(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited( - controller.setNavigationDelegate( - NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()), - ), + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()), ); - unawaited(controller.loadRequest(Uri.parse(blankPageEncoded))); + await controller.loadRequest(Uri.parse(blankPageEncoded)); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -517,9 +559,9 @@ Future main() async { ); final WebViewController controller = WebViewController(); - unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted)); - unawaited(controller.setNavigationDelegate(navigationDelegate)); - unawaited(controller.loadRequest(Uri.parse(primaryUrl))); + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate(navigationDelegate); + await controller.loadRequest(Uri.parse(primaryUrl)); await tester.pumpWidget(WebViewWidget(controller: controller)); @@ -541,6 +583,94 @@ Future main() async { await expectLater(urlChangeCompleter.future, completion(secondaryUrl)); }); }); + + testWidgets('clearLocalStorage', (WidgetTester tester) async { + Completer pageLoadCompleter = Completer(); + + final WebViewController controller = WebViewController(); + await controller.setJavaScriptMode(JavaScriptMode.unrestricted); + await controller.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => pageLoadCompleter.complete()), + ); + await controller.loadRequest(Uri.parse(primaryUrl)); + + await tester.pumpWidget(WebViewWidget(controller: controller)); + + await pageLoadCompleter.future; + pageLoadCompleter = Completer(); + + await controller.runJavaScript('localStorage.setItem("myCat", "Tom");'); + final String myCatItem = + await controller.runJavaScriptReturningResult( + 'localStorage.getItem("myCat");', + ) + as String; + expect(myCatItem, 'Tom'); + + await controller.clearLocalStorage(); + + // Reload page to have changes take effect. + await controller.reload(); + await pageLoadCompleter.future; + + final Object nullItem = await controller.runJavaScriptReturningResult( + 'localStorage.getItem("myCat");', + ); + expect(nullItem, ''); + }); + + // Tizen-specific: mounts and disposes two WebViews at the same time, which + // used to hit a native disposal race fixed in CHANGELOG.md's 0.10.1 entry. + testWidgets('multiple WebViews can be used simultaneously', ( + WidgetTester tester, + ) async { + final Completer firstPageFinished = Completer(); + final Completer secondPageFinished = Completer(); + + final WebViewController firstController = WebViewController(); + await firstController.setJavaScriptMode(JavaScriptMode.unrestricted); + await firstController.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => firstPageFinished.complete()), + ); + await firstController.loadRequest(Uri.parse(primaryUrl)); + + final WebViewController secondController = WebViewController(); + await secondController.setJavaScriptMode(JavaScriptMode.unrestricted); + await secondController.setNavigationDelegate( + NavigationDelegate(onPageFinished: (_) => secondPageFinished.complete()), + ); + await secondController.loadRequest(Uri.parse(secondaryUrl)); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Row( + children: [ + Expanded(child: WebViewWidget(controller: firstController)), + Expanded(child: WebViewWidget(controller: secondController)), + ], + ), + ), + ); + + await firstPageFinished.future; + await secondPageFinished.future; + + expect(await firstController.currentUrl(), primaryUrl); + expect(await secondController.currentUrl(), secondaryUrl); + + await expectLater( + firstController.runJavaScriptReturningResult('1 + 1'), + completion(2), + ); + await expectLater( + secondController.runJavaScriptReturningResult('2 + 2'), + completion(4), + ); + + // Unmount both WebViews together to exercise concurrent native teardown. + await tester.pumpWidget(Container()); + }); } class ResizableWebView extends StatefulWidget { diff --git a/packages/webview_flutter/example/lib/main.dart b/packages/webview_flutter/example/lib/main.dart index 36e248fb6..7899aa526 100644 --- a/packages/webview_flutter/example/lib/main.dart +++ b/packages/webview_flutter/example/lib/main.dart @@ -190,10 +190,9 @@ Page resource error: debugPrint('allowing navigation to ${request.url}'); return NavigationDecision.navigate; }, - // Note: onHttpError is not implemented by TizenWebview. - // onHttpError: (HttpResponseError error) { - // debugPrint('Error occurred on page: ${error.response?.statusCode}'); - // }, + onHttpError: (HttpResponseError error) { + debugPrint('Error occurred on page: ${error.response?.statusCode}'); + }, onUrlChange: (UrlChange change) { debugPrint('url change to ${change.url}'); }, @@ -491,8 +490,7 @@ class SampleMenu extends StatelessWidget { Future _onClearCache(BuildContext context) async { await webViewController.clearCache(); - // This is unimplemented in webview_flutter_tizen. - // await webViewController.clearLocalStorage(); + await webViewController.clearLocalStorage(); if (context.mounted) { ScaffoldMessenger.of( context, diff --git a/packages/webview_flutter/lib/src/tizen_webview.dart b/packages/webview_flutter/lib/src/tizen_webview.dart index 236dd4821..ca3a0758b 100644 --- a/packages/webview_flutter/lib/src/tizen_webview.dart +++ b/packages/webview_flutter/lib/src/tizen_webview.dart @@ -151,6 +151,10 @@ class TizenWebView { /// Clears all caches used by the [WebView]. Future clearCache() => _invokeChannelMethod('clearCache'); + /// Clears the local storage used by the [WebView]. + Future clearLocalStorage() => + _invokeChannelMethod('clearLocalStorage'); + /// Sets the JavaScript execution mode to be used by the webview. Future setJavaScriptMode(int javaScriptMode) => _invokeChannelMethod('javaScriptMode', javaScriptMode); diff --git a/packages/webview_flutter/lib/src/tizen_webview_controller.dart b/packages/webview_flutter/lib/src/tizen_webview_controller.dart index b364fa7a1..b6909e1bc 100644 --- a/packages/webview_flutter/lib/src/tizen_webview_controller.dart +++ b/packages/webview_flutter/lib/src/tizen_webview_controller.dart @@ -218,12 +218,7 @@ class TizenWebViewController extends PlatformWebViewController { Future clearCache() => _webview.clearCache(); @override - Future clearLocalStorage() { - throw UnimplementedError( - 'This version of `TizenWebViewController` currently has no ' - 'implementation.', - ); - } + Future clearLocalStorage() => _webview.clearLocalStorage(); @override Future setPlatformNavigationDelegate( @@ -477,6 +472,7 @@ class TizenNavigationDelegate extends PlatformNavigationDelegate { WebResourceErrorCallback? _onWebResourceError; NavigationRequestCallback? _onNavigationRequest; UrlChangeCallback? _onUrlChange; + HttpResponseErrorCallback? _onHttpError; /// Called when [TizenView] is created. void createNavigationDelegateChannel(int viewId) { @@ -525,6 +521,20 @@ class TizenNavigationDelegate extends PlatformNavigationDelegate { _onUrlChange!(UrlChange(url: arguments['url']! as String)); } return null; + case 'onHttpError': + if (_onHttpError != null) { + final Uri uri = Uri.parse(arguments['url']! as String); + _onHttpError!( + HttpResponseError( + request: WebResourceRequest(uri: uri), + response: WebResourceResponse( + uri: uri, + statusCode: arguments['statusCode']! as int, + ), + ), + ); + } + return null; } throw MissingPluginException( @@ -580,10 +590,7 @@ class TizenNavigationDelegate extends PlatformNavigationDelegate { @override Future setOnHttpError(HttpResponseErrorCallback onHttpError) async { - throw UnimplementedError( - 'This version of `TizenNavigationDelegate` currently has no ' - 'implementation for `setOnHttpError`', - ); + _onHttpError = onHttpError; } @override diff --git a/packages/webview_flutter/pubspec.yaml b/packages/webview_flutter/pubspec.yaml index fc461b335..d619ea013 100644 --- a/packages/webview_flutter/pubspec.yaml +++ b/packages/webview_flutter/pubspec.yaml @@ -2,7 +2,7 @@ name: webview_flutter_tizen description: Tizen implementation of the webview_flutter plugin. homepage: https://github.com/flutter-tizen/plugins repository: https://github.com/flutter-tizen/plugins/tree/master/packages/webview_flutter -version: 0.10.0 +version: 0.10.1 environment: sdk: ^3.8.0 diff --git a/packages/webview_flutter/tizen/src/buffer_pool.cc b/packages/webview_flutter/tizen/src/buffer_pool.cc index ed8fde021..28a2dd82f 100644 --- a/packages/webview_flutter/tizen/src/buffer_pool.cc +++ b/packages/webview_flutter/tizen/src/buffer_pool.cc @@ -4,11 +4,31 @@ #include "buffer_pool.h" +#include +#include + #include "log.h" -BufferUnit::BufferUnit(int32_t width, int32_t height) { Reset(width, height); } +namespace { +// Tracks live BufferUnits so the engine's release_callback (below) can detect +// one that was already destroyed instead of dereferencing freed memory. +std::set active_buffers; +std::mutex active_buffers_mutex; +} // namespace + +BufferUnit::BufferUnit(int32_t width, int32_t height) { + { + std::lock_guard lock(active_buffers_mutex); + active_buffers.insert(this); + } + Reset(width, height); +} BufferUnit::~BufferUnit() { + { + std::lock_guard lock(active_buffers_mutex); + active_buffers.erase(this); + } if (tbm_surface_ && !use_external_buffer_) { tbm_surface_destroy(tbm_surface_); tbm_surface_ = nullptr; @@ -76,7 +96,10 @@ void BufferUnit::Reset(int32_t width, int32_t height) { gpu_surface_->handle = tbm_surface_; gpu_surface_->release_callback = [](void* release_context) { BufferUnit* buffer = reinterpret_cast(release_context); - buffer->UnmarkInUse(); + std::lock_guard lock(active_buffers_mutex); + if (active_buffers.find(buffer) != active_buffers.end()) { + buffer->UnmarkInUse(); + } }; gpu_surface_->release_context = this; } diff --git a/packages/webview_flutter/tizen/src/webview.cc b/packages/webview_flutter/tizen/src/webview.cc index 9bc0e73ad..97d1385eb 100644 --- a/packages/webview_flutter/tizen/src/webview.cc +++ b/packages/webview_flutter/tizen/src/webview.cc @@ -8,8 +8,15 @@ #include #include #include +#include #include +#include +#include +#include +#include +#include + #include "buffer_pool.h" #include "log.h" #include "webview_factory.h" @@ -44,9 +51,15 @@ std::string ConvertLogLevelToString(Ewk_Console_Message_Level level) { class NavigationRequestResult : public FlMethodResult { public: - NavigationRequestResult(WebView* webview) : webview_(webview) {} + // Dart resolves "navigationRequest" asynchronously, so completion can run + // after |webview| is destroyed; |alive| gates every dereference. + NavigationRequestResult(WebView* webview, std::shared_ptr alive) + : webview_(webview), alive_(std::move(alive)) {} void SuccessInternal(const flutter::EncodableValue* should_load) override { + if (!*alive_) { + return; + } if (std::holds_alternative(*should_load)) { if (std::get(*should_load)) { webview_->Resume(); @@ -60,16 +73,23 @@ class NavigationRequestResult : public FlMethodResult { const std::string& error_message, const flutter::EncodableValue* error_details) override { LOG_ERROR("The request unexpectedly completed with an error."); + if (!*alive_) { + return; + } webview_->Stop(); } void NotImplementedInternal() override { LOG_ERROR("The target method was unexpectedly unimplemented."); + if (!*alive_) { + return; + } webview_->Stop(); } private: WebView* webview_; + std::shared_ptr alive_; }; template @@ -87,6 +107,45 @@ bool GetValueFromEncodableMap(const flutter::EncodableValue* arguments, return false; } +// Deferred: the raster thread may still be reading TBM surfaces the engine +// owns, and ewk_shutdown() fatally CHECKs if any Ewk_View is still alive. +// FlushPendingTeardowns() drains this registry before that shutdown call. +struct PendingTeardown { + Evas_Object* instance = nullptr; + // Guards against the queued callback and a force-delete both deleting + // the same instance. + std::atomic completed{false}; +}; + +std::mutex g_pending_teardown_mutex; +std::vector> g_pending_teardowns; + +Ecore_Evas* g_shared_canvas = nullptr; + +std::shared_ptr RegisterPendingTeardown( + Evas_Object* instance) { + auto pending = std::make_shared(); + pending->instance = instance; + std::lock_guard lock(g_pending_teardown_mutex); + g_pending_teardowns.push_back(pending); + return pending; +} + +// Idempotent: safe to call more than once for the same |pending|. +void CompletePendingTeardown(const std::shared_ptr& pending) { + bool expected = false; + if (pending->completed.compare_exchange_strong(expected, true) && + pending->instance) { + evas_object_del(pending->instance); + } + std::lock_guard lock(g_pending_teardown_mutex); + auto it = std::find(g_pending_teardowns.begin(), g_pending_teardowns.end(), + pending); + if (it != g_pending_teardowns.end()) { + g_pending_teardowns.erase(it); + } +} + } // namespace WebView::WebView(flutter::PluginRegistrar* registrar, int view_id, @@ -103,7 +162,7 @@ WebView::WebView(flutter::PluginRegistrar* registrar, int view_id, return; } - tbm_pool_ = std::make_unique(width, height); + tbm_pool_ = std::make_shared(width, height); texture_variant_ = std::make_unique(flutter::GpuSurfaceTexture( @@ -173,33 +232,128 @@ void WebView::Dispose() { if (disposed_) { return; } + disposed_ = true; + *is_alive_ = false; - texture_registrar_->UnregisterTexture(GetTextureId(), nullptr); + Evas_Object* instance = webview_instance_; + webview_instance_ = nullptr; - if (webview_instance_) { - evas_object_smart_callback_del(webview_instance_, - "offscreen,frame,rendered", + if (instance) { + // |instance| outlives this WebView until its deferred delete runs (see + // PendingTeardown), so every callback bound to it must be detached now. + evas_object_smart_callback_del(instance, "offscreen,frame,rendered", &WebView::OnFrameRendered); - evas_object_smart_callback_del(webview_instance_, "load,started", + evas_object_smart_callback_del(instance, "load,started", &WebView::OnLoadStarted); - evas_object_smart_callback_del(webview_instance_, "load,finished", + evas_object_smart_callback_del(instance, "load,finished", &WebView::OnLoadFinished); - evas_object_smart_callback_del(webview_instance_, "load,progress", + evas_object_smart_callback_del(instance, "load,progress", &WebView::OnProgress); - evas_object_smart_callback_del(webview_instance_, "load,error", + evas_object_smart_callback_del(instance, "load,error", &WebView::OnLoadError); - evas_object_smart_callback_del(webview_instance_, "console,message", + evas_object_smart_callback_del(instance, "console,message", &WebView::OnConsoleMessage); - evas_object_smart_callback_del(webview_instance_, - "policy,navigation,decide", + evas_object_smart_callback_del(instance, "policy,navigation,decide", &WebView::OnNavigationPolicy); - evas_object_smart_callback_del(webview_instance_, "url,changed", + evas_object_smart_callback_del(instance, "policy,response,decide", + &WebView::OnResponsePolicy); + evas_object_smart_callback_del(instance, "url,changed", &WebView::OnUrlChange); - evas_object_del(webview_instance_); + EwkInternalApiBinding::GetInstance().view.OnJavaScriptAlert( + instance, nullptr, nullptr); + EwkInternalApiBinding::GetInstance().view.OnJavaScriptConfirm( + instance, nullptr, nullptr); + EwkInternalApiBinding::GetInstance().view.OnJavaScriptPrompt( + instance, nullptr, nullptr); + evas_object_data_del(instance, kEwkInstance); + + // Stop the page so it cannot run while the deferred teardown is pending. + ewk_view_stop(instance); + ewk_view_suspend(instance); } - // ewk_shutdown(); - disposed_ = true; + std::shared_ptr pool; + { + std::lock_guard lock(mutex_); + is_disposing_ = true; + working_surface_ = nullptr; + candidate_surface_ = nullptr; + rendered_surface_ = nullptr; + pool = std::move(tbm_pool_); + } + + // UnregisterTexture()'s completion callback fires on the render thread, so + // hop back to the main loop before completing the deferred delete. + struct TeardownContext { + std::shared_ptr pending; + std::shared_ptr pool; + }; + auto* context = + new TeardownContext{RegisterPendingTeardown(instance), std::move(pool)}; + texture_registrar_->UnregisterTexture(GetTextureId(), [context]() { + // Must stay a high-priority timeout: g_idle_add() runs too late and the + // delete then races the raster thread on the TV emulator. + g_timeout_add_full( + G_PRIORITY_HIGH, 0, + [](gpointer data) -> gboolean { + auto* context = static_cast(data); + CompletePendingTeardown(context->pending); + return G_SOURCE_REMOVE; + }, + context, + [](gpointer data) { delete static_cast(data); }); + }); +} + +// static +Ecore_Evas* WebView::GetSharedCanvas() { + if (!g_shared_canvas) { + // "wayland_shm", not "wayland_egl": this canvas only hosts the ewk_view + // smart object (content arrives via tbm_surface), and wayland_egl raced + // libtpl-egl's wl_egl_thread teardown on disposal. + g_shared_canvas = ecore_evas_new("wayland_shm", 0, 0, 1, 1, 0); + } + return g_shared_canvas; +} + +// static +void WebView::FreeSharedCanvas() { + if (g_shared_canvas) { + ecore_evas_free(g_shared_canvas); + g_shared_canvas = nullptr; + } +} + +// static +void WebView::FlushPendingTeardowns() { + constexpr gint64 kDeadlineUsec = 2 * G_USEC_PER_SEC; + const gint64 deadline = g_get_monotonic_time() + kDeadlineUsec; + for (;;) { + std::shared_ptr pending; + { + std::lock_guard lock(g_pending_teardown_mutex); + if (g_pending_teardowns.empty()) { + return; + } + pending = g_pending_teardowns.front(); + } + if (g_get_monotonic_time() >= deadline) { + // Force stragglers through past the deadline instead of blocking + // forever. + LOG_WARN( + "Forcing WebView teardown past the deadline before ewk_shutdown()"); + CompletePendingTeardown(pending); + continue; + } + // Pump the same GLib context the queued g_timeout_add_full hop (see + // Dispose()) is scheduled on, so it gets a chance to run and remove + // this entry itself. Non-blocking: if that hop never arrives (e.g. the + // render thread already stalled), a blocking iteration here would never + // return to let the deadline check above fire. + if (!g_main_context_iteration(g_main_context_default(), FALSE)) { + g_usleep(1000); + } + } } void WebView::Offset(double left, double top) { @@ -331,9 +485,17 @@ bool WebView::SendKey(const char* key, const char* string, const char* compose, return true; } -void WebView::Resume() { ewk_view_resume(webview_instance_); } +void WebView::Resume() { + if (webview_instance_) { + ewk_view_resume(webview_instance_); + } +} -void WebView::Stop() { ewk_view_stop(webview_instance_); } +void WebView::Stop() { + if (webview_instance_) { + ewk_view_stop(webview_instance_); + } +} void WebView::SetDirection(int direction) { // TODO: Implement if necessary. @@ -355,16 +517,12 @@ bool WebView::InitWebView() { EwkInternalApiBinding::GetInstance().main.SetArguments(chromium_argc, chromium_argv); - // TODO(jsuya): ewk_init() and ewk_shutdown() are designed to be called only - // once in a process.(If ewk_init() is called after ewk_shutdown() is - // called, SIGTRAP is called internally.) ewk_init() initializes the efl - // modules and web engine's arguments data. The efl modules are initialized - // by default in OS, and arguments data is also initialized through - // SetArguments() API, so calling ewk_init() is not necessary. Therefore, - // temporarily comment out ewk_init() and ewk_shutdown(). It can be reverted - // depending on updates to chromium-efl. - // ewk_init(); - Ecore_Evas* evas = ecore_evas_new("wayland_egl", 0, 0, 1, 1, 0); + // ewk_init() is called once for the process lifetime by + // WebviewFlutterTizenPlugin's constructor, not per WebView instance. + Ecore_Evas* evas = GetSharedCanvas(); + if (!evas) { + return false; + } webview_instance_ = ewk_view_add(ecore_evas_get(evas)); if (!webview_instance_) { @@ -429,6 +587,8 @@ bool WebView::InitWebView() { &WebView::OnConsoleMessage, this); evas_object_smart_callback_add(webview_instance_, "policy,navigation,decide", &WebView::OnNavigationPolicy, this); + evas_object_smart_callback_add(webview_instance_, "policy,response,decide", + &WebView::OnResponsePolicy, this); evas_object_smart_callback_add(webview_instance_, "url,changed", &WebView::OnUrlChange, this); @@ -452,6 +612,12 @@ void WebView::HandleWebViewMethodCall(const FlMethodCall& method_call, const std::string& method_name = method_call.method_name(); const flutter::EncodableValue* arguments = method_call.arguments(); + if (disposed_) { + result->Error("Invalid operation", + "The webview instance has been disposed."); + return; + } + if (method_name == "setEnginePolicy") { const auto* engine_policy = std::get_if(arguments); if (engine_policy) { @@ -580,6 +746,10 @@ void WebView::HandleWebViewMethodCall(const FlMethodCall& method_call, Ewk_Context* context = ewk_view_context_get(webview_instance_); ewk_context_resource_cache_clear(context); result->Success(); + } else if (method_name == "clearLocalStorage") { + Ewk_Context* context = ewk_view_context_get(webview_instance_); + ewk_context_web_storage_delete_all(context); + result->Success(); } else if (method_name == "getTitle") { result->Success(flutter::EncodableValue( std::string(ewk_view_title_get(webview_instance_)))); @@ -745,6 +915,9 @@ void WebView::HandleCookieMethodCall(const FlMethodCall& method_call, FlutterDesktopGpuSurfaceDescriptor* WebView::ObtainGpuSurface(size_t width, size_t height) { std::lock_guard lock(mutex_); + if (is_disposing_ || !tbm_pool_) { + return nullptr; + } if (!candidate_surface_) { if (rendered_surface_) { return rendered_surface_->GpuSurface(); @@ -764,6 +937,9 @@ void WebView::OnFrameRendered(void* data, Evas_Object* obj, void* event_info) { WebView* webview = static_cast(data); std::lock_guard lock(webview->mutex_); + if (webview->is_disposing_ || !webview->tbm_pool_) { + return; + } if (!webview->working_surface_) { webview->working_surface_ = webview->tbm_pool_->GetAvailableBuffer(); webview->working_surface_->UseExternalBuffer(); @@ -865,12 +1041,35 @@ void WebView::OnNavigationPolicy(void* data, Evas_Object* obj, {flutter::EncodableValue("isForMainFrame"), flutter::EncodableValue(true)}, }; - auto result = std::make_unique(webview); + auto result = + std::make_unique(webview, webview->is_alive_); webview->navigation_delegate_channel_->InvokeMethod( "navigationRequest", std::make_unique(args), std::move(result)); } +void WebView::OnResponsePolicy(void* data, Evas_Object* obj, void* event_info) { + WebView* webview = static_cast(data); + Ewk_Policy_Decision* policy_decision = + static_cast(event_info); + int status_code = + ewk_policy_decision_response_status_code_get(policy_decision); + const char* url = ewk_policy_decision_url_get(policy_decision); + ewk_policy_decision_use(policy_decision); + + // HTTP error status codes (4xx, 5xx) are reported to the navigation delegate. + if (!webview->has_navigation_delegate_ || status_code < 400) { + return; + } + flutter::EncodableMap args = { + {flutter::EncodableValue("url"), flutter::EncodableValue(url ? url : "")}, + {flutter::EncodableValue("statusCode"), + flutter::EncodableValue(status_code)}, + }; + webview->navigation_delegate_channel_->InvokeMethod( + "onHttpError", std::make_unique(args)); +} + void WebView::OnUrlChange(void* data, Evas_Object* obj, void* event_info) { WebView* webview = static_cast(data); std::string url = std::string(ewk_view_url_get(webview->webview_instance_)); @@ -896,7 +1095,9 @@ void WebView::OnJavaScriptMessage(Evas_Object* obj, if (obj) { WebView* webview = static_cast(evas_object_data_get(obj, kEwkInstance)); - if (webview->webview_channel_) { + // The data key is removed in Dispose(), so a message arriving during the + // deferred teardown yields nullptr here rather than a dangling pointer. + if (webview && webview->webview_channel_) { std::string channel_name(message.name); std::string message_body(static_cast(message.body)); diff --git a/packages/webview_flutter/tizen/src/webview.h b/packages/webview_flutter/tizen/src/webview.h index 688f97ddc..4e981e6a4 100644 --- a/packages/webview_flutter/tizen/src/webview.h +++ b/packages/webview_flutter/tizen/src/webview.h @@ -27,6 +27,7 @@ typedef flutter::MethodChannel FlMethodChannel; class BufferPool; class BufferUnit; +typedef struct _Ecore_Evas Ecore_Evas; class WebView : public PlatformView { public: @@ -58,6 +59,16 @@ class WebView : public PlatformView { FlutterDesktopGpuSurfaceDescriptor* ObtainGpuSurface(size_t width, size_t height); + // Blocks until every WebView's deferred delete (queued in Dispose()) has + // run, force-deleting stragglers after a timeout. Must be called before + // ewk_shutdown(), which fatally CHECKs on any live Ewk_View. + static void FlushPendingTeardowns(); + + static Ecore_Evas* GetSharedCanvas(); + + // Must be called after FlushPendingTeardowns() and before ewk_shutdown(). + static void FreeSharedCanvas(); + private: void HandleWebViewMethodCall(const FlMethodCall& method_call, std::unique_ptr result); @@ -82,6 +93,7 @@ class WebView : public PlatformView { static void OnConsoleMessage(void* data, Evas_Object* obj, void* event_info); static void OnNavigationPolicy(void* data, Evas_Object* obj, void* event_info); + static void OnResponsePolicy(void* data, Evas_Object* obj, void* event_info); static void OnUrlChange(void* data, Evas_Object* obj, void* event_info); static void OnEvaluateJavaScript(Evas_Object* obj, const char* result_value, void* user_data); @@ -115,8 +127,14 @@ class WebView : public PlatformView { std::unique_ptr navigation_delegate_channel_; std::unique_ptr texture_variant_; std::mutex mutex_; - std::unique_ptr tbm_pool_; + std::shared_ptr tbm_pool_; bool disposed_ = false; + // Guarded by mutex_. Keeps the raster thread from being handed TBM surfaces + // during the deferred teardown. + bool is_disposing_ = false; + // Copied into pending async Dart replies so they can detect a WebView that + // was destroyed before the reply arrived. + std::shared_ptr is_alive_ = std::make_shared(true); Ewk_Mouse_Button_Type mouse_button_type_ = (Ewk_Mouse_Button_Type)0; bool scrollbar_enabled_ = true; }; diff --git a/packages/webview_flutter/tizen/src/webview_flutter_tizen_plugin.cc b/packages/webview_flutter/tizen/src/webview_flutter_tizen_plugin.cc index 749f51a9a..431b228e5 100644 --- a/packages/webview_flutter/tizen/src/webview_flutter_tizen_plugin.cc +++ b/packages/webview_flutter/tizen/src/webview_flutter_tizen_plugin.cc @@ -4,17 +4,23 @@ #include "webview_flutter_tizen_plugin.h" +#include #include #include #include +#include "webview.h" #include "webview_factory.h" namespace { constexpr char kViewType[] = "plugins.flutter.io/webview"; +// Tied to this plugin object's lifetime (constructed/destroyed exactly once +// by flutter-tizen's engine start/stop), not per-WebView: chromium-efl does +// not support re-initializing its browser process after ewk_shutdown() has +// run once. class WebviewFlutterTizenPlugin : public flutter::Plugin { public: static void RegisterWithRegistrar(flutter::PluginRegistrar* registrar) { @@ -22,9 +28,14 @@ class WebviewFlutterTizenPlugin : public flutter::Plugin { registrar->AddPlugin(std::move(plugin)); } - WebviewFlutterTizenPlugin() {} + WebviewFlutterTizenPlugin() { ewk_init(); } - virtual ~WebviewFlutterTizenPlugin() {} + virtual ~WebviewFlutterTizenPlugin() { + // See WebView::FlushPendingTeardowns() for why this must run first. + WebView::FlushPendingTeardowns(); + WebView::FreeSharedCanvas(); + ewk_shutdown(); + } }; } // namespace