From e282bf6c16de6d73527dedd8d20b0f123df650d8 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Wed, 11 Nov 2020 12:57:08 -0800 Subject: [PATCH 1/8] Update 1.22 engine to use Dart 2.10.4 --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index e10de2a338be4..9f8a48239d314 100644 --- a/DEPS +++ b/DEPS @@ -34,7 +34,7 @@ vars = { # Dart is: https://github.com/dart-lang/sdk/blob/master/DEPS. # You can use //tools/dart/create_updated_flutter_deps.py to produce # updated revision list of existing dependencies. - 'dart_revision': 'ecf9ce8ef42de11033801b2870e8b310c3722902', + 'dart_revision': '7c148d029de32590a8d0d332bf807d25929f080e', # WARNING: DO NOT EDIT MANUALLY # The lines between blank lines above and below are generated by a script. See create_updated_flutter_deps.py From e3e5aa6eadb1e2426a4cb1d37a7905483418ff39 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 21 Sep 2020 15:27:02 -0700 Subject: [PATCH 2/8] [iOS TextInput] Avoid Unnecessary UndateEditingClient Calls (#21303) --- .../Source/FlutterTextInputPlugin.mm | 18 ++++----- .../Source/FlutterTextInputPluginTest.m | 40 ++++++++++++++----- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index ca276ab990471..206f1b1219a65 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -521,6 +521,8 @@ - (void)setTextInputClient:(int)client { } // Return true if the new input state needs to be synced back to the framework. +// TODO(LongCatIsLooong): setTextInputState should never call updateEditingState. Sending the +// editing value back may overwrite the framework's updated editing value. - (BOOL)setTextInputState:(NSDictionary*)state { NSString* newText = state[@"text"]; BOOL textChanged = ![self.text isEqualToString:newText]; @@ -528,19 +530,14 @@ - (BOOL)setTextInputState:(NSDictionary*)state { [self.inputDelegate textWillChange:self]; [self.text setString:newText]; } - BOOL needsEditingStateUpdate = textChanged; NSInteger composingBase = [state[@"composingBase"] intValue]; NSInteger composingExtent = [state[@"composingExtent"] intValue]; NSRange composingRange = [self clampSelection:NSMakeRange(MIN(composingBase, composingExtent), ABS(composingBase - composingExtent)) forText:self.text]; - FlutterTextRange* newMarkedRange = + + self.markedTextRange = composingRange.length > 0 ? [FlutterTextRange rangeWithNSRange:composingRange] : nil; - needsEditingStateUpdate = - needsEditingStateUpdate || - (!newMarkedRange ? self.markedTextRange != nil - : ![newMarkedRange isEqualTo:(FlutterTextRange*)self.markedTextRange]); - self.markedTextRange = newMarkedRange; NSRange selectedRange = [self clampSelectionFromBase:[state[@"selectionBase"] intValue] extent:[state[@"selectionExtent"] intValue] @@ -548,7 +545,6 @@ - (BOOL)setTextInputState:(NSDictionary*)state { NSRange oldSelectedRange = [(FlutterTextRange*)self.selectedTextRange range]; if (!NSEqualRanges(selectedRange, oldSelectedRange)) { - needsEditingStateUpdate = YES; [self.inputDelegate selectionWillChange:self]; [self setSelectedTextRangeLocal:[FlutterTextRange rangeWithNSRange:selectedRange]]; @@ -563,8 +559,8 @@ - (BOOL)setTextInputState:(NSDictionary*)state { [self.inputDelegate textDidChange:self]; } - // For consistency with Android behavior, send an update to the framework if anything changed. - return needsEditingStateUpdate; + // For consistency with Android behavior, send an update to the framework if the text changed. + return textChanged; } // Extracts the selection information from the editing state dictionary. @@ -771,6 +767,8 @@ - (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelecte } - (void)unmarkText { + if (!self.markedTextRange) + return; self.markedTextRange = nil; [self updateEditingState]; } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m index f62f665fbc0c8..b601007834a25 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m @@ -216,7 +216,7 @@ - (void)testTextChangesTriggerUpdateEditingClient { XCTAssertFalse([inputView setTextInputState:@{@"text" : @"AFTER"}]); } -- (void)testSelectionChangeTriggersUpdateEditingClient { +- (void)testSelectionChangeDoesNotTriggerUpdateEditingClient { FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; inputView.textInputDelegate = engine; @@ -226,15 +226,15 @@ - (void)testSelectionChangeTriggersUpdateEditingClient { BOOL shouldUpdate = [inputView setTextInputState:@{@"text" : @"SELECTION", @"selectionBase" : @0, @"selectionExtent" : @3}]; - XCTAssertTrue(shouldUpdate); + XCTAssertFalse(shouldUpdate); shouldUpdate = [inputView setTextInputState:@{@"text" : @"SELECTION", @"selectionBase" : @1, @"selectionExtent" : @3}]; - XCTAssertTrue(shouldUpdate); + XCTAssertFalse(shouldUpdate); shouldUpdate = [inputView setTextInputState:@{@"text" : @"SELECTION", @"selectionBase" : @1, @"selectionExtent" : @2}]; - XCTAssertTrue(shouldUpdate); + XCTAssertFalse(shouldUpdate); // Don't send anything if there's nothing new. shouldUpdate = [inputView @@ -242,7 +242,7 @@ - (void)testSelectionChangeTriggersUpdateEditingClient { XCTAssertFalse(shouldUpdate); } -- (void)testComposingChangeTriggersUpdateEditingClient { +- (void)testComposingChangeDoesNotTriggerUpdateEditingClient { FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; inputView.textInputDelegate = engine; @@ -253,22 +253,44 @@ - (void)testComposingChangeTriggersUpdateEditingClient { BOOL shouldUpdate = [inputView setTextInputState:@{@"text" : @"COMPOSING", @"composingBase" : @0, @"composingExtent" : @3}]; - XCTAssertTrue(shouldUpdate); + XCTAssertFalse(shouldUpdate); shouldUpdate = [inputView setTextInputState:@{@"text" : @"COMPOSING", @"composingBase" : @1, @"composingExtent" : @3}]; - XCTAssertTrue(shouldUpdate); + XCTAssertFalse(shouldUpdate); shouldUpdate = [inputView setTextInputState:@{@"text" : @"COMPOSING", @"composingBase" : @1, @"composingExtent" : @2}]; - XCTAssertTrue(shouldUpdate); + XCTAssertFalse(shouldUpdate); - // Don't send anything if there's nothing new. shouldUpdate = [inputView setTextInputState:@{@"text" : @"COMPOSING", @"composingBase" : @1, @"composingExtent" : @2}]; XCTAssertFalse(shouldUpdate); } +- (void)testUITextInputAvoidUnnecessaryUndateEditingClientCalls { + FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; + inputView.textInputDelegate = engine; + + __block int updateCount = 0; + OCMStub([engine updateEditingClient:0 withState:[OCMArg isNotNil]]) + .andDo(^(NSInvocation* invocation) { + updateCount++; + }); + + [inputView unmarkText]; + // updateEditingClient shouldn't fire as the text is already unmarked. + XCTAssertEqual(updateCount, 0); + + [inputView setMarkedText:@"marked text" selectedRange:NSMakeRange(0, 1)]; + // updateEditingClient fires in response to setMarkedText. + XCTAssertEqual(updateCount, 1); + + [inputView unmarkText]; + // updateEditingClient fires in response to unmarkText. + XCTAssertEqual(updateCount, 2); +} + - (void)testUpdateEditingClientNegativeSelection { FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; inputView.textInputDelegate = engine; From 9c7440b5db45242f579b9176fdc26952f1722b6d Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Wed, 4 Nov 2020 13:56:29 -0800 Subject: [PATCH 3/8] added unit tests to the rasterizer (#22282) --- ci/licenses_golden/licenses_flutter | 1 + shell/common/BUILD.gn | 1 + shell/common/rasterizer_unittests.cc | 135 +++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 shell/common/rasterizer_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 7200fbee991c2..87ce3cf0d8751 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -620,6 +620,7 @@ FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc FILE: ../../../flutter/shell/common/pointer_data_dispatcher.h FILE: ../../../flutter/shell/common/rasterizer.cc FILE: ../../../flutter/shell/common/rasterizer.h +FILE: ../../../flutter/shell/common/rasterizer_unittests.cc FILE: ../../../flutter/shell/common/run_configuration.cc FILE: ../../../flutter/shell/common/run_configuration.h FILE: ../../../flutter/shell/common/serialization_callbacks.cc diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index f377d643ff1c5..3b59b0b6e7149 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -252,6 +252,7 @@ if (enable_unittests) { "input_events_unittests.cc", "persistent_cache_unittests.cc", "pipeline_unittests.cc", + "rasterizer_unittests.cc", "shell_unittests.cc", "skp_shader_warmup_unittests.cc", ] diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc new file mode 100644 index 0000000000000..47a1f700fc843 --- /dev/null +++ b/shell/common/rasterizer_unittests.cc @@ -0,0 +1,135 @@ +// 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 "flutter/shell/common/rasterizer.h" + +#include "flutter/shell/common/thread_host.h" +#include "flutter/testing/testing.h" +#include "gmock/gmock.h" + +using testing::_; +using testing::Return; +using testing::ReturnRef; + +namespace flutter { +namespace { +class MockDelegate : public Rasterizer::Delegate { + public: + MOCK_METHOD1(OnFrameRasterized, void(const FrameTiming& frame_timing)); + MOCK_METHOD0(GetFrameBudget, fml::Milliseconds()); + MOCK_CONST_METHOD0(GetLatestFrameTargetTime, fml::TimePoint()); + MOCK_CONST_METHOD0(GetTaskRunners, const TaskRunners&()); + MOCK_CONST_METHOD0(GetIsGpuDisabledSyncSwitch, + std::shared_ptr()); +}; + +class MockSurface : public Surface { + public: + MOCK_METHOD0(IsValid, bool()); + MOCK_METHOD1(AcquireFrame, + std::unique_ptr(const SkISize& size)); + MOCK_CONST_METHOD0(GetRootTransformation, SkMatrix()); + MOCK_METHOD0(GetContext, GrDirectContext*()); + MOCK_METHOD0(GetExternalViewEmbedder, ExternalViewEmbedder*()); + MOCK_METHOD0(MakeRenderContextCurrent, std::unique_ptr()); + MOCK_METHOD0(ClearRenderContext, bool()); +}; + +class MockExternalViewEmbedder : public ExternalViewEmbedder { + public: + MOCK_METHOD0(GetRootCanvas, SkCanvas*()); + MOCK_METHOD0(CancelFrame, void()); + MOCK_METHOD4(BeginFrame, + void(SkISize frame_size, + GrDirectContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger)); + MOCK_METHOD2(PrerollCompositeEmbeddedView, + void(int view_id, std::unique_ptr params)); + MOCK_METHOD1(PostPrerollAction, + PostPrerollResult( + fml::RefPtr raster_thread_merger)); + MOCK_METHOD0(GetCurrentCanvases, std::vector()); + MOCK_METHOD1(CompositeEmbeddedView, SkCanvas*(int view_id)); + MOCK_METHOD2(SubmitFrame, + void(GrDirectContext* context, + std::unique_ptr frame)); + MOCK_METHOD2(EndFrame, + void(bool should_resubmit_frame, + fml::RefPtr raster_thread_merger)); + MOCK_METHOD0(SupportsDynamicThreadMerging, bool()); +}; +} // namespace + +TEST(RasterizerTest, create) { + MockDelegate delegate; + auto rasterizer = std::make_unique(delegate); + EXPECT_TRUE(rasterizer != nullptr); +} + +TEST(RasterizerTest, drawEmptyPipeline) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::GPU | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + ON_CALL(delegate, GetTaskRunners()).WillByDefault(ReturnRef(task_runners)); + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(); + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = fml::AdoptRef(new Pipeline(/*depth=*/10)); + rasterizer->Draw(pipeline, nullptr); + latch.Signal(); + }); + latch.Wait(); +} + +TEST(RasterizerTest, drawWithExternalViewEmbedder) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::GPU | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)); + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(); + MockExternalViewEmbedder external_view_embedder; + EXPECT_CALL(*surface, GetExternalViewEmbedder()) + .WillRepeatedly(Return(&external_view_embedder)); + EXPECT_CALL(external_view_embedder, + BeginFrame(SkISize(), nullptr, 2.0, + fml::RefPtr(nullptr))); + EXPECT_CALL(external_view_embedder, + EndFrame(false, fml::RefPtr(nullptr))); + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = fml::AdoptRef(new Pipeline(/*depth=*/10)); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + std::function no_discard = [](LayerTree&) { + return false; + }; + rasterizer->Draw(pipeline, no_discard); + latch.Signal(); + }); + latch.Wait(); +} +} // namespace flutter From a53a2858bb1511075dfd61203ddd3abd9e377f7d Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 6 Nov 2020 21:04:02 -0800 Subject: [PATCH 4/8] Reland "Do not involve external_view_embedder in submit frame process if threads are not merged. #22275" (#22372) --- shell/common/rasterizer.cc | 6 +- shell/common/rasterizer_unittests.cc | 143 +++++++++++++++++- shell/common/shell_unittests.cc | 24 ++- .../framework/Source/FlutterPlatformViews.mm | 25 +-- .../Source/FlutterPlatformViewsTest.mm | 62 ++++++++ 5 files changed, 231 insertions(+), 29 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index c3f8001ee24e4..bb1b2a001f2ea 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -186,8 +186,7 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { consume_result = PipelineConsumeResult::MoreAvailable; } - // Merging the thread as we know the next `Draw` should be run on the platform - // thread. + // EndFrame should perform cleanups for the external_view_embedder. if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) { surface_->GetExternalViewEmbedder()->EndFrame(should_resubmit_frame, raster_thread_merger_); @@ -464,7 +463,8 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) { raster_status == RasterStatus::kSkipAndRetry) { return raster_status; } - if (external_view_embedder != nullptr) { + if (external_view_embedder != nullptr && + (!raster_thread_merger_ || raster_thread_merger_->IsMerged())) { FML_DCHECK(!frame->IsSubmitted()); external_view_embedder->SubmitFrame(surface_->GetContext(), std::move(frame)); diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index 47a1f700fc843..2ccc9ca706bbb 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#define FML_USED_ON_EMBEDDER + #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/common/thread_host.h" @@ -9,6 +11,7 @@ #include "gmock/gmock.h" using testing::_; +using testing::ByMove; using testing::Return; using testing::ReturnRef; @@ -92,7 +95,8 @@ TEST(RasterizerTest, drawEmptyPipeline) { latch.Wait(); } -TEST(RasterizerTest, drawWithExternalViewEmbedder) { +TEST(RasterizerTest, + drawWithExternalViewEmbedderExternalViewEmbedderSubmitFrameCalled) { std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); ThreadHost thread_host("io.flutter.test." + test_name + ".", @@ -108,14 +112,85 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) { EXPECT_CALL(delegate, OnFrameRasterized(_)); auto rasterizer = std::make_unique(delegate); auto surface = std::make_unique(); + MockExternalViewEmbedder external_view_embedder; EXPECT_CALL(*surface, GetExternalViewEmbedder()) .WillRepeatedly(Return(&external_view_embedder)); + + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AcquireFrame(SkISize())) + .WillOnce(Return(ByMove(std::move(surface_frame)))); + EXPECT_CALL(external_view_embedder, - BeginFrame(SkISize(), nullptr, 2.0, - fml::RefPtr(nullptr))); + BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr, + /*device_pixel_ratio=*/2.0, + /*raster_thread_merger=*/ + fml::RefPtr(nullptr))) + .Times(1); + EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1); + EXPECT_CALL( + external_view_embedder, + EndFrame(/*should_resubmit_frame=*/false, + /*raster_thread_merger=*/fml::RefPtr( + nullptr))) + .Times(1); + + rasterizer->Setup(std::move(surface)); + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = fml::AdoptRef(new Pipeline(/*depth=*/10)); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + auto no_discard = [](LayerTree&) { return false; }; + rasterizer->Draw(pipeline, no_discard); + latch.Signal(); + }); + latch.Wait(); +} + +TEST( + RasterizerTest, + drawWithExternalViewEmbedderAndThreadMergerNotMergedExternalViewEmbedderSubmitFrameNotCalled) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::GPU | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)); + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(); + MockExternalViewEmbedder external_view_embedder; + EXPECT_CALL(*surface, GetExternalViewEmbedder()) + .WillRepeatedly(Return(&external_view_embedder)); + EXPECT_CALL(external_view_embedder, SupportsDynamicThreadMerging) + .WillRepeatedly(Return(true)); + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AcquireFrame(SkISize())) + .WillOnce(Return(ByMove(std::move(surface_frame)))); + EXPECT_CALL(external_view_embedder, - EndFrame(false, fml::RefPtr(nullptr))); + BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr, + /*device_pixel_ratio=*/2.0, + /*raster_thread_merger=*/_)) + .Times(1); + EXPECT_CALL(external_view_embedder, SubmitFrame).Times(0); + EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, + /*raster_thread_merger=*/_)) + .Times(1); + rasterizer->Setup(std::move(surface)); fml::AutoResetWaitableEvent latch; thread_host.raster_thread->GetTaskRunner()->PostTask([&] { @@ -124,12 +199,66 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) { /*device_pixel_ratio=*/2.0f); bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); - std::function no_discard = [](LayerTree&) { - return false; - }; + auto no_discard = [](LayerTree&) { return false; }; rasterizer->Draw(pipeline, no_discard); latch.Signal(); }); latch.Wait(); } + +TEST( + RasterizerTest, + drawWithExternalViewEmbedderAndThreadsMergedExternalViewEmbedderSubmitFrameCalled) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::GPU | + ThreadHost::Type::IO | ThreadHost::Type::UI); + fml::MessageLoop::EnsureInitializedForCurrentThread(); + TaskRunners task_runners("test", + fml::MessageLoop::GetCurrent().GetTaskRunner(), + fml::MessageLoop::GetCurrent().GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + EXPECT_CALL(delegate, OnFrameRasterized(_)); + + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(); + + MockExternalViewEmbedder external_view_embedder; + EXPECT_CALL(*surface, GetExternalViewEmbedder()) + .WillRepeatedly(Return(&external_view_embedder)); + + auto surface_frame = std::make_unique( + /*surface=*/nullptr, /*supports_readback=*/true, + /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; }); + EXPECT_CALL(*surface, AcquireFrame(SkISize())) + .WillOnce(Return(ByMove(std::move(surface_frame)))); + EXPECT_CALL(external_view_embedder, SupportsDynamicThreadMerging) + .WillRepeatedly(Return(true)); + + EXPECT_CALL(external_view_embedder, + BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr, + /*device_pixel_ratio=*/2.0, + /*raster_thread_merger=*/_)) + .Times(1); + EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1); + EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, + /*raster_thread_merger=*/_)) + .Times(1); + + rasterizer->Setup(std::move(surface)); + + auto pipeline = fml::AdoptRef(new Pipeline(/*depth=*/10)); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + bool result = pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result); + auto no_discard = [](LayerTree&) { return false; }; + rasterizer->Draw(pipeline, no_discard); +} } // namespace flutter diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index c348b0e071bcf..3f2bf84220ee4 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -1052,12 +1052,18 @@ TEST_F(ShellTest, auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; std::shared_ptr external_view_embedder; - + fml::RefPtr raster_thread_merger_ref; auto end_frame_callback = [&](bool should_resubmit_frame, fml::RefPtr raster_thread_merger) { - external_view_embedder->UpdatePostPrerollResult( - PostPrerollResult::kSuccess); + if (!raster_thread_merger_ref) { + raster_thread_merger_ref = raster_thread_merger; + } + if (should_resubmit_frame && !raster_thread_merger->IsMerged()) { + raster_thread_merger->MergeWithLease(10); + external_view_embedder->UpdatePostPrerollResult( + PostPrerollResult::kSuccess); + } end_frame_latch.Signal(); }; external_view_embedder = std::make_shared( @@ -1065,7 +1071,6 @@ TEST_F(ShellTest, auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(), false, external_view_embedder); - PlatformViewNotifyCreated(shell.get()); auto configuration = RunConfiguration::InferFromSettings(settings); @@ -1075,13 +1080,18 @@ TEST_F(ShellTest, ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); PumpOneFrame(shell.get()); - // `EndFrame` changed the post preroll result to `kSuccess`. + // `EndFrame` changed the post preroll result to `kSuccess` and merged the + // threads. During the frame, the threads are not merged, So no + // `external_view_embedder->GetSubmittedFrameCount()` is called. end_frame_latch.Wait(); - ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount()); + ASSERT_TRUE(raster_thread_merger_ref->IsMerged()); + ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount()); + // This is the resubmitted frame, which threads are also merged. end_frame_latch.Wait(); - ASSERT_EQ(2, external_view_embedder->GetSubmittedFrameCount()); + ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount()); + PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 8169e699689fe..71559ccb41f94 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -342,6 +342,9 @@ void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack, UIView* embedded_view) { + if (flutter_view_ == nullptr) { + return; + } FML_DCHECK(CATransform3DEqualToTransform(embedded_view.layer.transform, CATransform3DIdentity)); ResetAnchor(embedded_view.layer); ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview; @@ -398,7 +401,6 @@ void FlutterPlatformViewsController::CompositeWithParams(int view_id, const EmbeddedViewParams& params) { - FML_DCHECK(flutter_view_); CGRect frame = CGRectMake(0, 0, params.sizePoints().width(), params.sizePoints().height()); UIView* touchInterceptor = touch_interceptors_[view_id].get(); touchInterceptor.layer.transform = CATransform3DIdentity; @@ -419,9 +421,8 @@ } SkCanvas* FlutterPlatformViewsController::CompositeEmbeddedView(int view_id) { - FML_DCHECK(flutter_view_); - // TODO(amirh): assert that this is running on the platform thread once we support the iOS - // embedded views thread configuration. + // Any UIKit related code has to run on main thread. + FML_DCHECK([[NSThread currentThread] isMainThread]); // Do nothing if the view doesn't need to be composited. if (views_to_recomposite_.count(view_id) == 0) { @@ -463,12 +464,11 @@ bool FlutterPlatformViewsController::SubmitFrame(GrDirectContext* gr_context, std::shared_ptr ios_context, std::unique_ptr frame) { - FML_DCHECK(flutter_view_); - // Any UIKit related code has to run on main thread. - // When on a non-main thread, we only allow the rest of the method to run if there is no - // Pending UIView operations. - FML_DCHECK([[NSThread currentThread] isMainThread] || !HasPlatformViewThisOrNextFrame()); + FML_DCHECK([[NSThread currentThread] isMainThread]); + if (flutter_view_ == nullptr) { + return frame->Submit(); + } DisposeViews(); @@ -552,8 +552,6 @@ BringLayersIntoView(platform_view_layers); // Mark all layers as available, so they can be used in the next frame. layer_pool_->RecycleLayers(); - // Reset the composition order, so next frame starts empty. - composition_order_.clear(); did_submit &= frame->Submit(); @@ -593,7 +591,10 @@ void FlutterPlatformViewsController::EndFrame( bool should_resubmit_frame, - fml::RefPtr raster_thread_merger) {} + fml::RefPtr raster_thread_merger) { + // Reset the composition order, so next frame starts empty. + composition_order_.clear(); +} std::shared_ptr FlutterPlatformViewsController::GetLayer( GrDirectContext* gr_context, diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 39d6b987dc4ca..5f20541840e3b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -574,6 +574,68 @@ - (void)testSetFlutterViewControllerAfterCreateCanStillDispatchTouchEvents { flutterPlatformViewsController->Reset(); } +- (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashing { + flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate; + auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest"); + flutter::TaskRunners runners(/*label=*/self.name.UTF8String, + /*platform=*/thread_task_runner, + /*raster=*/thread_task_runner, + /*ui=*/thread_task_runner, + /*io=*/thread_task_runner); + auto surface_factory = flutter::IOSSurfaceFactory::Create(flutter::IOSRenderingAPI::kSoftware); + auto platform_view = std::make_unique( + /*delegate=*/mock_delegate, + /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware, + /*ios_surface_factory=*/surface_factory, + /*task_runners=*/runners); + + auto flutterPlatformViewsController = + std::make_shared(surface_factory); + surface_factory->SetPlatformViewsController(flutterPlatformViewsController); + + FlutterPlatformViewsTestMockFlutterPlatformFactory* factory = + [[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease]; + flutterPlatformViewsController->RegisterViewFactory( + factory, @"MockFlutterPlatformView", + FlutterPlatformViewGestureRecognizersBlockingPolicyEager); + FlutterResult result = ^(id result) { + }; + flutterPlatformViewsController->OnMethodCall( + [FlutterMethodCall + methodCallWithMethodName:@"create" + arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}], + result); + + XCTAssertNotNil(gMockPlatformView); + + // Create embedded view params + flutter::MutatorsStack stack; + SkMatrix finalMatrix; + + auto embeddedViewParams_1 = + std::make_unique(finalMatrix, SkSize::Make(300, 300), stack); + + flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams_1)); + flutterPlatformViewsController->CompositeEmbeddedView(2); + auto mock_surface = std::make_unique( + nullptr, true, + [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return false; }); + XCTAssertFalse( + flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface))); + + auto embeddedViewParams_2 = + std::make_unique(finalMatrix, SkSize::Make(300, 300), stack); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams_2)); + flutterPlatformViewsController->CompositeEmbeddedView(2); + auto mock_surface_submit_false = std::make_unique( + nullptr, true, + [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); + XCTAssertTrue(flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, + std::move(mock_surface_submit_false))); + + flutterPlatformViewsController->Reset(); +} + - (int)alphaOfPoint:(CGPoint)point onView:(UIView*)view { unsigned char pixel[4] = {0}; From b28e58a61c025aab70e4e1508f50735d20ed1c8e Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Wed, 11 Nov 2020 14:07:27 -0800 Subject: [PATCH 5/8] fix clang-tidy --- shell/common/rasterizer_unittests.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index 2ccc9ca706bbb..f39d70c5387f1 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -89,7 +89,7 @@ TEST(RasterizerTest, drawEmptyPipeline) { fml::AutoResetWaitableEvent latch; thread_host.raster_thread->GetTaskRunner()->PostTask([&] { auto pipeline = fml::AdoptRef(new Pipeline(/*depth=*/10)); - rasterizer->Draw(pipeline, nullptr); + rasterizer->Draw(pipeline); latch.Signal(); }); latch.Wait(); @@ -146,7 +146,7 @@ TEST(RasterizerTest, bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); auto no_discard = [](LayerTree&) { return false; }; - rasterizer->Draw(pipeline, no_discard); + rasterizer->Draw(pipeline); latch.Signal(); }); latch.Wait(); @@ -200,7 +200,7 @@ TEST( bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); auto no_discard = [](LayerTree&) { return false; }; - rasterizer->Draw(pipeline, no_discard); + rasterizer->Draw(pipeline); latch.Signal(); }); latch.Wait(); @@ -259,6 +259,6 @@ TEST( bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); auto no_discard = [](LayerTree&) { return false; }; - rasterizer->Draw(pipeline, no_discard); + rasterizer->Draw(pipeline); } } // namespace flutter From 252a783f41841c7751e891443e8d60d1bc84fd2b Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Wed, 11 Nov 2020 14:34:58 -0800 Subject: [PATCH 6/8] remove unused var --- shell/common/rasterizer_unittests.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index f39d70c5387f1..40211fad49b6b 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -145,7 +145,6 @@ TEST(RasterizerTest, /*device_pixel_ratio=*/2.0f); bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); - auto no_discard = [](LayerTree&) { return false; }; rasterizer->Draw(pipeline); latch.Signal(); }); @@ -199,7 +198,6 @@ TEST( /*device_pixel_ratio=*/2.0f); bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); - auto no_discard = [](LayerTree&) { return false; }; rasterizer->Draw(pipeline); latch.Signal(); }); @@ -258,7 +256,6 @@ TEST( /*device_pixel_ratio=*/2.0f); bool result = pipeline->Produce().Complete(std::move(layer_tree)); EXPECT_TRUE(result); - auto no_discard = [](LayerTree&) { return false; }; rasterizer->Draw(pipeline); } } // namespace flutter From 372044f11dd6245503a5c031e9e3f6f9cc815fb0 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Wed, 11 Nov 2020 15:05:01 -0800 Subject: [PATCH 7/8] update license golden --- ci/licenses_golden/licenses_third_party | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/licenses_golden/licenses_third_party b/ci/licenses_golden/licenses_third_party index 8f3f3360d00a4..fc128cf54a439 100644 --- a/ci/licenses_golden/licenses_third_party +++ b/ci/licenses_golden/licenses_third_party @@ -1,4 +1,4 @@ -Signature: 35f962fd83423ee7de3761729e0c25c4 +Signature: 07e2f9b6ddb8b3f757d8a0f88f0bcfba UNUSED LICENSES: From f789991ba9a4851d2f04dfff83fd3844b82d3482 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Wed, 11 Nov 2020 16:30:50 -0800 Subject: [PATCH 8/8] disable test --- shell/common/shell_unittests.cc | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 3f2bf84220ee4..96f30c2c5d9bc 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -996,13 +996,7 @@ TEST_F(ShellTest, } // TODO(https://github.com/flutter/flutter/issues/59816): Enable on fuchsia. -TEST_F(ShellTest, -#if defined(OS_FUCHSIA) - DISABLED_SkipAndSubmitFrame -#else - SkipAndSubmitFrame -#endif -) { +TEST_F(ShellTest, DISABLED_SkipAndSubmitFrame) { auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; std::shared_ptr external_view_embedder;