From 6b6d72393fa9a18ee2156aac26f766d7419281be Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 9 Jul 2021 15:44:40 -0400 Subject: [PATCH 01/10] Initial proof of concept --- .../example/windows/CMakeLists.txt | 4 ++- .../example/windows/flutter/CMakeLists.txt | 1 + .../windows/CMakeLists.txt | 30 +++++++++++++++++++ .../test/url_launcher_windows_test.cpp | 6 ++++ 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp diff --git a/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt index abf90408efb4..d67f4b7b0865 100644 --- a/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt @@ -46,11 +46,13 @@ add_subdirectory(${FLUTTER_MANAGED_DIR}) # Application build add_subdirectory("runner") +# Enable the test target. +set(include_url_launcher_windows_tests TRUE) + # Generated plugin build rules, which manage building the plugins and adding # them to the application. include(flutter/generated_plugins.cmake) - # === Installation === # Support files are copied into place next to the executable, so that it can # run in place. This is done instead of making a separate bundle (as on Linux) diff --git a/packages/url_launcher/url_launcher_windows/example/windows/flutter/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/example/windows/flutter/CMakeLists.txt index c7a8c7607d81..744f08a9389b 100644 --- a/packages/url_launcher/url_launcher_windows/example/windows/flutter/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/example/windows/flutter/CMakeLists.txt @@ -91,6 +91,7 @@ add_custom_command( ${FLUTTER_TOOL_ENVIRONMENT} "${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.bat" windows-x64 $ + VERBATIM ) add_custom_target(flutter_assemble DEPENDS "${FLUTTER_LIBRARY}" diff --git a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt index 57d87e3f6f85..f9319b367323 100644 --- a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt @@ -20,3 +20,33 @@ set(file_chooser_bundled_libraries "" PARENT_SCOPE ) + +# === Tests === + +if (${include_${PROJECT_NAME}_tests}) +set(TEST_RUNNER_NAME "${PROJECT_NAME}_test") +enable_testing() +# TODO(stuartmorgan): Use a single shared, pre-checked-in googletest instance +# rather than downloading for each plugin. This approach makes sense for a +# template, but not for a monorepo with many plugins. +include(FetchContent) +FetchContent_Declare( + googletest + URL https://github.com/google/googletest/archive/release-1.11.0.zip +) +# Prevent overriding the parent project's compiler/linker settings +set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) +# Disable install commands for gtest so it doesn't end up in the bundle. +set(INSTALL_GTEST OFF CACHE BOOL "Disable installation of googletest" FORCE) + +FetchContent_MakeAvailable(googletest) + +add_executable(${TEST_RUNNER_NAME} + test/url_launcher_windows_test.cpp +) +target_link_libraries(${TEST_RUNNER_NAME} + gtest_main +) +include(GoogleTest) +gtest_discover_tests(${TEST_RUNNER_NAME}) +endif() diff --git a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp new file mode 100644 index 000000000000..0f2b983aaebb --- /dev/null +++ b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp @@ -0,0 +1,6 @@ +#include + +TEST(TestMe, HelloTestWorld) { + EXPECT_STRNE("hello", "world"); + EXPECT_TRUE(false); +} From 99b2ab190e4f089ad03145d33057c298172e48c8 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 13 Jul 2021 14:28:08 -0400 Subject: [PATCH 02/10] Refactor file structure, plugin class, for testability --- .../windows/system_apis.cc | 34 +++++++ .../windows/system_apis.h | 56 +++++++++++ .../windows/url_launcher_plugin.cpp | 94 +++++++++++-------- .../windows/url_launcher_plugin.h | 48 ++++++++++ 4 files changed, 193 insertions(+), 39 deletions(-) create mode 100644 packages/url_launcher/url_launcher_windows/windows/system_apis.cc create mode 100644 packages/url_launcher/url_launcher_windows/windows/system_apis.h create mode 100644 packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h diff --git a/packages/url_launcher/url_launcher_windows/windows/system_apis.cc b/packages/url_launcher/url_launcher_windows/windows/system_apis.cc new file mode 100644 index 000000000000..c089e968800a --- /dev/null +++ b/packages/url_launcher/url_launcher_windows/windows/system_apis.cc @@ -0,0 +1,34 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#include "system_apis.h" + +#include + +namespace url_launcher_plugin { + +SystemApis::SystemApis() {} + +SystemApisImpl::SystemApisImpl() {} + +LSTATUS SystemApisImpl::RegCloseKey(HKEY key) { return ::RegCloseKey(key); } + +LSTATUS SystemApisImpl::RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options, + REGSAM desired, PHKEY result) { + return ::RegOpenKeyExW(key, sub_key, options, desired, result); +} + +LSTATUS SystemApisImpl::RegQueryValueExW(HKEY key, LPCWSTR value_name, + LPDWORD type, LPBYTE data, + LPDWORD data_size) { + return ::RegQueryValueExW(key, value_name, nullptr, type, data, data_size); +} + +HINSTANCE SystemApisImpl::ShellExecuteW(HWND hwnd, LPCWSTR operation, + LPCWSTR file, LPCWSTR parameters, + LPCWSTR directory, int show_flags) { + return ::ShellExecuteW(hwnd, operation, file, parameters, directory, + show_flags); +} + +} // namespace url_launcher_plugin diff --git a/packages/url_launcher/url_launcher_windows/windows/system_apis.h b/packages/url_launcher/url_launcher_windows/windows/system_apis.h new file mode 100644 index 000000000000..7b56704d8e04 --- /dev/null +++ b/packages/url_launcher/url_launcher_windows/windows/system_apis.h @@ -0,0 +1,56 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#include + +namespace url_launcher_plugin { + +// An interface wrapping system APIs used by the plugin, for mocking. +class SystemApis { + public: + SystemApis(); + virtual ~SystemApis(); + + // Disallow copy and move. + SystemApis(const SystemApis&) = delete; + SystemApis& operator=(const SystemApis&) = delete; + + // Wrapper for RegCloseKey. + virtual LSTATUS RegCloseKey(HKEY key) = 0; + + // Wrapper for RegQueryValueEx. + virtual LSTATUS RegQueryValueExW(HKEY key, LPCWSTR value_name, LPDWORD type, + LPBYTE data, LPDWORD data_size) = 0; + + // Wrapper for RegOpenKeyEx. + virtual LSTATUS RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options, + REGSAM desired, PHKEY result) = 0; + + // Wrapper for ShellExecute. + virtual HINSTANCE ShellExecuteW(HWND hwnd, LPCWSTR operation, LPCWSTR file, + LPCWSTR parameters, LPCWSTR directory, + int show_flags) = 0; +}; + +// Implementation of SystemApis using the Win32 APIs. +class SystemApisImpl : public SystemApis { + public: + SystemApisImpl(); + virtual ~SystemApisImpl(); + + // Disallow copy and move. + SystemApisImpl(const SystemApisImpl&) = delete; + SystemApisImpl& operator=(const SystemApisImpl&) = delete; + + // SystemApis Implementation: + virtual LSTATUS RegCloseKey(HKEY key); + virtual LSTATUS RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options, + REGSAM desired, PHKEY result); + virtual LSTATUS RegQueryValueExW(HKEY key, LPCWSTR value_name, LPDWORD type, + LPBYTE data, LPDWORD data_size); + virtual HINSTANCE ShellExecuteW(HWND hwnd, LPCWSTR operation, LPCWSTR file, + LPCWSTR parameters, LPCWSTR directory, + int show_flags); +}; + +} // namespace url_launcher_plugin diff --git a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp index 51740a3a4b04..815627491e61 100644 --- a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp @@ -1,7 +1,7 @@ // Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "include/url_launcher_windows/url_launcher_plugin.h" +#include "url_launcher_plugin.h" #include #include @@ -9,9 +9,14 @@ #include #include +#include #include #include +#include "include/url_launcher_windows/url_launcher_plugin.h" + +namespace url_launcher_plugin { + namespace { using flutter::EncodableMap; @@ -54,19 +59,7 @@ std::string GetUrlArgument(const flutter::MethodCall<>& method_call) { return url; } -class UrlLauncherPlugin : public flutter::Plugin { - public: - static void RegisterWithRegistrar(flutter::PluginRegistrar* registrar); - - virtual ~UrlLauncherPlugin(); - - private: - UrlLauncherPlugin(); - - // Called when a method is called on plugin channel; - void HandleMethodCall(const flutter::MethodCall<>& method_call, - std::unique_ptr> result); -}; +} // namespace // static void UrlLauncherPlugin::RegisterWithRegistrar( @@ -86,7 +79,11 @@ void UrlLauncherPlugin::RegisterWithRegistrar( registrar->AddPlugin(std::move(plugin)); } -UrlLauncherPlugin::UrlLauncherPlugin() = default; +UrlLauncherPlugin::UrlLauncherPlugin() + : system_apis_(std::make_unique()) {} + +UrlLauncherPlugin::UrlLauncherPlugin(std::unique_ptr system_apis) + : system_apis_(std::move(system_apis)) {} UrlLauncherPlugin::~UrlLauncherPlugin() = default; @@ -99,17 +96,10 @@ void UrlLauncherPlugin::HandleMethodCall( result->Error("argument_error", "No URL provided"); return; } - std::wstring url_wide = Utf16FromUtf8(url); - int status = static_cast(reinterpret_cast( - ::ShellExecute(nullptr, TEXT("open"), url_wide.c_str(), nullptr, - nullptr, SW_SHOWNORMAL))); - - if (status <= 32) { - std::ostringstream error_message; - error_message << "Failed to open " << url << ": ShellExecute error code " - << status; - result->Error("open_error", error_message.str()); + std::optional error = LaunchUrl(url); + if (error) { + result->Error("open_error", error.value()); return; } result->Success(EncodableValue(true)); @@ -120,29 +110,55 @@ void UrlLauncherPlugin::HandleMethodCall( return; } - bool can_launch = false; - size_t separator_location = url.find(":"); - if (separator_location != std::string::npos) { - std::wstring scheme = Utf16FromUtf8(url.substr(0, separator_location)); - HKEY key = nullptr; - if (::RegOpenKeyEx(HKEY_CLASSES_ROOT, scheme.c_str(), 0, KEY_QUERY_VALUE, - &key) == ERROR_SUCCESS) { - can_launch = ::RegQueryValueEx(key, L"URL Protocol", nullptr, nullptr, - nullptr, nullptr) == ERROR_SUCCESS; - ::RegCloseKey(key); - } - } + bool can_launch = CanLaunchUrl(url); result->Success(EncodableValue(can_launch)); } else { result->NotImplemented(); } } -} // namespace +bool UrlLauncherPlugin::CanLaunchUrl(const std::string& url) { + size_t separator_location = url.find(":"); + if (separator_location == std::string::npos) { + return false; + } + std::wstring scheme = Utf16FromUtf8(url.substr(0, separator_location)); + + HKEY key = nullptr; + if (system_apis_->RegOpenKeyExW(HKEY_CLASSES_ROOT, scheme.c_str(), 0, + KEY_QUERY_VALUE, &key) != ERROR_SUCCESS) { + return false; + } + bool has_handler = + system_apis_->RegQueryValueExW(key, L"URL Protocol", nullptr, nullptr, + nullptr) == ERROR_SUCCESS; + system_apis_->RegCloseKey(key); + return has_handler; +} + +std::optional UrlLauncherPlugin::LaunchUrl( + const std::string& url) { + std::wstring url_wide = Utf16FromUtf8(url); + + int status = static_cast(reinterpret_cast( + system_apis_->ShellExecuteW(nullptr, TEXT("open"), url_wide.c_str(), + nullptr, nullptr, SW_SHOWNORMAL))); + + // Per ::ShellExecuteW documentation, anything >32 indicates success. + if (status <= 32) { + std::ostringstream error_message; + error_message << "Failed to open " << url << ": ShellExecute error code " + << status; + return std::optional(error_message.str()); + } + return std::nullopt; +} + +} // namespace url_launcher_plugin void UrlLauncherPluginRegisterWithRegistrar( FlutterDesktopPluginRegistrarRef registrar) { - UrlLauncherPlugin::RegisterWithRegistrar( + url_launcher_plugin::UrlLauncherPlugin::RegisterWithRegistrar( flutter::PluginRegistrarManager::GetInstance() ->GetRegistrar(registrar)); } diff --git a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h new file mode 100644 index 000000000000..db39995474d7 --- /dev/null +++ b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h @@ -0,0 +1,48 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#include +#include +#include + +#include +#include +#include +#include + +#include "system_apis.h" + +namespace url_launcher_plugin { + +class UrlLauncherPlugin : public flutter::Plugin { + public: + static void RegisterWithRegistrar(flutter::PluginRegistrar* registrar); + + virtual ~UrlLauncherPlugin(); + + // Disallow copy and move. + UrlLauncherPlugin(const UrlLauncherPlugin&) = delete; + UrlLauncherPlugin& operator=(const UrlLauncherPlugin&) = delete; + + private: + UrlLauncherPlugin(); + + // Creates a plugin instance with the given SystemApi instance. + // + // Exists for unit testing with mock implementations. + UrlLauncherPlugin(std::unique_ptr system_apis); + + // Called when a method is called on plugin channel; + void HandleMethodCall(const flutter::MethodCall<>& method_call, + std::unique_ptr> result); + + // Returns whether or not the given URL has a registered handler. + bool CanLaunchUrl(const std::string& url); + + // Attempts to launch the given URL. On failure, returns an error string. + std::optional LaunchUrl(const std::string& url); + + std::unique_ptr system_apis_; +}; + +} // namespace url_launcher_plugin From bc94f32468490275bab4165302be946ab27af8bc Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 14 Jul 2021 14:27:40 -0400 Subject: [PATCH 03/10] Add real tests, and make them build. --- .../windows/CMakeLists.txt | 35 +++- .../{system_apis.cc => system_apis.cpp} | 4 + .../test/url_launcher_windows_test.cpp | 170 +++++++++++++++++- .../windows/url_launcher_plugin.cpp | 13 +- .../windows/url_launcher_plugin.h | 16 +- .../url_launcher_plugin_registration.cpp | 14 ++ 6 files changed, 221 insertions(+), 31 deletions(-) rename packages/url_launcher/url_launcher_windows/windows/{system_apis.cc => system_apis.cpp} (94%) create mode 100644 packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp diff --git a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt index f9319b367323..bc7dfc95bd76 100644 --- a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt @@ -4,16 +4,23 @@ project(${PROJECT_NAME} LANGUAGES CXX) set(PLUGIN_NAME "${PROJECT_NAME}_plugin") -add_library(${PLUGIN_NAME} SHARED +list(APPEND PLUGIN_SOURCES + "system_apis.cpp" + "system_apis.h" "url_launcher_plugin.cpp" + "url_launcher_plugin.h" +) + +add_library(${PLUGIN_NAME} SHARED + "url_launcher_plugin_registration.cpp" + ${PLUGIN_SOURCES} ) apply_standard_settings(${PLUGIN_NAME}) -set_target_properties(${PLUGIN_NAME} PROPERTIES - CXX_VISIBILITY_PRESET hidden) +set_target_properties(${PLUGIN_NAME} PROPERTIES CXX_VISIBILITY_PRESET hidden) target_compile_definitions(${PLUGIN_NAME} PRIVATE FLUTTER_PLUGIN_IMPL) target_include_directories(${PLUGIN_NAME} INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/include") -target_link_libraries(${PLUGIN_NAME} PRIVATE flutter flutter_wrapper_plugin) +target_link_libraries(${PLUGIN_NAME} PRIVATE flutter_wrapper_plugin) # List of absolute paths to libraries that should be bundled with the plugin set(file_chooser_bundled_libraries @@ -24,7 +31,7 @@ set(file_chooser_bundled_libraries # === Tests === if (${include_${PROJECT_NAME}_tests}) -set(TEST_RUNNER_NAME "${PROJECT_NAME}_test") +set(TEST_RUNNER "${PROJECT_NAME}_test") enable_testing() # TODO(stuartmorgan): Use a single shared, pre-checked-in googletest instance # rather than downloading for each plugin. This approach makes sense for a @@ -41,12 +48,22 @@ set(INSTALL_GTEST OFF CACHE BOOL "Disable installation of googletest" FORCE) FetchContent_MakeAvailable(googletest) -add_executable(${TEST_RUNNER_NAME} +# The plugin's C API is not very useful for unit testing, so build the sources +# directly into the test binary rather than using the DLL. +add_executable(${TEST_RUNNER} test/url_launcher_windows_test.cpp + ${PLUGIN_SOURCES} ) -target_link_libraries(${TEST_RUNNER_NAME} - gtest_main +apply_standard_settings(${TEST_RUNNER}) +target_include_directories(${TEST_RUNNER} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}") +target_link_libraries(${TEST_RUNNER} PRIVATE flutter_wrapper_plugin) +target_link_libraries(${TEST_RUNNER} PRIVATE gtest_main gmock) +# flutter_wrapper_plugin has link dependencies on the Flutter DLL. +add_custom_command(TARGET ${TEST_RUNNER} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy_if_different + "${FLUTTER_LIBRARY}" $ ) + include(GoogleTest) -gtest_discover_tests(${TEST_RUNNER_NAME}) +gtest_discover_tests(${TEST_RUNNER}) endif() diff --git a/packages/url_launcher/url_launcher_windows/windows/system_apis.cc b/packages/url_launcher/url_launcher_windows/windows/system_apis.cpp similarity index 94% rename from packages/url_launcher/url_launcher_windows/windows/system_apis.cc rename to packages/url_launcher/url_launcher_windows/windows/system_apis.cpp index c089e968800a..abd690b6e47f 100644 --- a/packages/url_launcher/url_launcher_windows/windows/system_apis.cc +++ b/packages/url_launcher/url_launcher_windows/windows/system_apis.cpp @@ -9,8 +9,12 @@ namespace url_launcher_plugin { SystemApis::SystemApis() {} +SystemApis::~SystemApis() {} + SystemApisImpl::SystemApisImpl() {} +SystemApisImpl::~SystemApisImpl() {} + LSTATUS SystemApisImpl::RegCloseKey(HKEY key) { return ::RegCloseKey(key); } LSTATUS SystemApisImpl::RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options, diff --git a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp index 0f2b983aaebb..dc5bd824a41b 100644 --- a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp @@ -1,6 +1,170 @@ +#include +#include +#include +#include #include +#include -TEST(TestMe, HelloTestWorld) { - EXPECT_STRNE("hello", "world"); - EXPECT_TRUE(false); +#include +#include + +#include "url_launcher_plugin.h" + +namespace url_launcher_plugin { +namespace test { + +namespace { + +using flutter::EncodableMap; +using flutter::EncodableValue; +using ::testing::DoAll; +using ::testing::Pointee; +using ::testing::Return; +using ::testing::SetArgPointee; + +class MockSystemApis : public SystemApis { + public: + MOCK_METHOD(LSTATUS, RegCloseKey, (HKEY key), (override)); + MOCK_METHOD(LSTATUS, RegQueryValueExW, + (HKEY key, LPCWSTR value_name, LPDWORD type, LPBYTE data, + LPDWORD data_size), + (override)); + MOCK_METHOD(LSTATUS, RegOpenKeyExW, + (HKEY key, LPCWSTR sub_key, DWORD options, REGSAM desired, + PHKEY result), + (override)); + MOCK_METHOD(HINSTANCE, ShellExecuteW, + (HWND hwnd, LPCWSTR operation, LPCWSTR file, LPCWSTR parameters, + LPCWSTR directory, int show_flags), + (override)); +}; + +class MockMethodResult : public flutter::MethodResult<> { + public: + MOCK_METHOD(void, SuccessInternal, (const EncodableValue* result), + (override)); + MOCK_METHOD(void, ErrorInternal, + (const std::string& error_code, const std::string& error_message, + const EncodableValue* details), + (override)); + MOCK_METHOD(void, NotImplementedInternal, (), (override)); +}; + +std::unique_ptr CreateArgumentsWithUrl(const std::string& url) { + EncodableMap args = { + {EncodableValue("url"), EncodableValue(url)}, + }; + return std::make_unique(args); +} + +} // namespace + +TEST(UrlLauncherPlugin, CanLaunchSuccessTrue) { + std::unique_ptr system = std::make_unique(); + std::unique_ptr result = + std::make_unique(); + + // Return success values from the registery commands. + HKEY fake_key = reinterpret_cast(1); + EXPECT_CALL(*system, RegOpenKeyExW) + .WillOnce(DoAll(SetArgPointee<4>(fake_key), Return(ERROR_SUCCESS))); + EXPECT_CALL(*system, RegQueryValueExW).WillOnce(Return(ERROR_SUCCESS)); + EXPECT_CALL(*system, RegCloseKey(fake_key)).WillOnce(Return(ERROR_SUCCESS)); + // Expect a success response. + EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(true)))); + + UrlLauncherPlugin plugin(std::move(system)); + plugin.HandleMethodCall( + flutter::MethodCall( + "canLaunch", CreateArgumentsWithUrl("https://some.url.com") + + ), + std::move(result)); +} + +TEST(UrlLauncherPlugin, CanLaunchQueryFailure) { + std::unique_ptr system = std::make_unique(); + std::unique_ptr result = + std::make_unique(); + + // Return success values from the registery commands, except for the query, + // to simulate a scheme that is in the registry, but has no URL handler. + HKEY fake_key = reinterpret_cast(1); + EXPECT_CALL(*system, RegOpenKeyExW) + .WillOnce(DoAll(SetArgPointee<4>(fake_key), Return(ERROR_SUCCESS))); + EXPECT_CALL(*system, RegQueryValueExW).WillOnce(Return(ERROR_FILE_NOT_FOUND)); + EXPECT_CALL(*system, RegCloseKey(fake_key)).WillOnce(Return(ERROR_SUCCESS)); + // Expect a success response. + EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(false)))); + + UrlLauncherPlugin plugin(std::move(system)); + plugin.HandleMethodCall( + flutter::MethodCall( + "canLaunch", CreateArgumentsWithUrl("https://some.url.com") + + ), + std::move(result)); } + +TEST(UrlLauncherPlugin, CanLaunchHandlesOpenFailure) { + std::unique_ptr system = std::make_unique(); + std::unique_ptr result = + std::make_unique(); + + // Return failure for opening. + EXPECT_CALL(*system, RegOpenKeyExW) + .WillOnce(Return(ERROR_BAD_PATHNAME)); + // Expect a success response. + EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(false)))); + + UrlLauncherPlugin plugin(std::move(system)); + plugin.HandleMethodCall( + flutter::MethodCall( + "canLaunch", CreateArgumentsWithUrl("https://some.url.com") + + ), + std::move(result)); +} + +TEST(UrlLauncherPlugin, LaunchSuccess) { + std::unique_ptr system = std::make_unique(); + std::unique_ptr result = + std::make_unique(); + + // Return a success value (>32) from launching. + EXPECT_CALL(*system, ShellExecuteW) + .WillOnce(Return(reinterpret_cast(33))); + // Expect a success response. + EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(true)))); + + UrlLauncherPlugin plugin(std::move(system)); + plugin.HandleMethodCall( + flutter::MethodCall( + "launch", CreateArgumentsWithUrl("https://some.url.com") + + ), + std::move(result)); +} + +TEST(UrlLauncherPlugin, LaunchReportsFailure) { + std::unique_ptr system = std::make_unique(); + std::unique_ptr result = + std::make_unique(); + + // Return a faile value (<=32) from launching. + EXPECT_CALL(*system, ShellExecuteW) + .WillOnce(Return(reinterpret_cast(32))); + // Expect an error response. + EXPECT_CALL(*result, ErrorInternal); + + UrlLauncherPlugin plugin(std::move(system)); + plugin.HandleMethodCall( + flutter::MethodCall( + "launch", CreateArgumentsWithUrl("https://some.url.com") + + ), + std::move(result)); +} + +} // namespace test +} // namespace url_launcher_plugin diff --git a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp index 815627491e61..748c75ddd243 100644 --- a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.cpp @@ -13,8 +13,6 @@ #include #include -#include "include/url_launcher_windows/url_launcher_plugin.h" - namespace url_launcher_plugin { namespace { @@ -68,8 +66,8 @@ void UrlLauncherPlugin::RegisterWithRegistrar( registrar->messenger(), "plugins.flutter.io/url_launcher", &flutter::StandardMethodCodec::GetInstance()); - // Uses new instead of make_unique due to private constructor. - std::unique_ptr plugin(new UrlLauncherPlugin()); + std::unique_ptr plugin = + std::make_unique(); channel->SetMethodCallHandler( [plugin_pointer = plugin.get()](const auto& call, auto result) { @@ -155,10 +153,3 @@ std::optional UrlLauncherPlugin::LaunchUrl( } } // namespace url_launcher_plugin - -void UrlLauncherPluginRegisterWithRegistrar( - FlutterDesktopPluginRegistrarRef registrar) { - url_launcher_plugin::UrlLauncherPlugin::RegisterWithRegistrar( - flutter::PluginRegistrarManager::GetInstance() - ->GetRegistrar(registrar)); -} diff --git a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h index db39995474d7..45e70e5fc067 100644 --- a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h +++ b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin.h @@ -18,13 +18,6 @@ class UrlLauncherPlugin : public flutter::Plugin { public: static void RegisterWithRegistrar(flutter::PluginRegistrar* registrar); - virtual ~UrlLauncherPlugin(); - - // Disallow copy and move. - UrlLauncherPlugin(const UrlLauncherPlugin&) = delete; - UrlLauncherPlugin& operator=(const UrlLauncherPlugin&) = delete; - - private: UrlLauncherPlugin(); // Creates a plugin instance with the given SystemApi instance. @@ -32,10 +25,17 @@ class UrlLauncherPlugin : public flutter::Plugin { // Exists for unit testing with mock implementations. UrlLauncherPlugin(std::unique_ptr system_apis); - // Called when a method is called on plugin channel; + virtual ~UrlLauncherPlugin(); + + // Disallow copy and move. + UrlLauncherPlugin(const UrlLauncherPlugin&) = delete; + UrlLauncherPlugin& operator=(const UrlLauncherPlugin&) = delete; + + // Called when a method is called on the plugin channel. void HandleMethodCall(const flutter::MethodCall<>& method_call, std::unique_ptr> result); + private: // Returns whether or not the given URL has a registered handler. bool CanLaunchUrl(const std::string& url); diff --git a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp new file mode 100644 index 000000000000..fdc2d8d47f93 --- /dev/null +++ b/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp @@ -0,0 +1,14 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#include + +#include "include/url_launcher_windows/url_launcher_plugin.h" +#include "url_launcher_plugin.h" + +void UrlLauncherPluginRegisterWithRegistrar( + FlutterDesktopPluginRegistrarRef registrar) { + url_launcher_plugin::UrlLauncherPlugin::RegisterWithRegistrar( + flutter::PluginRegistrarManager::GetInstance() + ->GetRegistrar(registrar)); +} From e239b3dac63de0e1a1b0c843e623217e41eee247 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 14 Jul 2021 15:49:15 -0400 Subject: [PATCH 04/10] Tweaks --- .../url_launcher_windows/example/windows/CMakeLists.txt | 1 + .../url_launcher_windows/windows/CMakeLists.txt | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt index d67f4b7b0865..5b1622bcb333 100644 --- a/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/example/windows/CMakeLists.txt @@ -53,6 +53,7 @@ set(include_url_launcher_windows_tests TRUE) # them to the application. include(flutter/generated_plugins.cmake) + # === Installation === # Support files are copied into place next to the executable, so that it can # run in place. This is done instead of making a separate bundle (as on Linux) diff --git a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt index bc7dfc95bd76..21a55be58778 100644 --- a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt @@ -20,7 +20,7 @@ set_target_properties(${PLUGIN_NAME} PROPERTIES CXX_VISIBILITY_PRESET hidden) target_compile_definitions(${PLUGIN_NAME} PRIVATE FLUTTER_PLUGIN_IMPL) target_include_directories(${PLUGIN_NAME} INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/include") -target_link_libraries(${PLUGIN_NAME} PRIVATE flutter_wrapper_plugin) +target_link_libraries(${PLUGIN_NAME} PRIVATE flutter flutter_wrapper_plugin) # List of absolute paths to libraries that should be bundled with the plugin set(file_chooser_bundled_libraries @@ -28,14 +28,15 @@ set(file_chooser_bundled_libraries PARENT_SCOPE ) + # === Tests === if (${include_${PROJECT_NAME}_tests}) set(TEST_RUNNER "${PROJECT_NAME}_test") enable_testing() -# TODO(stuartmorgan): Use a single shared, pre-checked-in googletest instance -# rather than downloading for each plugin. This approach makes sense for a -# template, but not for a monorepo with many plugins. +# TODO(stuartmorgan): Consider using a single shared, pre-checked-in googletest +# instance rather than downloading for each plugin. This approach makes sense +# for a template, but not for a monorepo with many plugins. include(FetchContent) FetchContent_Declare( googletest From ec3b3e2267c8012140208097bb78d582ec3b30e4 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 14 Jul 2021 16:03:59 -0400 Subject: [PATCH 05/10] Update changelog --- packages/url_launcher/url_launcher_windows/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/url_launcher/url_launcher_windows/CHANGELOG.md b/packages/url_launcher/url_launcher_windows/CHANGELOG.md index e906254eef44..4fcdd20298f5 100644 --- a/packages/url_launcher/url_launcher_windows/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_windows/CHANGELOG.md @@ -1,3 +1,7 @@ +## NEXT + +* Added unit tests. + ## 2.0.0 * Migrate to null-safety. From a6a1e8bb2235a2fc874e7834842ab5adac202c8e Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 14 Jul 2021 16:18:52 -0400 Subject: [PATCH 06/10] Improve the filename mess --- .../example/windows/flutter/generated_plugin_registrant.cc | 6 +++--- packages/url_launcher/url_launcher_windows/pubspec.yaml | 2 +- .../url_launcher_windows/windows/CMakeLists.txt | 3 ++- .../{url_launcher_plugin.h => url_launcher_windows.h} | 2 +- ...her_plugin_registration.cpp => url_launcher_windows.cpp} | 5 +++-- 5 files changed, 10 insertions(+), 8 deletions(-) rename packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/{url_launcher_plugin.h => url_launcher_windows.h} (92%) rename packages/url_launcher/url_launcher_windows/windows/{url_launcher_plugin_registration.cpp => url_launcher_windows.cpp} (81%) diff --git a/packages/url_launcher/url_launcher_windows/example/windows/flutter/generated_plugin_registrant.cc b/packages/url_launcher/url_launcher_windows/example/windows/flutter/generated_plugin_registrant.cc index d9fdd53925c5..4f7884874da7 100644 --- a/packages/url_launcher/url_launcher_windows/example/windows/flutter/generated_plugin_registrant.cc +++ b/packages/url_launcher/url_launcher_windows/example/windows/flutter/generated_plugin_registrant.cc @@ -6,9 +6,9 @@ #include "generated_plugin_registrant.h" -#include +#include void RegisterPlugins(flutter::PluginRegistry* registry) { - UrlLauncherPluginRegisterWithRegistrar( - registry->GetRegistrarForPlugin("UrlLauncherPlugin")); + UrlLauncherWindowsRegisterWithRegistrar( + registry->GetRegistrarForPlugin("UrlLauncherWindows")); } diff --git a/packages/url_launcher/url_launcher_windows/pubspec.yaml b/packages/url_launcher/url_launcher_windows/pubspec.yaml index 1a82f3e94a43..d1f9b6361cc1 100644 --- a/packages/url_launcher/url_launcher_windows/pubspec.yaml +++ b/packages/url_launcher/url_launcher_windows/pubspec.yaml @@ -13,7 +13,7 @@ flutter: implements: url_launcher platforms: windows: - pluginClass: UrlLauncherPlugin + pluginClass: UrlLauncherWindows dependencies: flutter: diff --git a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt index 21a55be58778..a4185acff6a1 100644 --- a/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt +++ b/packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt @@ -12,7 +12,8 @@ list(APPEND PLUGIN_SOURCES ) add_library(${PLUGIN_NAME} SHARED - "url_launcher_plugin_registration.cpp" + "include/url_launcher_windows/url_launcher_windows.h" + "url_launcher_windows.cpp" ${PLUGIN_SOURCES} ) apply_standard_settings(${PLUGIN_NAME}) diff --git a/packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/url_launcher_plugin.h b/packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/url_launcher_windows.h similarity index 92% rename from packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/url_launcher_plugin.h rename to packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/url_launcher_windows.h index 8af3924ded81..251471c9fe56 100644 --- a/packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/url_launcher_plugin.h +++ b/packages/url_launcher/url_launcher_windows/windows/include/url_launcher_windows/url_launcher_windows.h @@ -16,7 +16,7 @@ extern "C" { #endif -FLUTTER_PLUGIN_EXPORT void UrlLauncherPluginRegisterWithRegistrar( +FLUTTER_PLUGIN_EXPORT void UrlLauncherWindowsRegisterWithRegistrar( FlutterDesktopPluginRegistrarRef registrar); #if defined(__cplusplus) diff --git a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp b/packages/url_launcher/url_launcher_windows/windows/url_launcher_windows.cpp similarity index 81% rename from packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp rename to packages/url_launcher/url_launcher_windows/windows/url_launcher_windows.cpp index fdc2d8d47f93..05de586d8fe0 100644 --- a/packages/url_launcher/url_launcher_windows/windows/url_launcher_plugin_registration.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/url_launcher_windows.cpp @@ -1,12 +1,13 @@ // Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "include/url_launcher_windows/url_launcher_windows.h" + #include -#include "include/url_launcher_windows/url_launcher_plugin.h" #include "url_launcher_plugin.h" -void UrlLauncherPluginRegisterWithRegistrar( +void UrlLauncherWindowsRegisterWithRegistrar( FlutterDesktopPluginRegistrarRef registrar) { url_launcher_plugin::UrlLauncherPlugin::RegisterWithRegistrar( flutter::PluginRegistrarManager::GetInstance() From ce8bb54d7bba50a11ed419c317ebc580fae28303 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 14 Jul 2021 16:26:43 -0400 Subject: [PATCH 07/10] Format --- .../windows/test/url_launcher_windows_test.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp index dc5bd824a41b..6d76e7b2e4e3 100644 --- a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp @@ -112,8 +112,7 @@ TEST(UrlLauncherPlugin, CanLaunchHandlesOpenFailure) { std::make_unique(); // Return failure for opening. - EXPECT_CALL(*system, RegOpenKeyExW) - .WillOnce(Return(ERROR_BAD_PATHNAME)); + EXPECT_CALL(*system, RegOpenKeyExW).WillOnce(Return(ERROR_BAD_PATHNAME)); // Expect a success response. EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(false)))); From 8542f52858b839cf5707003a19a35fabecac37d3 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Mon, 26 Jul 2021 13:21:15 -0400 Subject: [PATCH 08/10] Missing license --- .../windows/test/url_launcher_windows_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp index 6d76e7b2e4e3..26b71a33877d 100644 --- a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp @@ -1,3 +1,6 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. #include #include #include From cef2f07a1cde67c403f76f9129dd7ad6a5b0abee Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 17 Aug 2021 14:17:52 -0400 Subject: [PATCH 09/10] Fix weird autoformat --- .../windows/test/url_launcher_windows_test.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp index 26b71a33877d..7abd1310858f 100644 --- a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp @@ -78,10 +78,8 @@ TEST(UrlLauncherPlugin, CanLaunchSuccessTrue) { UrlLauncherPlugin plugin(std::move(system)); plugin.HandleMethodCall( - flutter::MethodCall( - "canLaunch", CreateArgumentsWithUrl("https://some.url.com") - - ), + flutter::MethodCall("canLaunch", + CreateArgumentsWithUrl("https://some.url.com")), std::move(result)); } From c5c0a8ff2e1df3cde98413961ba28e4d9f9700cb Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 17 Aug 2021 14:18:20 -0400 Subject: [PATCH 10/10] Fix weird autoformat --- .../test/url_launcher_windows_test.cpp | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp index 7abd1310858f..191d51a0caa8 100644 --- a/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp +++ b/packages/url_launcher/url_launcher_windows/windows/test/url_launcher_windows_test.cpp @@ -100,10 +100,8 @@ TEST(UrlLauncherPlugin, CanLaunchQueryFailure) { UrlLauncherPlugin plugin(std::move(system)); plugin.HandleMethodCall( - flutter::MethodCall( - "canLaunch", CreateArgumentsWithUrl("https://some.url.com") - - ), + flutter::MethodCall("canLaunch", + CreateArgumentsWithUrl("https://some.url.com")), std::move(result)); } @@ -119,10 +117,8 @@ TEST(UrlLauncherPlugin, CanLaunchHandlesOpenFailure) { UrlLauncherPlugin plugin(std::move(system)); plugin.HandleMethodCall( - flutter::MethodCall( - "canLaunch", CreateArgumentsWithUrl("https://some.url.com") - - ), + flutter::MethodCall("canLaunch", + CreateArgumentsWithUrl("https://some.url.com")), std::move(result)); } @@ -139,10 +135,8 @@ TEST(UrlLauncherPlugin, LaunchSuccess) { UrlLauncherPlugin plugin(std::move(system)); plugin.HandleMethodCall( - flutter::MethodCall( - "launch", CreateArgumentsWithUrl("https://some.url.com") - - ), + flutter::MethodCall("launch", + CreateArgumentsWithUrl("https://some.url.com")), std::move(result)); } @@ -159,10 +153,8 @@ TEST(UrlLauncherPlugin, LaunchReportsFailure) { UrlLauncherPlugin plugin(std::move(system)); plugin.HandleMethodCall( - flutter::MethodCall( - "launch", CreateArgumentsWithUrl("https://some.url.com") - - ), + flutter::MethodCall("launch", + CreateArgumentsWithUrl("https://some.url.com")), std::move(result)); }