[webview_flutter_lwe] Add integration test based on upstream v4.13.1

packages/webview_flutter_lwe/CHANGELOG.md

@@ -1,3 +1,15 @@
+## 0.5.0
+
+* Implement `onUrlChange` for the navigation delegate.
+* Fix navigation delegate callbacks being dropped when the navigation delegate
+  is replaced after the WebView is created.
+* Fix a crash on the raster thread when a WebView is disposed while a frame is
+  still being composited.
+* Fix a use-after-free when navigation-delegate or JavaScript-channel callbacks
+  run after the WebView has been disposed.
+* Add 1 integration test case.
+* Update lightweight web engine (1.3.40).
+
 ## 0.4.1
 
 * Remove Ecore API.

packages/webview_flutter_lwe/README.md

@@ -21,7 +21,7 @@ This package is not an _endorsed_ implementation of `webview_flutter`. Therefore
 ```yaml
 dependencies:
   webview_flutter: ^4.13.1
-  webview_flutter_lwe: ^0.4.1
+  webview_flutter_lwe: ^0.4.2
 ```
 
 ## Example

packages/webview_flutter_lwe/example/integration_test/webview_flutter_test.dart

@@ -447,6 +447,36 @@ Future<void> main() async {
       final String? currentUrl = await controller.currentUrl();
       expect(currentUrl, secondaryUrl);
     });
+
+    testWidgets('can receive url changes', (WidgetTester tester) async {
+      final Completer<void> pageLoaded = Completer<void>();
+
+      final WebViewController controller = WebViewController();
+      unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted));
+      unawaited(
+        controller.setNavigationDelegate(
+          NavigationDelegate(onPageFinished: (_) => pageLoaded.complete()),
+        ),
+      );
+      unawaited(controller.loadRequest(Uri.parse(blankPageEncoded)));
+
+      await tester.pumpWidget(WebViewWidget(controller: controller));
+
+      await pageLoaded.future;
+
+      final Completer<String> urlChangeCompleter = Completer<String>();
+      await controller.setNavigationDelegate(
+        NavigationDelegate(
+          onUrlChange: (UrlChange change) {
+            urlChangeCompleter.complete(change.url);
+          },
+        ),
+      );
+
+      await controller.runJavaScript('location.href = "$primaryUrl"');
+
+      await expectLater(urlChangeCompleter.future, completion(primaryUrl));
+    });
   });
 }
 

packages/webview_flutter_lwe/lib/src/lwe_webview_controller.dart

