diff --git a/fml/memory/task_runner_checker.cc b/fml/memory/task_runner_checker.cc index dae642dfea8d5..6390472234602 100644 --- a/fml/memory/task_runner_checker.cc +++ b/fml/memory/task_runner_checker.cc @@ -7,14 +7,18 @@ namespace fml { TaskRunnerChecker::TaskRunnerChecker() - : initialized_queue_id_(InitTaskQueueId()){}; + : initialized_queue_id_(InitTaskQueueId()), + subsumed_queue_id_( + MessageLoopTaskQueues::GetInstance()->GetSubsumedTaskQueueId( + initialized_queue_id_)){}; TaskRunnerChecker::~TaskRunnerChecker() = default; bool TaskRunnerChecker::RunsOnCreationTaskRunner() const { FML_CHECK(fml::MessageLoop::IsInitializedForCurrentThread()); const auto current_queue_id = MessageLoop::GetCurrentTaskQueueId(); - return RunsOnTheSameThread(current_queue_id, initialized_queue_id_); + return RunsOnTheSameThread(current_queue_id, initialized_queue_id_) || + RunsOnTheSameThread(current_queue_id, subsumed_queue_id_); }; bool TaskRunnerChecker::RunsOnTheSameThread(TaskQueueId queue_a, diff --git a/fml/memory/task_runner_checker.h b/fml/memory/task_runner_checker.h index cb36dadcff201..4732452408801 100644 --- a/fml/memory/task_runner_checker.h +++ b/fml/memory/task_runner_checker.h @@ -22,6 +22,7 @@ class TaskRunnerChecker final { private: TaskQueueId initialized_queue_id_; + TaskQueueId subsumed_queue_id_; TaskQueueId InitTaskQueueId(); }; diff --git a/fml/memory/task_runner_checker_unittest.cc b/fml/memory/task_runner_checker_unittest.cc index fbbc6e4934550..7da0dc93c0e04 100644 --- a/fml/memory/task_runner_checker_unittest.cc +++ b/fml/memory/task_runner_checker_unittest.cc @@ -115,5 +115,57 @@ TEST(TaskRunnerCheckerTests, MergedTaskRunnersRunsOnTheSameThread) { thread2.join(); } +TEST(TaskRunnerCheckerTests, + PassesRunsOnCreationTaskRunnerIfOnDifferentTaskRunner) { + fml::MessageLoop* loop1 = nullptr; + fml::AutoResetWaitableEvent latch1; + std::thread thread1([&]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop1 = &fml::MessageLoop::GetCurrent(); + latch1.Signal(); + loop1->Run(); + }); + + fml::MessageLoop* loop2 = nullptr; + fml::AutoResetWaitableEvent latch2; + std::thread thread2([&]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop2 = &fml::MessageLoop::GetCurrent(); + latch2.Signal(); + loop2->Run(); + }); + + latch1.Wait(); + latch2.Wait(); + + fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId(); + fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId(); + fml::MessageLoopTaskQueues::GetInstance()->Merge(qid1, qid2); + + std::unique_ptr checker; + + fml::AutoResetWaitableEvent latch3; + loop2->GetTaskRunner()->PostTask([&]() { + checker = std::make_unique(); + EXPECT_EQ(checker->RunsOnCreationTaskRunner(), true); + latch3.Signal(); + }); + latch3.Wait(); + + fml::MessageLoopTaskQueues::GetInstance()->Unmerge(qid1); + + fml::AutoResetWaitableEvent latch4; + loop2->GetTaskRunner()->PostTask([&]() { + EXPECT_EQ(checker->RunsOnCreationTaskRunner(), true); + latch4.Signal(); + }); + latch4.Wait(); + + loop1->Terminate(); + loop2->Terminate(); + thread1.join(); + thread2.join(); +} + } // namespace testing } // namespace fml diff --git a/fml/message_loop_task_queues.cc b/fml/message_loop_task_queues.cc index 34ddab8819e54..e1c59c9190909 100644 --- a/fml/message_loop_task_queues.cc +++ b/fml/message_loop_task_queues.cc @@ -238,7 +238,14 @@ bool MessageLoopTaskQueues::Unmerge(TaskQueueId owner) { bool MessageLoopTaskQueues::Owns(TaskQueueId owner, TaskQueueId subsumed) const { std::lock_guard guard(queue_mutex_); - return subsumed == queue_entries_.at(owner)->owner_of; + return owner != _kUnmerged && subsumed != _kUnmerged && + subsumed == queue_entries_.at(owner)->owner_of; +} + +TaskQueueId MessageLoopTaskQueues::GetSubsumedTaskQueueId( + TaskQueueId owner) const { + std::lock_guard guard(queue_mutex_); + return queue_entries_.at(owner)->owner_of; } // Subsumed queues will never have pending tasks. diff --git a/fml/message_loop_task_queues.h b/fml/message_loop_task_queues.h index eb056d6935c03..a555faf60e699 100644 --- a/fml/message_loop_task_queues.h +++ b/fml/message_loop_task_queues.h @@ -122,6 +122,10 @@ class MessageLoopTaskQueues // Returns true if owner owns the subsumed task queue. bool Owns(TaskQueueId owner, TaskQueueId subsumed) const; + // Returns the subsumed task queue if any or |TaskQueueId::kUnmerged| + // otherwise. + TaskQueueId GetSubsumedTaskQueueId(TaskQueueId owner) const; + private: class MergedQueuesRunner; diff --git a/fml/message_loop_task_queues_unittests.cc b/fml/message_loop_task_queues_unittests.cc index 052f70c2ae745..43310505b1227 100644 --- a/fml/message_loop_task_queues_unittests.cc +++ b/fml/message_loop_task_queues_unittests.cc @@ -185,6 +185,13 @@ TEST(MessageLoopTaskQueue, QueueDoNotOwnItself) { ASSERT_FALSE(task_queue->Owns(queue_id, queue_id)); } +TEST(MessageLoopTaskQueue, QueueDoNotOwnUnmergedTaskQueueId) { + auto task_queue = fml::MessageLoopTaskQueues::GetInstance(); + ASSERT_FALSE(task_queue->Owns(task_queue->CreateTaskQueue(), _kUnmerged)); + ASSERT_FALSE(task_queue->Owns(_kUnmerged, task_queue->CreateTaskQueue())); + ASSERT_FALSE(task_queue->Owns(_kUnmerged, _kUnmerged)); +} + // TODO(chunhtai): This unit-test is flaky and sometimes fails asynchronizely // after the test has finished. // https://github.com/flutter/flutter/issues/43858 diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index fd6529b748f6f..d44d5fb8c5e41 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -458,6 +458,7 @@ public void detach() { if (platformViewsChannel != null) { platformViewsChannel.setPlatformViewsHandler(null); } + destroyOverlaySurfaces(); platformViewsChannel = null; context = null; textureRegistry = null; @@ -485,6 +486,7 @@ public void attachToView(@NonNull View flutterView) { * the previously attached {@code View}. */ public void detachFromView() { + destroyOverlaySurfaces(); this.flutterView = null; // Inform all existing platform views that they are no longer associated with @@ -703,6 +705,12 @@ private void initializeRootImageViewIfNeeded() { } } + /** + * Initializes a platform view and adds it to the view hierarchy. + * + * @param viewId The view ID. + * @hide + */ @VisibleForTesting void initializePlatformViewIfNeeded(int viewId) { final PlatformView platformView = platformViews.get(viewId); @@ -733,6 +741,19 @@ public void attachToFlutterRenderer(FlutterRenderer flutterRenderer) { androidTouchProcessor = new AndroidTouchProcessor(flutterRenderer, /*trackMotionEvents=*/ true); } + /** + * Called when a platform view id displayed in the current frame. + * + * @param viewId The ID of the platform view. + * @param x The left position relative to {@code FlutterView}. + * @param y The top position relative to {@code FlutterView}. + * @param width The width of the platform view. + * @param height The height of the platform view. + * @param viewWidth The original width of the platform view before applying the mutator stack. + * @param viewHeight The original height of the platform view before applying the mutator stack. + * @param mutatorsStack The mutator stack. + * @hide + */ public void onDisplayPlatformView( int viewId, int x, @@ -760,7 +781,20 @@ public void onDisplayPlatformView( currentFrameUsedPlatformViewIds.add(viewId); } + /** + * Called when an overlay surface is displayed in the current frame. + * + * @param id The ID of the surface. + * @param x The left position relative to {@code FlutterView}. + * @param y The top position relative to {@code FlutterView}. + * @param width The width of the surface. + * @param height The height of the surface. + * @hide + */ public void onDisplayOverlaySurface(int id, int x, int y, int width, int height) { + if (overlayLayerViews.get(id) == null) { + throw new IllegalStateException("The overlay surface (id:" + id + ") doesn't exist"); + } initializeRootImageViewIfNeeded(); final FlutterImageView overlayView = overlayLayerViews.get(id); @@ -782,6 +816,11 @@ public void onBeginFrame() { currentFrameUsedPlatformViewIds.clear(); } + /** + * Called by {@code FlutterJNI} when the Flutter frame was submitted. + * + * @hide + */ public void onEndFrame() { final FlutterView view = (FlutterView) flutterView; // If there are no platform views in the current frame, @@ -850,6 +889,13 @@ private void finishFrame(boolean isFrameRenderedUsingImageReaders) { } } + /** + * Creates and tracks the overlay surface. + * + * @param imageView The surface that displays the overlay. + * @return Wrapper object that provides the layer id and the surface. + * @hide + */ @VisibleForTesting @TargetApi(19) public FlutterOverlaySurface createOverlaySurface(@NonNull FlutterImageView imageView) { @@ -858,6 +904,13 @@ public FlutterOverlaySurface createOverlaySurface(@NonNull FlutterImageView imag return new FlutterOverlaySurface(id, imageView.getSurface()); } + /** + * Creates an overlay surface while the Flutter view is rendered by {@code FlutterImageView}. + * + *

