[url_launcher] Simplify Linux implementation (flutter#5376)

stuartmorgan-g · web-flow · commit 73d2f3eb966d · 2023-12-13T01:35:19.000Z
The Linux implementation's method channel code was never simplified after the switch from a shared method channel to per-package method channels, so there was still cruft from the cross-platform channel API. This removes everything that's not used by the Linux native implementation, simplifying the protocol.

This also adds direct support for `launchUrl`, so we're no longer relying on the redirection from the deprecated `launch`.
diff --git a/packages/url_launcher/url_launcher_linux/CHANGELOG.md b/packages/url_launcher/url_launcher_linux/CHANGELOG.md
@@ -1,5 +1,7 @@
-## NEXT
+## 3.1.1
 
+* Implements `launchUrl`.
+* Simplifies method channel interface by removing unused elements.
 * Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
 
 ## 3.1.0
diff --git a/packages/url_launcher/url_launcher_linux/lib/url_launcher_linux.dart b/packages/url_launcher/url_launcher_linux/lib/url_launcher_linux.dart
@@ -22,11 +22,8 @@ class UrlLauncherLinux extends UrlLauncherPlatform {
   final LinkDelegate? linkDelegate = null;
 
   @override
-  Future<bool> canLaunch(String url) {
-    return _channel.invokeMethod<bool>(
-      'canLaunch',
-      <String, Object>{'url': url},
-    ).then((bool? value) => value ?? false);
+  Future<bool> canLaunch(String url) async {
+    return (await _channel.invokeMethod<bool>('canLaunch', url)) ?? false;
   }
 
   @override
@@ -40,16 +37,14 @@ class UrlLauncherLinux extends UrlLauncherPlatform {
     required Map<String, String> headers,
     String? webOnlyWindowName,
   }) {
-    return _channel.invokeMethod<bool>(
-      'launch',
-      <String, Object>{
-        'url': url,
-        'enableJavaScript': enableJavaScript,
-        'enableDomStorage': enableDomStorage,
-        'universalLinksOnly': universalLinksOnly,
-        'headers': headers,
-      },
-    ).then((bool? value) => value ?? false);
+    // None of the options are supported, so they don't need to be converted to
+    // LaunchOptions.
+    return launchUrl(url, const LaunchOptions());
+  }
+
+  @override
+  Future<bool> launchUrl(String url, LaunchOptions options) async {
+    return (await _channel.invokeMethod<bool>('launch', url)) ?? false;
   }
 
   @override
diff --git a/packages/url_launcher/url_launcher_linux/linux/test/url_launcher_linux_test.cc b/packages/url_launcher/url_launcher_linux/linux/test/url_launcher_linux_test.cc
@@ -15,9 +15,7 @@ namespace url_launcher_plugin {
 namespace test {
 
 TEST(UrlLauncherPlugin, CanLaunchSuccess) {
-  g_autoptr(FlValue) args = fl_value_new_map();
-  fl_value_set_string_take(args, "url",
-                           fl_value_new_string("https://flutter.dev"));
+  g_autoptr(FlValue) args = fl_value_new_string("https://flutter.dev");
   g_autoptr(FlMethodResponse) response = can_launch(nullptr, args);
   ASSERT_NE(response, nullptr);
   ASSERT_TRUE(FL_IS_METHOD_SUCCESS_RESPONSE(response));
@@ -28,8 +26,7 @@ TEST(UrlLauncherPlugin, CanLaunchSuccess) {
 }
 
 TEST(UrlLauncherPlugin, CanLaunchFailureUnhandled) {
-  g_autoptr(FlValue) args = fl_value_new_map();
-  fl_value_set_string_take(args, "url", fl_value_new_string("madeup:scheme"));
+  g_autoptr(FlValue) args = fl_value_new_string("madeup:scheme");
   g_autoptr(FlMethodResponse) response = can_launch(nullptr, args);
   ASSERT_NE(response, nullptr);
   ASSERT_TRUE(FL_IS_METHOD_SUCCESS_RESPONSE(response));
@@ -40,8 +37,7 @@ TEST(UrlLauncherPlugin, CanLaunchFailureUnhandled) {
 }
 
 TEST(UrlLauncherPlugin, CanLaunchFileSuccess) {
-  g_autoptr(FlValue) args = fl_value_new_map();
-  fl_value_set_string_take(args, "url", fl_value_new_string("file:///"));
+  g_autoptr(FlValue) args = fl_value_new_string("file:///");
   g_autoptr(FlMethodResponse) response = can_launch(nullptr, args);
   ASSERT_NE(response, nullptr);
   ASSERT_TRUE(FL_IS_METHOD_SUCCESS_RESPONSE(response));
@@ -52,9 +48,8 @@ TEST(UrlLauncherPlugin, CanLaunchFileSuccess) {
 }
 
 TEST(UrlLauncherPlugin, CanLaunchFailureInvalidFileExtension) {
-  g_autoptr(FlValue) args = fl_value_new_map();
-  fl_value_set_string_take(
-      args, "url", fl_value_new_string("file:///madeup.madeupextension"));
+  g_autoptr(FlValue) args =
+      fl_value_new_string("file:///madeup.madeupextension");
   g_autoptr(FlMethodResponse) response = can_launch(nullptr, args);
   ASSERT_NE(response, nullptr);
   ASSERT_TRUE(FL_IS_METHOD_SUCCESS_RESPONSE(response));
@@ -67,8 +62,7 @@ TEST(UrlLauncherPlugin, CanLaunchFailureInvalidFileExtension) {
 // For consistency with the established mobile implementations,
 // an invalid URL should return false, not an error.
 TEST(UrlLauncherPlugin, CanLaunchFailureInvalidUrl) {
-  g_autoptr(FlValue) args = fl_value_new_map();
-  fl_value_set_string_take(args, "url", fl_value_new_string(""));
+  g_autoptr(FlValue) args = fl_value_new_string("");
   g_autoptr(FlMethodResponse) response = can_launch(nullptr, args);
   ASSERT_NE(response, nullptr);
   ASSERT_TRUE(FL_IS_METHOD_SUCCESS_RESPONSE(response));
diff --git a/packages/url_launcher/url_launcher_linux/linux/url_launcher_plugin.cc b/packages/url_launcher/url_launcher_linux/linux/url_launcher_plugin.cc
@@ -13,11 +13,9 @@
 
 // See url_launcher_channel.dart for documentation.
 const char kChannelName[] = "plugins.flutter.io/url_launcher_linux";
-const char kBadArgumentsError[] = "Bad Arguments";
 const char kLaunchError[] = "Launch Error";
 const char kCanLaunchMethod[] = "canLaunch";
 const char kLaunchMethod[] = "launch";
-const char kUrlKey[] = "url";
 
 struct _FlUrlLauncherPlugin {
   GObject parent_instance;
@@ -30,21 +28,6 @@ struct _FlUrlLauncherPlugin {
 
 G_DEFINE_TYPE(FlUrlLauncherPlugin, fl_url_launcher_plugin, g_object_get_type())
 
-// Gets the URL from the arguments or generates an error.
-static gchar* get_url(FlValue* args, GError** error) {
-  if (fl_value_get_type(args) != FL_VALUE_TYPE_MAP) {
-    g_set_error(error, 0, 0, "Argument map missing or malformed");
-    return nullptr;
-  }
-  FlValue* url_value = fl_value_lookup_string(args, kUrlKey);
-  if (url_value == nullptr) {
-    g_set_error(error, 0, 0, "Missing URL");
-    return nullptr;
-  }
-
-  return g_strdup(fl_value_get_string(url_value));
-}
-
 // Checks if URI has launchable file resource.
 static gboolean can_launch_uri_with_file_resource(FlUrlLauncherPlugin* self,
                                                   const gchar* url) {
@@ -57,12 +40,7 @@ static gboolean can_launch_uri_with_file_resource(FlUrlLauncherPlugin* self,
 
 // Called to check if a URL can be launched.
 FlMethodResponse* can_launch(FlUrlLauncherPlugin* self, FlValue* args) {
-  g_autoptr(GError) error = nullptr;
-  g_autofree gchar* url = get_url(args, &error);
-  if (url == nullptr) {
-    return FL_METHOD_RESPONSE(fl_method_error_response_new(
-        kBadArgumentsError, error->message, nullptr));
-  }
+  const gchar* url = fl_value_get_string(args);
 
   gboolean is_launchable = FALSE;
   g_autofree gchar* scheme = g_uri_parse_scheme(url);
@@ -82,14 +60,10 @@ FlMethodResponse* can_launch(FlUrlLauncherPlugin* self, FlValue* args) {
 
 // Called when a URL should launch.
 static FlMethodResponse* launch(FlUrlLauncherPlugin* self, FlValue* args) {
-  g_autoptr(GError) error = nullptr;
-  g_autofree gchar* url = get_url(args, &error);
-  if (url == nullptr) {
-    return FL_METHOD_RESPONSE(fl_method_error_response_new(
-        kBadArgumentsError, error->message, nullptr));
-  }
+  const gchar* url = fl_value_get_string(args);
 
   FlView* view = fl_plugin_registrar_get_view(self->registrar);
+  g_autoptr(GError) error = nullptr;
   gboolean launched;
   if (view != nullptr) {
     GtkWindow* window = GTK_WINDOW(gtk_widget_get_toplevel(GTK_WIDGET(view)));
diff --git a/packages/url_launcher/url_launcher_linux/pubspec.yaml b/packages/url_launcher/url_launcher_linux/pubspec.yaml
@@ -2,7 +2,7 @@ name: url_launcher_linux
 description: Linux implementation of the url_launcher plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/url_launcher/url_launcher_linux
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22
-version: 3.1.0
+version: 3.1.1
 
 environment:
   sdk: ">=3.0.0 <4.0.0"
diff --git a/packages/url_launcher/url_launcher_linux/test/url_launcher_linux_test.dart b/packages/url_launcher/url_launcher_linux/test/url_launcher_linux_test.dart
@@ -38,11 +38,7 @@ void main() {
       await launcher.canLaunch('http://example.com/');
       expect(
         log,
-        <Matcher>[
-          isMethodCall('canLaunch', arguments: <String, Object>{
-            'url': 'http://example.com/',
-          })
-        ],
+        <Matcher>[isMethodCall('canLaunch', arguments: 'http://example.com/')],
       );
     });
 
@@ -66,65 +62,7 @@ void main() {
       );
       expect(
         log,
-        <Matcher>[
-          isMethodCall('launch', arguments: <String, Object>{
-            'url': 'http://example.com/',
-            'enableJavaScript': false,
-            'enableDomStorage': false,
-            'universalLinksOnly': false,
-            'headers': <String, String>{},
-          })
-        ],
-      );
-    });
-
-    test('launch with headers', () async {
-      final UrlLauncherLinux launcher = UrlLauncherLinux();
-      await launcher.launch(
-        'http://example.com/',
-        useSafariVC: true,
-        useWebView: false,
-        enableJavaScript: false,
-        enableDomStorage: false,
-        universalLinksOnly: false,
-        headers: const <String, String>{'key': 'value'},
-      );
-      expect(
-        log,
-        <Matcher>[
-          isMethodCall('launch', arguments: <String, Object>{
-            'url': 'http://example.com/',
-            'enableJavaScript': false,
-            'enableDomStorage': false,
-            'universalLinksOnly': false,
-            'headers': <String, String>{'key': 'value'},
-          })
-        ],
-      );
-    });
-
-    test('launch universal links only', () async {
-      final UrlLauncherLinux launcher = UrlLauncherLinux();
-      await launcher.launch(
-        'http://example.com/',
-        useSafariVC: false,
-        useWebView: false,
-        enableJavaScript: false,
-        enableDomStorage: false,
-        universalLinksOnly: true,
-        headers: const <String, String>{},
-      );
-      expect(
-        log,
-        <Matcher>[
-          isMethodCall('launch', arguments: <String, Object>{
-            'url': 'http://example.com/',
-            'enableJavaScript': false,
-            'enableDomStorage': false,
-            'universalLinksOnly': true,
-            'headers': <String, String>{},
-          })
-        ],
+        <Matcher>[isMethodCall('launch', arguments: 'http://example.com/')],
       );
     });
 
@@ -143,6 +81,25 @@ void main() {
       expect(launched, false);
     });
 
+    group('launchUrl', () {
+      test('passes URL', () async {
+        final UrlLauncherLinux launcher = UrlLauncherLinux();
+        await launcher.launchUrl('http://example.com/', const LaunchOptions());
+        expect(
+          log,
+          <Matcher>[isMethodCall('launch', arguments: 'http://example.com/')],
+        );
+      });
+
+      test('returns false if platform returns null', () async {
+        final UrlLauncherLinux launcher = UrlLauncherLinux();
+        final bool launched = await launcher.launchUrl(
+            'http://example.com/', const LaunchOptions());
+
+        expect(launched, false);
+      });
+    });
+
     group('supportsMode', () {
       test('returns true for platformDefault', () async {
         final UrlLauncherLinux launcher = UrlLauncherLinux();