@@ -25,9 +25,11 @@ class LweWebViewController extends PlatformWebViewController {
 
   final LweWebView _webview;
   late LweNavigationDelegate _lweNavigationDelegate;
+  int? _viewId;
 
   /// Called when [TizenView] is created.
   void onCreate(int viewId) {
+    _viewId = viewId;
     if (_webview.hasNavigationDelegate) {
       _lweNavigationDelegate.onCreate(viewId);
     }
@@ -142,6 +144,15 @@ class LweWebViewController extends PlatformWebViewController {
   ) async {
     _lweNavigationDelegate = handler;
     _webview.hasNavigationDelegate = true;
+    // If the view has already been created, the previous delegate's method
+    // call handler is the one currently bound to the platform channel. Bind
+    // the new delegate now so its callbacks (e.g. onUrlChange, onPageFinished)
+    // are the ones invoked; otherwise events would keep going to the old
+    // delegate and be dropped.
+    final int? viewId = _viewId;
+    if (viewId != null) {
+      handler.onCreate(viewId);
+    }
   }
 
   @override
@@ -382,15 +393,21 @@ class LweNavigationDelegate extends PlatformNavigationDelegate {
   /// Creates a new [LweNavigationDelegate].
   LweNavigationDelegate(super.params) : super.implementation();
 
-  late final MethodChannel _navigationDelegateChannel;
+  late MethodChannel _navigationDelegateChannel;
+  int? _viewId;
   PageEventCallback? _onPageFinished;
   PageEventCallback? _onPageStarted;
   ProgressCallback? _onProgress;
   WebResourceErrorCallback? _onWebResourceError;
   NavigationRequestCallback? _onNavigationRequest;
+  UrlChangeCallback? _onUrlChange;
 
   /// Called when [TizenView] is created.
   void onCreate(int viewId) {
+    if (_viewId == viewId) {
+      return;
+    }
+    _viewId = viewId;
     _navigationDelegateChannel = MethodChannel(
       kLweNavigationDelegateChannelName + viewId.toString(),
     );
@@ -431,6 +448,11 @@ class LweNavigationDelegate extends PlatformNavigationDelegate {
             );
           }
           return null;
+        case 'onUrlChange':
+          if (_onUrlChange != null) {
+            _onUrlChange!(UrlChange(url: arguments['url']! as String));
+          }
+          return null;
       }
 
       throw MissingPluginException(
@@ -506,10 +528,7 @@ class LweNavigationDelegate extends PlatformNavigationDelegate {
 
   @override
   Future<void> setOnUrlChange(UrlChangeCallback onUrlChange) async {
-    throw UnimplementedError(
-      'This version of `LweNavigationDelegate` currently has no '
-      'implementation for `setOnUrlChange`',
-    );
+    _onUrlChange = onUrlChange;
   }
 
   @override

packages/webview_flutter_lwe/pubspec.yaml

@@ -2,7 +2,7 @@ name: webview_flutter_lwe
 description: Tizen implementation of the webview_flutter plugin backed by Lightweight Web Engine.
 homepage: https://github.com/flutter-tizen/plugins
 repository: https://github.com/flutter-tizen/plugins/tree/master/packages/webview_flutter_lwe
-version: 0.4.1
+version: 0.5.0
 
 environment:
   sdk: ^3.8.0

packages/webview_flutter_lwe/tizen/src/webview.cc

@@ -37,8 +37,12 @@ extern "C" size_t LWE_EXPORT createWebViewInstance(
 
 class NavigationRequestResult : public FlMethodResult {
  public:
-  NavigationRequestResult(std::string url, WebView* webview)
-      : url_(url), webview_(webview) {}
+  // Dart resolves "navigationRequest" asynchronously, so this result may
+  // complete after the WebView has been disposed. |alive| (the WebView's
+  // is_alive_ flag) is checked before dereferencing |webview_|.
+  NavigationRequestResult(std::string url, WebView* webview,
+                          std::shared_ptr<bool> alive)
+      : url_(url), webview_(webview), alive_(std::move(alive)) {}
 
   void SuccessInternal(const flutter::EncodableValue* should_load) override {
     if (std::holds_alternative<bool>(*should_load)) {
@@ -60,13 +64,17 @@ class NavigationRequestResult : public FlMethodResult {
 
  private:
   void LoadUrl() {
+    if (!*alive_) {
+      return;
+    }
     if (webview_ && webview_->GetWebViewInstance()) {
       webview_->GetWebViewInstance()->LoadURL(url_);
     }
   }
 
   std::string url_;
   WebView* webview_;
+  std::shared_ptr<bool> alive_;
 };
 
 template <typename T>
@@ -125,7 +133,7 @@ WebView::WebView(flutter::PluginRegistrar* registrar, int view_id,
 
   InitWebView();
 
-  dispatcher_ = std::make_unique<MessageDispatcher>();
+  dispatcher_ = std::make_shared<MessageDispatcher>();
 
   webview_channel_ = std::make_unique<FlMethodChannel>(
       GetPluginRegistrar()->messenger(), GetWebViewChannelName(),
@@ -153,28 +161,39 @@ WebView::WebView(flutter::PluginRegistrar* registrar, int view_id,
         flutter::EncodableMap args = {
             {flutter::EncodableValue("url"), flutter::EncodableValue(url)}};
 
-        dispatcher_->dispatchTaskOnMainThread([this, args]() {
+        dispatcher_->dispatchTaskOnMainThread([this, args,
+                                               alive = is_alive_]() {
+          if (!*alive) return;
           navigation_delegate_channel_->InvokeMethod(
               "onPageStarted", std::make_unique<flutter::EncodableValue>(args));
+          // The lightweight web engine has no dedicated URL-change
+          // callback, so report a URL change whenever a navigation
+          // starts.
+          navigation_delegate_channel_->InvokeMethod(
+              "onUrlChange", std::make_unique<flutter::EncodableValue>(args));
         });
       });
   webview_instance_->RegisterOnPageLoadedHandler(
       [this](LWE::WebContainer* container, const std::string& url) {
         flutter::EncodableMap args = {
             {flutter::EncodableValue("url"), flutter::EncodableValue(url)}};
 
-        dispatcher_->dispatchTaskOnMainThread([this, args]() {
-          navigation_delegate_channel_->InvokeMethod(
-              "onPageFinished",
-              std::make_unique<flutter::EncodableValue>(args));
-        });
+        dispatcher_->dispatchTaskOnMainThread(
+            [this, args, alive = is_alive_]() {
+              if (!*alive) return;
+              navigation_delegate_channel_->InvokeMethod(
+                  "onPageFinished",
+                  std::make_unique<flutter::EncodableValue>(args));
+            });
       });
   webview_instance_->RegisterOnProgressChangedHandler(
       [this](LWE::WebContainer* container, int progress) {
         flutter::EncodableMap args = {{flutter::EncodableValue("progress"),
                                        flutter::EncodableValue(progress)}};
 
-        dispatcher_->dispatchTaskOnMainThread([this, args]() {
+        dispatcher_->dispatchTaskOnMainThread([this, args,
+                                               alive = is_alive_]() {
+          if (!*alive) return;
           navigation_delegate_channel_->InvokeMethod(
               "onProgress", std::make_unique<flutter::EncodableValue>(args));
         });
@@ -189,11 +208,13 @@ WebView::WebView(flutter::PluginRegistrar* registrar, int view_id,
             {flutter::EncodableValue("failingUrl"),
              flutter::EncodableValue(error.GetUrl())},
         };
-        dispatcher_->dispatchTaskOnMainThread([this, args]() {
-          navigation_delegate_channel_->InvokeMethod(
-              "onWebResourceError",
-              std::make_unique<flutter::EncodableValue>(args));
-        });
+        dispatcher_->dispatchTaskOnMainThread(
+            [this, args, alive = is_alive_]() {
+              if (!*alive) return;
+              navigation_delegate_channel_->InvokeMethod(
+                  "onWebResourceError",
+                  std::make_unique<flutter::EncodableValue>(args));
+            });
       });
   webview_instance_->RegisterShouldOverrideUrlLoadingHandler(
       [this](LWE::WebContainer* view, const std::string& url) -> bool {
@@ -206,13 +227,16 @@ WebView::WebView(flutter::PluginRegistrar* registrar, int view_id,
              flutter::EncodableValue(true)},
         };
 
-        dispatcher_->dispatchTaskOnMainThread([this, args, url]() {
-          auto result = std::make_unique<NavigationRequestResult>(url, this);
-          navigation_delegate_channel_->InvokeMethod(
-              "navigationRequest",
-              std::make_unique<flutter::EncodableValue>(args),
-              std::move(result));
-        });
+        dispatcher_->dispatchTaskOnMainThread(
+            [this, args, url, alive = is_alive_]() {
+              if (!*alive) return;
+              auto result =
+                  std::make_unique<NavigationRequestResult>(url, this, alive);
+              navigation_delegate_channel_->InvokeMethod(
+                  "navigationRequest",
+                  std::make_unique<flutter::EncodableValue>(args),
+                  std::move(result));
+            });
         return true;
       });
 }
@@ -231,7 +255,8 @@ void WebView::RegisterJavaScriptChannelName(const std::string& name) {
         {flutter::EncodableValue("message"), flutter::EncodableValue(message)},
     };
 
-    dispatcher_->dispatchTaskOnMainThread([this, args]() {
+    dispatcher_->dispatchTaskOnMainThread([this, args, alive = is_alive_]() {
+      if (!*alive) return;
       webview_channel_->InvokeMethod(
           "javaScriptChannelMessage",
           std::make_unique<flutter::EncodableValue>(args));
@@ -257,12 +282,39 @@ std::string WebView::GetNavigationDelegateChannelName() {
 }
 
 void WebView::Dispose() {
-  texture_registrar_->UnregisterTexture(GetTextureId(), nullptr);
+  // Set this first so that dispatcher_ callbacks already queued on the main
+  // loop bail out instead of touching a destroyed WebView.
+  *is_alive_ = false;
 
+  // Stop the web engine first so its renderer thread stops writing into the
+  // shared TBM surfaces before anything is torn down.
   if (webview_instance_) {
     webview_instance_->Destroy();
     webview_instance_ = nullptr;
   }
+
+  // Detach the buffers under the lock so that the raster thread stops handing
+  // out buffers that are about to be released.
+  std::shared_ptr<BufferPool> pool;
+  {
+    std::lock_guard<std::mutex> lock(mutex_);
+    is_disposing_ = true;
+    working_surface_ = nullptr;
+    candidate_surface_ = nullptr;
+    rendered_surface_ = nullptr;
+    pool = std::move(tbm_pool_);
+  }
+
+  // The callback runs on the render thread only after the raster thread is
+  // done with the TBM surfaces, so the pool is destroyed in a main-thread
+  // task. dispatcher_ is captured as a shared_ptr copy because the WebView
+  // may already be destroyed by the time the callback fires.
+  texture_registrar_->UnregisterTexture(
+      GetTextureId(), [pool, dispatcher = dispatcher_]() {
+        dispatcher->dispatchTaskOnMainThread([pool]() {
+          // The pool destructor frees all TBM surfaces.
+        });
+      });
 }
 
 void WebView::Resize(double width, double height) {
@@ -802,6 +854,9 @@ void WebView::HandleCookieMethodCall(const FlMethodCall& method_call,
 FlutterDesktopGpuSurfaceDescriptor* WebView::ObtainGpuSurface(size_t width,
                                                               size_t height) {
   std::lock_guard<std::mutex> lock(mutex_);
+  if (is_disposing_ || !tbm_pool_) {
+    return nullptr;
+  }
   if (!candidate_surface_) {
     if (rendered_surface_) {
       return rendered_surface_->GpuSurface();

packages/webview_flutter_lwe/tizen/src/webview.h

@@ -80,7 +80,16 @@ class WebView : public PlatformView {
   BufferUnit* rendered_surface_ = nullptr;
   bool is_mouse_lbutton_down_ = false;
   bool has_navigation_delegate_ = false;
-  std::unique_ptr<MessageDispatcher> dispatcher_;
+  bool is_disposing_ = false;
+  // Set to false at the start of Dispose(). Every dispatcher_ callback that
+  // captures `this` also captures a copy of this shared_ptr, so a callback
+  // still queued on the main-loop when the WebView is disposed can check it
+  // and bail out instead of dereferencing an already-destroyed WebView.
+  std::shared_ptr<bool> is_alive_ = std::make_shared<bool>(true);
+  // Shared (not unique) so a pending dispose callback can keep the
+  // dispatcher alive by holding its own reference after the WebView that
+  // created it has been destroyed.
+  std::shared_ptr<MessageDispatcher> dispatcher_;
   std::unique_ptr<FlMethodChannel> webview_channel_;
   std::unique_ptr<FlMethodChannel> navigation_delegate_channel_;
   std::unique_ptr<flutter::TextureVariant> texture_variant_;