This method is invoked by {@code FlutterJNI} only. + * + * @hide + */ @TargetApi(19) public FlutterOverlaySurface createOverlaySurface() { // Overlay surfaces have the same size as the background surface. @@ -874,12 +927,21 @@ public FlutterOverlaySurface createOverlaySurface() { FlutterImageView.SurfaceKind.overlay)); } + /** + * Destroys the overlay surfaces and removes them from the view hierarchy. + * + *

This method is used only internally by {@code FlutterJNI}. + * + * @hide + */ public void destroyOverlaySurfaces() { for (int i = 0; i < overlayLayerViews.size(); i++) { int overlayId = overlayLayerViews.keyAt(i); FlutterImageView overlayView = overlayLayerViews.valueAt(i); overlayView.detachFromRenderer(); - ((FlutterView) flutterView).removeView(overlayView); + if (flutterView != null) { + ((FlutterView) flutterView).removeView(overlayView); + } } overlayLayerViews.clear(); } diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index 62806032cd092..1b75c81705f94 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -498,6 +498,131 @@ public void onEndFrame__removesPlatformViewParent() { assertEquals(flutterView.getChildCount(), 1); } + @Test + @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + public void detach__destroysOverlaySurfaces() { + final PlatformViewsController platformViewsController = new PlatformViewsController(); + + final int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + final PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + final PlatformView platformView = mock(PlatformView.class); + when(platformView.getView()).thenReturn(mock(View.class)); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + final FlutterJNI jni = new FlutterJNI(); + jni.attachToNative(false); + attach(jni, platformViewsController); + + jni.onFirstFrame(); + + // Simulate create call from the framework. + createPlatformView(jni, platformViewsController, platformViewId, "testType"); + + // Produce a frame that displays a platform view and an overlay surface. + platformViewsController.onBeginFrame(); + platformViewsController.onDisplayPlatformView( + platformViewId, + /* x=*/ 0, + /* y=*/ 0, + /* width=*/ 10, + /* height=*/ 10, + /* viewWidth=*/ 10, + /* viewHeight=*/ 10, + /* mutatorsStack=*/ new FlutterMutatorsStack()); + + final FlutterImageView overlayImageView = mock(FlutterImageView.class); + when(overlayImageView.acquireLatestImage()).thenReturn(true); + + final FlutterOverlaySurface overlaySurface = + platformViewsController.createOverlaySurface(overlayImageView); + // This is OK. + platformViewsController.onDisplayOverlaySurface( + overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); + + platformViewsController.detach(); + + assertThrows( + IllegalStateException.class, + () -> { + platformViewsController.onDisplayOverlaySurface( + overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); + }); + } + + @Test + @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + public void detachFromView__removesOverlaySurfaces() { + final PlatformViewsController platformViewsController = new PlatformViewsController(); + + final int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + final PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + final PlatformView platformView = mock(PlatformView.class); + when(platformView.getView()).thenReturn(mock(View.class)); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + final FlutterJNI jni = new FlutterJNI(); + jni.attachToNative(false); + attach(jni, platformViewsController); + + final FlutterImageView overlayImageView = mock(FlutterImageView.class); + when(overlayImageView.acquireLatestImage()).thenReturn(true); + + final FlutterOverlaySurface overlaySurface = + platformViewsController.createOverlaySurface(overlayImageView); + + platformViewsController.onDisplayOverlaySurface( + overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); + + platformViewsController.detachFromView(); + + assertThrows( + IllegalStateException.class, + () -> { + platformViewsController.onDisplayOverlaySurface( + overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); + }); + } + + @Test + @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + public void destroyOverlaySurfaces__doesNotThrowIfControllerIsDetached() { + final PlatformViewsController platformViewsController = new PlatformViewsController(); + + final int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + final PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + final PlatformView platformView = mock(PlatformView.class); + when(platformView.getView()).thenReturn(mock(View.class)); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + final FlutterJNI jni = new FlutterJNI(); + jni.attachToNative(false); + attach(jni, platformViewsController); + + final FlutterImageView overlayImageView = mock(FlutterImageView.class); + when(overlayImageView.acquireLatestImage()).thenReturn(true); + + final FlutterOverlaySurface overlaySurface = + platformViewsController.createOverlaySurface(overlayImageView); + + platformViewsController.onDisplayOverlaySurface( + overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); + + platformViewsController.detachFromView(); + platformViewsController.destroyOverlaySurfaces(); + } + @Test public void checkInputConnectionProxy__falseIfViewIsNull() { final PlatformViewsController platformViewsController = new PlatformViewsController(); diff --git a/testing/scenario_app/android/app/build.gradle b/testing/scenario_app/android/app/build.gradle index a2313057b9cb7..16288dd35f2cb 100644 --- a/testing/scenario_app/android/app/build.gradle +++ b/testing/scenario_app/android/app/build.gradle @@ -1,6 +1,8 @@ apply plugin: 'com.android.application' apply plugin: 'com.facebook.testing.screenshot' +def leakcanary_version = '2.6' + screenshots { failureDir = "${rootProject.buildDir}/reports/diff_failures" recordDir = "${rootProject.projectDir}/reports/screenshots" @@ -13,12 +15,14 @@ android { targetCompatibility JavaVersion.VERSION_1_8 } defaultConfig { - applicationId "dev.flutter.scenarios" + applicationId 'dev.flutter.scenarios' minSdkVersion 18 targetSdkVersion 30 versionCode 1 - versionName "1.0" - testInstrumentationRunner "dev.flutter.TestRunner" + versionName '1.0' + testInstrumentationRunner 'dev.flutter.TestRunner' + testInstrumentationRunnerArgument 'listener', 'leakcanary.FailTestOnLeakRunListener' + testInstrumentationRunnerArguments clearPackageData: 'true' } buildTypes { release { @@ -38,8 +42,10 @@ dependencies { implementation 'androidx.constraintlayout:constraintlayout:1.1.3' implementation 'com.google.android.material:material:1.0.0' implementation 'androidx.lifecycle:lifecycle-common-java8:2.2.0-alpha01' + implementation "com.squareup.leakcanary:leakcanary-android:$leakcanary_version" testImplementation 'junit:junit:4.12' androidTestImplementation 'androidx.test:runner:1.2.0' androidTestImplementation 'androidx.test:rules:1.2.0' androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0' + androidTestImplementation "com.squareup.leakcanary:leakcanary-android-instrumentation:$leakcanary_version" } diff --git a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java new file mode 100644 index 0000000000000..3ec310b5ef159 --- /dev/null +++ b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java @@ -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. + +package dev.flutter.scenariosui; + +import android.content.Intent; +import androidx.test.filters.LargeTest; +import androidx.test.rule.ActivityTestRule; +import androidx.test.runner.AndroidJUnit4; +import dev.flutter.scenarios.TextPlatformViewActivity; +import leakcanary.FailTestOnLeak; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +@LargeTest +public class MemoryLeakTests { + @Rule + public ActivityTestRule activityRule = + new ActivityTestRule<>( + TextPlatformViewActivity.class, /*initialTouchMode=*/ false, /*launchActivity=*/ false); + + @Test + @FailTestOnLeak + public void platformViewHybridComposition_launchActivityFinishAndLaunchAgain() throws Exception { + Intent intent = new Intent(Intent.ACTION_MAIN); + intent.putExtra("scenario", "platform_view"); + intent.putExtra("use_android_view", true); + + activityRule.launchActivity(intent); + } +} diff --git a/testing/scenario_app/android/app/src/main/res/values/strings.xml b/testing/scenario_app/android/app/src/main/res/values/strings.xml index 1cc6e997d7abe..c9b0fe65b7f92 100644 --- a/testing/scenario_app/android/app/src/main/res/values/strings.xml +++ b/testing/scenario_app/android/app/src/main/res/values/strings.xml @@ -1,4 +1,5 @@ Scenarios MainActivity + assertk.Assert