From 9dc7f75fb768ef0c9444ce937e778c12a6fb5421 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 3 Aug 2020 14:45:11 -0700 Subject: [PATCH 01/10] Added unit tests to the engine. --- shell/common/BUILD.gn | 2 + shell/common/engine.cc | 75 +++++------ shell/common/engine.h | 62 +++++++++ shell/common/engine_unittests.cc | 216 +++++++++++++++++++++++++++++++ 4 files changed, 309 insertions(+), 46 deletions(-) create mode 100644 shell/common/engine_unittests.cc diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 28ca87584f6db..8ef3f1c3c5b41 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -258,6 +258,7 @@ if (enable_unittests) { sources = [ "animator_unittests.cc", "canvas_spy_unittests.cc", + "engine_unittests.cc", "input_events_unittests.cc", "persistent_cache_unittests.cc", "pipeline_unittests.cc", @@ -268,6 +269,7 @@ if (enable_unittests) { deps = [ "//flutter/assets", "//flutter/shell/version", + "//third_party/googletest:gmock", ] public_deps_legacy_and_next = [ ":shell_test_fixture_sources" ] diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 79f885dcbb7ce..b5b3150ef9828 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -28,12 +28,26 @@ namespace flutter { -static constexpr char kAssetChannel[] = "flutter/assets"; -static constexpr char kLifecycleChannel[] = "flutter/lifecycle"; -static constexpr char kNavigationChannel[] = "flutter/navigation"; -static constexpr char kLocalizationChannel[] = "flutter/localization"; -static constexpr char kSettingsChannel[] = "flutter/settings"; -static constexpr char kIsolateChannel[] = "flutter/isolate"; +Engine::Engine( + Delegate& delegate, + const PointerDataDispatcherMaker& dispatcher_maker, + std::shared_ptr image_decoder_task_runner, + TaskRunners task_runners, + Settings settings, + std::unique_ptr animator, + fml::WeakPtr io_manager, + std::unique_ptr runtime_controller) + : delegate_(delegate), + settings_(std::move(settings)), + animator_(std::move(animator)), + runtime_controller_(std::move(runtime_controller)), + activity_running_(true), + have_surface_(false), + image_decoder_(task_runners, image_decoder_task_runner, io_manager), + task_runners_(std::move(task_runners)), + weak_factory_(this) { + pointer_data_dispatcher_ = dispatcher_maker(*this); +} Engine::Engine(Delegate& delegate, const PointerDataDispatcherMaker& dispatcher_maker, @@ -46,19 +60,14 @@ Engine::Engine(Delegate& delegate, fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::WeakPtr snapshot_delegate) - : delegate_(delegate), - settings_(std::move(settings)), - animator_(std::move(animator)), - activity_running_(true), - have_surface_(false), - image_decoder_(task_runners, - vm.GetConcurrentWorkerTaskRunner(), - io_manager), - task_runners_(std::move(task_runners)), - weak_factory_(this) { - // Runtime controller is initialized here because it takes a reference to this - // object as its delegate. The delegate may be called in the constructor and - // we want to be fully initilazed by that point. + : Engine(delegate, + dispatcher_maker, + vm.GetConcurrentWorkerTaskRunner(), + task_runners, + settings, + std::move(animator), + io_manager, + nullptr) { runtime_controller_ = std::make_unique( *this, // runtime delegate &vm, // VM @@ -76,8 +85,6 @@ Engine::Engine(Delegate& delegate, settings_.isolate_shutdown_callback, // isolate shutdown callback settings_.persistent_isolate_data // persistent isolate data ); - - pointer_data_dispatcher_ = dispatcher_maker(*this); } Engine::~Engine() = default; @@ -291,31 +298,7 @@ void Engine::SetViewportMetrics(const ViewportMetrics& metrics) { } void Engine::DispatchPlatformMessage(fml::RefPtr message) { - std::string channel = message->channel(); - if (channel == kLifecycleChannel) { - if (HandleLifecyclePlatformMessage(message.get())) { - return; - } - } else if (channel == kLocalizationChannel) { - if (HandleLocalizationPlatformMessage(message.get())) { - return; - } - } else if (channel == kSettingsChannel) { - HandleSettingsPlatformMessage(message.get()); - return; - } else if (!runtime_controller_->IsRootIsolateRunning() && - channel == kNavigationChannel) { - // If there's no runtime_, we may still need to set the initial route. - HandleNavigationPlatformMessage(std::move(message)); - return; - } - - if (runtime_controller_->IsRootIsolateRunning() && - runtime_controller_->DispatchPlatformMessage(std::move(message))) { - return; - } - - FML_DLOG(WARNING) << "Dropping platform message on channel: " << channel; + DispatchPlatformMessage(message, runtime_controller_.get()); } bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) { diff --git a/shell/common/engine.h b/shell/common/engine.h index bbbcc264c6d42..3e2694a583796 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -248,6 +248,20 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { const std::vector& supported_locale_data) = 0; }; + //---------------------------------------------------------------------------- + /// @brief Creates an instance of the engine with a supplied + /// `RuntimeController`. Use the other constructor except for + /// tests. + /// + Engine(Delegate& delegate, + const PointerDataDispatcherMaker& dispatcher_maker, + std::shared_ptr image_decoder_task_runner, + TaskRunners task_runners, + Settings settings, + std::unique_ptr animator, + fml::WeakPtr io_manager, + std::unique_ptr runtime_controller); + //---------------------------------------------------------------------------- /// @brief Creates an instance of the engine. This is done by the Shell /// on the UI task runner. @@ -663,6 +677,41 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// void DispatchPlatformMessage(fml::RefPtr message); + //---------------------------------------------------------------------------- + /// @brief A version of `DispatchPlatformMessage` that allows delegating + /// to a RuntimeControllerProxy. You probably want the other + /// `DispatchPlatformMessage` unless you are writing tests. + /// + template + void DispatchPlatformMessage(fml::RefPtr message, + RuntimeControllerProxy runtime_controller) { + std::string channel = message->channel(); + if (channel == kLifecycleChannel) { + if (HandleLifecyclePlatformMessage(message.get())) { + return; + } + } else if (channel == kLocalizationChannel) { + if (HandleLocalizationPlatformMessage(message.get())) { + return; + } + } else if (channel == kSettingsChannel) { + HandleSettingsPlatformMessage(message.get()); + return; + } else if (!runtime_controller->IsRootIsolateRunning() && + channel == kNavigationChannel) { + // If there's no runtime_, we may still need to set the initial route. + HandleNavigationPlatformMessage(std::move(message)); + return; + } + + if (runtime_controller->IsRootIsolateRunning() && + runtime_controller->DispatchPlatformMessage(std::move(message))) { + return; + } + + FML_DLOG(WARNING) << "Dropping platform message on channel: " << channel; + } + //---------------------------------------------------------------------------- /// @brief Notifies the engine that the embedder has sent it a pointer /// data packet. A pointer data packet may contain multiple @@ -756,7 +805,20 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// const std::string& GetLastEntrypointLibrary() const; + //---------------------------------------------------------------------------- + /// @brief Getter for the initial route. This can be set with a platform + /// message. + /// + const std::string& InitialRoute() const { return initial_route_; } + private: + static constexpr char kAssetChannel[] = "flutter/assets"; + static constexpr char kLifecycleChannel[] = "flutter/lifecycle"; + static constexpr char kNavigationChannel[] = "flutter/navigation"; + static constexpr char kLocalizationChannel[] = "flutter/localization"; + static constexpr char kSettingsChannel[] = "flutter/settings"; + static constexpr char kIsolateChannel[] = "flutter/isolate"; + Engine::Delegate& delegate_; const Settings settings_; std::unique_ptr animator_; diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc new file mode 100644 index 0000000000000..49274d345aa52 --- /dev/null +++ b/shell/common/engine_unittests.cc @@ -0,0 +1,216 @@ +// 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. +// FLUTTER_NOLINT + +#include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/shell/common/engine.h" +#include "flutter/shell/common/thread_host.h" +#include "flutter/testing/testing.h" +#include "gmock/gmock.h" +#include "rapidjson/document.h" +#include "rapidjson/stringbuffer.h" +#include "rapidjson/writer.h" + +namespace flutter { + +namespace { +class MockDelegate : public Engine::Delegate { + MOCK_METHOD(void, + OnEngineUpdateSemantics, + (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), + (override)); + MOCK_METHOD(void, + OnEngineHandlePlatformMessage, + (fml::RefPtr), + (override)); + MOCK_METHOD(void, OnPreEngineRestart, (), (override)); + MOCK_METHOD(void, + UpdateIsolateDescription, + (const std::string, int64_t), + (override)); + MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); + + MOCK_METHOD(std::unique_ptr>, + ComputePlatformResolvedLocale, + (const std::vector&), + (override)); +}; + +class MockResponse : public PlatformMessageResponse { + public: + MOCK_METHOD(void, Complete, (std::unique_ptr data), (override)); + MOCK_METHOD(void, CompleteEmpty, (), (override)); +}; + +class MockRuntimeController { + public: + MOCK_METHOD(bool, IsRootIsolateRunning, ()); + MOCK_METHOD(bool, DispatchPlatformMessage, (fml::RefPtr)); +}; + +fml::RefPtr MakePlatformMessage( + const std::string& channel, + const std::map& values, + fml::RefPtr response) { + rapidjson::Document document; + auto& allocator = document.GetAllocator(); + document.SetObject(); + + for (const auto& pair : values) { + rapidjson::Value key(pair.first.c_str(), strlen(pair.first.c_str()), + allocator); + rapidjson::Value value(pair.second.c_str(), strlen(pair.second.c_str()), + allocator); + document.AddMember(key, value, allocator); + } + + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + document.Accept(writer); + const uint8_t* data = reinterpret_cast(buffer.GetString()); + + fml::RefPtr message = fml::MakeRefCounted( + channel, std::vector(data, data + buffer.GetSize()), response); + return message; +} + +class EngineTest : public ::testing::Test { + public: + EngineTest() + : thread_host_("EngineTest", + ThreadHost::Type::Platform | ThreadHost::Type::IO | + ThreadHost::Type::UI | ThreadHost::Type::GPU), + task_runners_({ + "EngineTest", + thread_host_.platform_thread->GetTaskRunner(), // platform + thread_host_.raster_thread->GetTaskRunner(), // raster + thread_host_.ui_thread->GetTaskRunner(), // ui + thread_host_.io_thread->GetTaskRunner() // io + }) {} + + void PostUITaskSync(const std::function& function) { + fml::AutoResetWaitableEvent latch; + task_runners_.GetUITaskRunner()->PostTask([&] { + function(); + latch.Signal(); + }); + latch.Wait(); + } + + protected: + void SetUp() override { + dispatcher_maker_ = [](PointerDataDispatcher::Delegate&) { + return nullptr; + }; + } + + MockDelegate delegate_; + PointerDataDispatcherMaker dispatcher_maker_; + ThreadHost thread_host_; + TaskRunners task_runners_; + Settings settings_; + std::unique_ptr animator_; + fml::WeakPtr io_manager_; + std::unique_ptr runtime_controller_; + std::shared_ptr image_decoder_task_runner_; +}; +} // namespace + +TEST_F(EngineTest, Create) { + PostUITaskSync([this] { + auto engine = std::make_unique( + /*delegate=*/delegate_, + /*dispatcher_maker=*/dispatcher_maker_, + /*image_decoder_task_runner=*/image_decoder_task_runner_, + /*task_runners=*/task_runners_, + /*settings=*/settings_, + /*animator=*/std::move(animator_), + /*io_manager=*/io_manager_, + /*runtime_controller=*/std::move(runtime_controller_)); + EXPECT_TRUE(engine); + }); +} + +TEST_F(EngineTest, DispatchPlatformMessageUnknown) { + PostUITaskSync([this] { + auto engine = std::make_unique( + /*delegate=*/delegate_, + /*dispatcher_maker=*/dispatcher_maker_, + /*image_decoder_task_runner=*/image_decoder_task_runner_, + /*task_runners=*/task_runners_, + /*settings=*/settings_, + /*animator=*/std::move(animator_), + /*io_manager=*/io_manager_, + /*runtime_controller=*/std::move(runtime_controller_)); + + fml::RefPtr response = + fml::MakeRefCounted(); + fml::RefPtr message = + fml::MakeRefCounted("foo", response); + MockRuntimeController mock_runtime_controller; + EXPECT_CALL(mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(false)); + engine->DispatchPlatformMessage(message, &mock_runtime_controller); + }); +} + +TEST_F(EngineTest, DispatchPlatformMessageInitialRoute) { + PostUITaskSync([this] { + auto engine = std::make_unique( + /*delegate=*/delegate_, + /*dispatcher_maker=*/dispatcher_maker_, + /*image_decoder_task_runner=*/image_decoder_task_runner_, + /*task_runners=*/task_runners_, + /*settings=*/settings_, + /*animator=*/std::move(animator_), + /*io_manager=*/io_manager_, + /*runtime_controller=*/std::move(runtime_controller_)); + + fml::RefPtr response = + fml::MakeRefCounted(); + std::map values{ + {"method", "setInitialRoute"}, + {"args", "test_initial_route"}, + }; + fml::RefPtr message = + MakePlatformMessage("flutter/navigation", values, response); + MockRuntimeController mock_runtime_controller; + EXPECT_CALL(mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(false)); + engine->DispatchPlatformMessage(message, &mock_runtime_controller); + EXPECT_EQ(engine->InitialRoute(), "test_initial_route"); + }); +} + +TEST_F(EngineTest, DispatchPlatformMessageInitialRouteIgnored) { + PostUITaskSync([this] { + auto engine = std::make_unique( + /*delegate=*/delegate_, + /*dispatcher_maker=*/dispatcher_maker_, + /*image_decoder_task_runner=*/image_decoder_task_runner_, + /*task_runners=*/task_runners_, + /*settings=*/settings_, + /*animator=*/std::move(animator_), + /*io_manager=*/io_manager_, + /*runtime_controller=*/std::move(runtime_controller_)); + + fml::RefPtr response = + fml::MakeRefCounted(); + std::map values{ + {"method", "setInitialRoute"}, + {"args", "test_initial_route"}, + }; + fml::RefPtr message = + MakePlatformMessage("flutter/navigation", values, response); + MockRuntimeController mock_runtime_controller; + EXPECT_CALL(mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(true)); + EXPECT_CALL(mock_runtime_controller, DispatchPlatformMessage(::testing::_)) + .WillRepeatedly(::testing::Return(true)); + engine->DispatchPlatformMessage(message, &mock_runtime_controller); + EXPECT_EQ(engine->InitialRoute(), ""); + }); +} + +} // namespace flutter From 1cff1c5101a5ee60b86ad8bcb99a94ab6e9f9759 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 4 Aug 2020 10:47:37 -0700 Subject: [PATCH 02/10] added missing license check --- ci/licenses_golden/licenses_flutter | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2ec58e6f5ae90..6449f00e4742b 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -600,6 +600,7 @@ FILE: ../../../flutter/shell/common/canvas_spy.h FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc FILE: ../../../flutter/shell/common/engine.cc FILE: ../../../flutter/shell/common/engine.h +FILE: ../../../flutter/shell/common/engine_unittests.cc FILE: ../../../flutter/shell/common/fixtures/shell_test.dart FILE: ../../../flutter/shell/common/fixtures/shelltest_screenshot.png FILE: ../../../flutter/shell/common/input_events_unittests.cc From 7932aa840024a25b227da1421dd26ac771c86f12 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 4 Aug 2020 14:33:40 -0700 Subject: [PATCH 03/10] switched to mocking out the runtimecontroller --- runtime/runtime_controller.cc | 5 ++- runtime/runtime_controller.h | 10 +++-- shell/common/engine.cc | 26 ++++++++++- shell/common/engine_unittests.cc | 77 +++++++++++++++++++++++--------- 4 files changed, 93 insertions(+), 25 deletions(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index bc4d502e6b728..603ae92b9e2f0 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -16,6 +16,9 @@ namespace flutter { +RuntimeController::RuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners) + : client_(client), vm_(nullptr), task_runners_(p_task_runners) {} + RuntimeController::RuntimeController( RuntimeDelegate& p_client, DartVM* p_vm, @@ -113,7 +116,7 @@ bool RuntimeController::IsRootIsolateRunning() const { std::unique_ptr RuntimeController::Clone() const { return std::unique_ptr(new RuntimeController( - client_, // + client_, // vm_, // isolate_snapshot_, // task_runners_, // diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index e67b5847ce0ac..18adbc2c1d720 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -38,7 +38,7 @@ class Window; /// used by the engine to copy the currently accumulated window state so it can /// be referenced by the new runtime controller. /// -class RuntimeController final : public PlatformConfigurationClient { +class RuntimeController : public PlatformConfigurationClient { public: //---------------------------------------------------------------------------- /// @brief Creates a new instance of a runtime controller. This is @@ -340,7 +340,7 @@ class RuntimeController final : public PlatformConfigurationClient { /// /// @return True if root isolate running, False otherwise. /// - bool IsRootIsolateRunning() const; + virtual bool IsRootIsolateRunning() const; //---------------------------------------------------------------------------- /// @brief Dispatch the specified platform message to running root @@ -351,7 +351,7 @@ class RuntimeController final : public PlatformConfigurationClient { /// @return If the message was dispatched to the running root isolate. /// This may fail is an isolate is not running. /// - bool DispatchPlatformMessage(fml::RefPtr message); + virtual bool DispatchPlatformMessage(fml::RefPtr message); //---------------------------------------------------------------------------- /// @brief Dispatch the specified pointer data message to the running @@ -440,6 +440,10 @@ class RuntimeController final : public PlatformConfigurationClient { /// std::pair GetRootIsolateReturnCode(); + protected: + /// Constructor for Mocks. + RuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners); + private: struct Locale { Locale(std::string language_code_, diff --git a/shell/common/engine.cc b/shell/common/engine.cc index b5b3150ef9828..baeb79a1793b8 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -298,7 +298,31 @@ void Engine::SetViewportMetrics(const ViewportMetrics& metrics) { } void Engine::DispatchPlatformMessage(fml::RefPtr message) { - DispatchPlatformMessage(message, runtime_controller_.get()); + std::string channel = message->channel(); + if (channel == kLifecycleChannel) { + if (HandleLifecyclePlatformMessage(message.get())) { + return; + } + } else if (channel == kLocalizationChannel) { + if (HandleLocalizationPlatformMessage(message.get())) { + return; + } + } else if (channel == kSettingsChannel) { + HandleSettingsPlatformMessage(message.get()); + return; + } else if (!runtime_controller_->IsRootIsolateRunning() && + channel == kNavigationChannel) { + // If there's no runtime_, we may still need to set the initial route. + HandleNavigationPlatformMessage(std::move(message)); + return; + } + + if (runtime_controller_->IsRootIsolateRunning() && + runtime_controller_->DispatchPlatformMessage(std::move(message))) { + return; + } + + FML_DLOG(WARNING) << "Dropping platform message on channel: " << channel; } bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) { diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 49274d345aa52..b63dfdd9cee47 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -43,10 +43,41 @@ class MockResponse : public PlatformMessageResponse { MOCK_METHOD(void, CompleteEmpty, (), (override)); }; -class MockRuntimeController { +class MockRuntimeDelegate : public RuntimeDelegate { public: - MOCK_METHOD(bool, IsRootIsolateRunning, ()); - MOCK_METHOD(bool, DispatchPlatformMessage, (fml::RefPtr)); + MOCK_METHOD(std::string, DefaultRouteName, (), (override)); + MOCK_METHOD(void, ScheduleFrame, (bool), (override)); + MOCK_METHOD(void, Render, (std::unique_ptr), (override)); + MOCK_METHOD(void, + UpdateSemantics, + (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), + (override)); + MOCK_METHOD(void, + HandlePlatformMessage, + (fml::RefPtr), + (override)); + MOCK_METHOD(FontCollection&, GetFontCollection, (), (override)); + MOCK_METHOD(void, + UpdateIsolateDescription, + (const std::string, int64_t), + (override)); + MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); + + MOCK_METHOD(std::unique_ptr>, + ComputePlatformResolvedLocale, + (const std::vector&), + (override)); +}; + +class MockRuntimeController : public RuntimeController { + public: + MockRuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners) + : RuntimeController(client, p_task_runners) {} + MOCK_METHOD(bool, IsRootIsolateRunning, (), (override, const)); + MOCK_METHOD(bool, + DispatchPlatformMessage, + (fml::RefPtr), + (override)); }; fml::RefPtr MakePlatformMessage( @@ -134,6 +165,11 @@ TEST_F(EngineTest, Create) { TEST_F(EngineTest, DispatchPlatformMessageUnknown) { PostUITaskSync([this] { + MockRuntimeDelegate client; + auto mock_runtime_controller = + std::make_unique(client, task_runners_); + EXPECT_CALL(*mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(false)); auto engine = std::make_unique( /*delegate=*/delegate_, /*dispatcher_maker=*/dispatcher_maker_, @@ -142,21 +178,23 @@ TEST_F(EngineTest, DispatchPlatformMessageUnknown) { /*settings=*/settings_, /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, - /*runtime_controller=*/std::move(runtime_controller_)); + /*runtime_controller=*/std::move(mock_runtime_controller)); fml::RefPtr response = fml::MakeRefCounted(); fml::RefPtr message = fml::MakeRefCounted("foo", response); - MockRuntimeController mock_runtime_controller; - EXPECT_CALL(mock_runtime_controller, IsRootIsolateRunning()) - .WillRepeatedly(::testing::Return(false)); - engine->DispatchPlatformMessage(message, &mock_runtime_controller); + engine->DispatchPlatformMessage(message); }); } TEST_F(EngineTest, DispatchPlatformMessageInitialRoute) { PostUITaskSync([this] { + MockRuntimeDelegate client; + auto mock_runtime_controller = + std::make_unique(client, task_runners_); + EXPECT_CALL(*mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(false)); auto engine = std::make_unique( /*delegate=*/delegate_, /*dispatcher_maker=*/dispatcher_maker_, @@ -165,7 +203,7 @@ TEST_F(EngineTest, DispatchPlatformMessageInitialRoute) { /*settings=*/settings_, /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, - /*runtime_controller=*/std::move(runtime_controller_)); + /*runtime_controller=*/std::move(mock_runtime_controller)); fml::RefPtr response = fml::MakeRefCounted(); @@ -175,16 +213,20 @@ TEST_F(EngineTest, DispatchPlatformMessageInitialRoute) { }; fml::RefPtr message = MakePlatformMessage("flutter/navigation", values, response); - MockRuntimeController mock_runtime_controller; - EXPECT_CALL(mock_runtime_controller, IsRootIsolateRunning()) - .WillRepeatedly(::testing::Return(false)); - engine->DispatchPlatformMessage(message, &mock_runtime_controller); + engine->DispatchPlatformMessage(message); EXPECT_EQ(engine->InitialRoute(), "test_initial_route"); }); } TEST_F(EngineTest, DispatchPlatformMessageInitialRouteIgnored) { PostUITaskSync([this] { + MockRuntimeDelegate client; + auto mock_runtime_controller = + std::make_unique(client, task_runners_); + EXPECT_CALL(*mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(true)); + EXPECT_CALL(*mock_runtime_controller, DispatchPlatformMessage(::testing::_)) + .WillRepeatedly(::testing::Return(true)); auto engine = std::make_unique( /*delegate=*/delegate_, /*dispatcher_maker=*/dispatcher_maker_, @@ -193,7 +235,7 @@ TEST_F(EngineTest, DispatchPlatformMessageInitialRouteIgnored) { /*settings=*/settings_, /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, - /*runtime_controller=*/std::move(runtime_controller_)); + /*runtime_controller=*/std::move(mock_runtime_controller)); fml::RefPtr response = fml::MakeRefCounted(); @@ -203,12 +245,7 @@ TEST_F(EngineTest, DispatchPlatformMessageInitialRouteIgnored) { }; fml::RefPtr message = MakePlatformMessage("flutter/navigation", values, response); - MockRuntimeController mock_runtime_controller; - EXPECT_CALL(mock_runtime_controller, IsRootIsolateRunning()) - .WillRepeatedly(::testing::Return(true)); - EXPECT_CALL(mock_runtime_controller, DispatchPlatformMessage(::testing::_)) - .WillRepeatedly(::testing::Return(true)); - engine->DispatchPlatformMessage(message, &mock_runtime_controller); + engine->DispatchPlatformMessage(message); EXPECT_EQ(engine->InitialRoute(), ""); }); } From b39f6a01ebfdd80e168bcf323a23edd53ca6dacd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 4 Aug 2020 14:47:18 -0700 Subject: [PATCH 04/10] added space --- runtime/runtime_controller.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 603ae92b9e2f0..b2c3b7d72063d 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -116,7 +116,7 @@ bool RuntimeController::IsRootIsolateRunning() const { std::unique_ptr RuntimeController::Clone() const { return std::unique_ptr(new RuntimeController( - client_, // + client_, // vm_, // isolate_snapshot_, // task_runners_, // From cf87907c9c53b922729f6ed6436dcc46fab89210 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 4 Aug 2020 14:49:46 -0700 Subject: [PATCH 05/10] removed unused method --- shell/common/engine.h | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/shell/common/engine.h b/shell/common/engine.h index 3e2694a583796..becfcd3d9adf4 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -677,41 +677,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// void DispatchPlatformMessage(fml::RefPtr message); - //---------------------------------------------------------------------------- - /// @brief A version of `DispatchPlatformMessage` that allows delegating - /// to a RuntimeControllerProxy. You probably want the other - /// `DispatchPlatformMessage` unless you are writing tests. - /// - template - void DispatchPlatformMessage(fml::RefPtr message, - RuntimeControllerProxy runtime_controller) { - std::string channel = message->channel(); - if (channel == kLifecycleChannel) { - if (HandleLifecyclePlatformMessage(message.get())) { - return; - } - } else if (channel == kLocalizationChannel) { - if (HandleLocalizationPlatformMessage(message.get())) { - return; - } - } else if (channel == kSettingsChannel) { - HandleSettingsPlatformMessage(message.get()); - return; - } else if (!runtime_controller->IsRootIsolateRunning() && - channel == kNavigationChannel) { - // If there's no runtime_, we may still need to set the initial route. - HandleNavigationPlatformMessage(std::move(message)); - return; - } - - if (runtime_controller->IsRootIsolateRunning() && - runtime_controller->DispatchPlatformMessage(std::move(message))) { - return; - } - - FML_DLOG(WARNING) << "Dropping platform message on channel: " << channel; - } - //---------------------------------------------------------------------------- /// @brief Notifies the engine that the embedder has sent it a pointer /// data packet. A pointer data packet may contain multiple From d352848ae52c5f64d654929c4a9f5bee4e64ec32 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 4 Aug 2020 14:52:42 -0700 Subject: [PATCH 06/10] moved the channel names back into .cc --- shell/common/engine.cc | 7 +++++++ shell/common/engine.h | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/common/engine.cc b/shell/common/engine.cc index baeb79a1793b8..d4338c663be06 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -28,6 +28,13 @@ namespace flutter { +static constexpr char kAssetChannel[] = "flutter/assets"; +static constexpr char kLifecycleChannel[] = "flutter/lifecycle"; +static constexpr char kNavigationChannel[] = "flutter/navigation"; +static constexpr char kLocalizationChannel[] = "flutter/localization"; +static constexpr char kSettingsChannel[] = "flutter/settings"; +static constexpr char kIsolateChannel[] = "flutter/isolate"; + Engine::Engine( Delegate& delegate, const PointerDataDispatcherMaker& dispatcher_maker, diff --git a/shell/common/engine.h b/shell/common/engine.h index becfcd3d9adf4..d16ae0ffed2c7 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -777,13 +777,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { const std::string& InitialRoute() const { return initial_route_; } private: - static constexpr char kAssetChannel[] = "flutter/assets"; - static constexpr char kLifecycleChannel[] = "flutter/lifecycle"; - static constexpr char kNavigationChannel[] = "flutter/navigation"; - static constexpr char kLocalizationChannel[] = "flutter/localization"; - static constexpr char kSettingsChannel[] = "flutter/settings"; - static constexpr char kIsolateChannel[] = "flutter/isolate"; - Engine::Delegate& delegate_; const Settings settings_; std::unique_ptr animator_; From 3a586fc908792a5c76d6d3a0cafdcc77a36ddd7d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 5 Aug 2020 10:02:43 -0700 Subject: [PATCH 07/10] ran formatter --- runtime/runtime_controller.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index b2c3b7d72063d..f66cfed778660 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -16,7 +16,8 @@ namespace flutter { -RuntimeController::RuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners) +RuntimeController::RuntimeController(RuntimeDelegate& client, + TaskRunners p_task_runners) : client_(client), vm_(nullptr), task_runners_(p_task_runners) {} RuntimeController::RuntimeController( From f5d6a2bc9807cca8d55f940c9a695e1729cd7643 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 5 Aug 2020 13:28:57 -0700 Subject: [PATCH 08/10] tried to fix windows builds --- shell/common/engine_unittests.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index b63dfdd9cee47..a7ec5d19e0603 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -12,6 +12,12 @@ #include "rapidjson/stringbuffer.h" #include "rapidjson/writer.h" +// This is a hack to get gmock working on windows until a fix lands for: +// https://github.com/google/googletest/issues/2490 +#undef GTEST_STRINGIFY_ +#define GTEST_STRINGIFY_HELPER_(name, ...) #name +#define GTEST_STRINGIFY_(...) GTEST_STRINGIFY_HELPER_(__VA_ARGS__,) + namespace flutter { namespace { From 6e6bffdfb54ff9501dc8c044daccbe0a7e8602b1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 5 Aug 2020 13:36:27 -0700 Subject: [PATCH 09/10] formatter --- shell/common/engine_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index a7ec5d19e0603..e27c41e4c8f87 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -16,7 +16,7 @@ // https://github.com/google/googletest/issues/2490 #undef GTEST_STRINGIFY_ #define GTEST_STRINGIFY_HELPER_(name, ...) #name -#define GTEST_STRINGIFY_(...) GTEST_STRINGIFY_HELPER_(__VA_ARGS__,) +#define GTEST_STRINGIFY_(...) GTEST_STRINGIFY_HELPER_(__VA_ARGS__, ) namespace flutter { From 31be985b5f390b06f506c4303e7b82ea8ff5515b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 6 Aug 2020 14:01:54 -0700 Subject: [PATCH 10/10] switched to deprecated mock declarations to support windows bug --- shell/common/engine_unittests.cc | 79 +++++++++++--------------------- 1 file changed, 27 insertions(+), 52 deletions(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index e27c41e4c8f87..54f6f00d7a8f6 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -12,78 +12,53 @@ #include "rapidjson/stringbuffer.h" #include "rapidjson/writer.h" -// This is a hack to get gmock working on windows until a fix lands for: +///\note Deprecated MOCK_METHOD macros used until this issue is resolved: // https://github.com/google/googletest/issues/2490 -#undef GTEST_STRINGIFY_ -#define GTEST_STRINGIFY_HELPER_(name, ...) #name -#define GTEST_STRINGIFY_(...) GTEST_STRINGIFY_HELPER_(__VA_ARGS__, ) namespace flutter { namespace { class MockDelegate : public Engine::Delegate { - MOCK_METHOD(void, - OnEngineUpdateSemantics, - (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), - (override)); - MOCK_METHOD(void, - OnEngineHandlePlatformMessage, - (fml::RefPtr), - (override)); - MOCK_METHOD(void, OnPreEngineRestart, (), (override)); - MOCK_METHOD(void, - UpdateIsolateDescription, - (const std::string, int64_t), - (override)); - MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); - - MOCK_METHOD(std::unique_ptr>, - ComputePlatformResolvedLocale, - (const std::vector&), - (override)); + MOCK_METHOD2(OnEngineUpdateSemantics, + void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates)); + MOCK_METHOD1(OnEngineHandlePlatformMessage, + void(fml::RefPtr)); + MOCK_METHOD0(OnPreEngineRestart, void()); + MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t)); + MOCK_METHOD1(SetNeedsReportTimings, void(bool)); + MOCK_METHOD1(ComputePlatformResolvedLocale, + std::unique_ptr>( + const std::vector&)); }; class MockResponse : public PlatformMessageResponse { public: - MOCK_METHOD(void, Complete, (std::unique_ptr data), (override)); - MOCK_METHOD(void, CompleteEmpty, (), (override)); + MOCK_METHOD1(Complete, void(std::unique_ptr data)); + MOCK_METHOD0(CompleteEmpty, void()); }; class MockRuntimeDelegate : public RuntimeDelegate { public: - MOCK_METHOD(std::string, DefaultRouteName, (), (override)); - MOCK_METHOD(void, ScheduleFrame, (bool), (override)); - MOCK_METHOD(void, Render, (std::unique_ptr), (override)); - MOCK_METHOD(void, - UpdateSemantics, - (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), - (override)); - MOCK_METHOD(void, - HandlePlatformMessage, - (fml::RefPtr), - (override)); - MOCK_METHOD(FontCollection&, GetFontCollection, (), (override)); - MOCK_METHOD(void, - UpdateIsolateDescription, - (const std::string, int64_t), - (override)); - MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override)); - - MOCK_METHOD(std::unique_ptr>, - ComputePlatformResolvedLocale, - (const std::vector&), - (override)); + MOCK_METHOD0(DefaultRouteName, std::string()); + MOCK_METHOD1(ScheduleFrame, void(bool)); + MOCK_METHOD1(Render, void(std::unique_ptr)); + MOCK_METHOD2(UpdateSemantics, + void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates)); + MOCK_METHOD1(HandlePlatformMessage, void(fml::RefPtr)); + MOCK_METHOD0(GetFontCollection, FontCollection&()); + MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t)); + MOCK_METHOD1(SetNeedsReportTimings, void(bool)); + MOCK_METHOD1(ComputePlatformResolvedLocale, + std::unique_ptr>( + const std::vector&)); }; class MockRuntimeController : public RuntimeController { public: MockRuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners) : RuntimeController(client, p_task_runners) {} - MOCK_METHOD(bool, IsRootIsolateRunning, (), (override, const)); - MOCK_METHOD(bool, - DispatchPlatformMessage, - (fml::RefPtr), - (override)); + MOCK_CONST_METHOD0(IsRootIsolateRunning, bool()); + MOCK_METHOD1(DispatchPlatformMessage, bool(fml::RefPtr)); }; fml::RefPtr MakePlatformMessage(