diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 5f784be07af1e..0763f58c424f5 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -359,7 +359,6 @@ FILE: ../../../flutter/lib/ui/painting/path.cc FILE: ../../../flutter/lib/ui/painting/path.h FILE: ../../../flutter/lib/ui/painting/path_measure.cc FILE: ../../../flutter/lib/ui/painting/path_measure.h -FILE: ../../../flutter/lib/ui/painting/path_unittests.cc FILE: ../../../flutter/lib/ui/painting/picture.cc FILE: ../../../flutter/lib/ui/painting/picture.h FILE: ../../../flutter/lib/ui/painting/picture_recorder.cc @@ -403,8 +402,6 @@ FILE: ../../../flutter/lib/ui/ui.dart FILE: ../../../flutter/lib/ui/ui_benchmarks.cc FILE: ../../../flutter/lib/ui/ui_dart_state.cc FILE: ../../../flutter/lib/ui/ui_dart_state.h -FILE: ../../../flutter/lib/ui/volatile_path_tracker.cc -FILE: ../../../flutter/lib/ui/volatile_path_tracker.h FILE: ../../../flutter/lib/ui/window.dart FILE: ../../../flutter/lib/ui/window/platform_configuration.cc FILE: ../../../flutter/lib/ui/window/platform_configuration.h diff --git a/fml/trace_event.h b/fml/trace_event.h index 3928d68c7d24c..ae80298cfd699 100644 --- a/fml/trace_event.h +++ b/fml/trace_event.h @@ -68,19 +68,6 @@ ::fml::tracing::TraceCounter((category_group), (name), (counter_id), (arg1), \ __VA_ARGS__); -// Avoid using the same `name` and `argX_name` for nested traces, which can -// lead to double free errors. E.g. the following code should be avoided: -// -// ```cpp -// { -// TRACE_EVENT1("flutter", "Foo::Bar", "count", "initial_count_value"); -// ... -// TRACE_EVENT_INSTANT1("flutter", "Foo::Bar", -// "count", "updated_count_value"); -// } -// ``` -// -// Instead, either use different `name` or `arg1` parameter names. #define FML_TRACE_EVENT(category_group, name, ...) \ ::fml::tracing::TraceEvent((category_group), (name), __VA_ARGS__); \ __FML__AUTO_TRACE_END(name) diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 2f9efe9e9acfe..81c115454ea2b 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -91,8 +91,6 @@ source_set("ui") { "text/text_box.h", "ui_dart_state.cc", "ui_dart_state.h", - "volatile_path_tracker.cc", - "volatile_path_tracker.h", "window/platform_configuration.cc", "window/platform_configuration.h", "window/platform_message.cc", @@ -193,7 +191,6 @@ if (enable_unittests) { sources = [ "painting/image_dispose_unittests.cc", "painting/image_encoding_unittests.cc", - "painting/path_unittests.cc", "painting/vertices_unittests.cc", "window/platform_configuration_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 9e274e94eb70e..7fd54da91b9f2 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -37,18 +37,6 @@ void createVertices() { } void _validateVertices(Vertices vertices) native 'ValidateVertices'; -@pragma('vm:entry-point') -void createPath() { - final Path path = Path()..lineTo(10, 10); - _validatePath(path); - // Arbitrarily hold a reference to the path to make sure it does not get - // garbage collected. - Future.delayed(const Duration(days: 100)).then((_) { - path.lineTo(100, 100); - }); -} -void _validatePath(Path path) native 'ValidatePath'; - @pragma('vm:entry-point') void frameCallback(FrameInfo info) { print('called back'); diff --git a/lib/ui/painting/path.cc b/lib/ui/painting/path.cc index 6cc2b0529d0b1..d0898a4ca283c 100644 --- a/lib/ui/painting/path.cc +++ b/lib/ui/painting/path.cc @@ -67,69 +67,43 @@ void CanvasPath::RegisterNatives(tonic::DartLibraryNatives* natives) { FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); } -CanvasPath::CanvasPath() - : path_tracker_(UIDartState::Current()->GetVolatilePathTracker()), - tracked_path_(std::make_shared()) { - FML_DCHECK(path_tracker_); - resetVolatility(); -} - -CanvasPath::~CanvasPath() = default; - -void CanvasPath::resetVolatility() { - if (!tracked_path_->tracking_volatility) { - mutable_path().setIsVolatile(true); - tracked_path_->frame_count = 0; - tracked_path_->tracking_volatility = true; - path_tracker_->Insert(tracked_path_); - } -} +CanvasPath::CanvasPath() {} -void CanvasPath::ReleaseDartWrappableReference() const { - FML_DCHECK(path_tracker_); - path_tracker_->Erase(tracked_path_); -} +CanvasPath::~CanvasPath() {} int CanvasPath::getFillType() { - return static_cast(path().getFillType()); + return static_cast(path_.getFillType()); } void CanvasPath::setFillType(int fill_type) { - mutable_path().setFillType(static_cast(fill_type)); - resetVolatility(); + path_.setFillType(static_cast(fill_type)); } void CanvasPath::moveTo(float x, float y) { - mutable_path().moveTo(x, y); - resetVolatility(); + path_.moveTo(x, y); } void CanvasPath::relativeMoveTo(float x, float y) { - mutable_path().rMoveTo(x, y); - resetVolatility(); + path_.rMoveTo(x, y); } void CanvasPath::lineTo(float x, float y) { - mutable_path().lineTo(x, y); - resetVolatility(); + path_.lineTo(x, y); } void CanvasPath::relativeLineTo(float x, float y) { - mutable_path().rLineTo(x, y); - resetVolatility(); + path_.rLineTo(x, y); } void CanvasPath::quadraticBezierTo(float x1, float y1, float x2, float y2) { - mutable_path().quadTo(x1, y1, x2, y2); - resetVolatility(); + path_.quadTo(x1, y1, x2, y2); } void CanvasPath::relativeQuadraticBezierTo(float x1, float y1, float x2, float y2) { - mutable_path().rQuadTo(x1, y1, x2, y2); - resetVolatility(); + path_.rQuadTo(x1, y1, x2, y2); } void CanvasPath::cubicTo(float x1, @@ -138,8 +112,7 @@ void CanvasPath::cubicTo(float x1, float y2, float x3, float y3) { - mutable_path().cubicTo(x1, y1, x2, y2, x3, y3); - resetVolatility(); + path_.cubicTo(x1, y1, x2, y2, x3, y3); } void CanvasPath::relativeCubicTo(float x1, @@ -148,13 +121,11 @@ void CanvasPath::relativeCubicTo(float x1, float y2, float x3, float y3) { - mutable_path().rCubicTo(x1, y1, x2, y2, x3, y3); - resetVolatility(); + path_.rCubicTo(x1, y1, x2, y2, x3, y3); } void CanvasPath::conicTo(float x1, float y1, float x2, float y2, float w) { - mutable_path().conicTo(x1, y1, x2, y2, w); - resetVolatility(); + path_.conicTo(x1, y1, x2, y2, w); } void CanvasPath::relativeConicTo(float x1, @@ -162,8 +133,7 @@ void CanvasPath::relativeConicTo(float x1, float x2, float y2, float w) { - mutable_path().rConicTo(x1, y1, x2, y2, w); - resetVolatility(); + path_.rConicTo(x1, y1, x2, y2, w); } void CanvasPath::arcTo(float left, @@ -173,10 +143,9 @@ void CanvasPath::arcTo(float left, float startAngle, float sweepAngle, bool forceMoveTo) { - mutable_path().arcTo(SkRect::MakeLTRB(left, top, right, bottom), - startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI, - forceMoveTo); - resetVolatility(); + path_.arcTo(SkRect::MakeLTRB(left, top, right, bottom), + startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI, + forceMoveTo); } void CanvasPath::arcToPoint(float arcEndX, @@ -191,9 +160,8 @@ void CanvasPath::arcToPoint(float arcEndX, const auto direction = isClockwiseDirection ? SkPathDirection::kCW : SkPathDirection::kCCW; - mutable_path().arcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, - arcEndX, arcEndY); - resetVolatility(); + path_.arcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, arcEndX, + arcEndY); } void CanvasPath::relativeArcToPoint(float arcEndDeltaX, @@ -207,19 +175,16 @@ void CanvasPath::relativeArcToPoint(float arcEndDeltaX, : SkPath::ArcSize::kSmall_ArcSize; const auto direction = isClockwiseDirection ? SkPathDirection::kCW : SkPathDirection::kCCW; - mutable_path().rArcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, - arcEndDeltaX, arcEndDeltaY); - resetVolatility(); + path_.rArcTo(radiusX, radiusY, xAxisRotation, arcSize, direction, + arcEndDeltaX, arcEndDeltaY); } void CanvasPath::addRect(float left, float top, float right, float bottom) { - mutable_path().addRect(SkRect::MakeLTRB(left, top, right, bottom)); - resetVolatility(); + path_.addRect(SkRect::MakeLTRB(left, top, right, bottom)); } void CanvasPath::addOval(float left, float top, float right, float bottom) { - mutable_path().addOval(SkRect::MakeLTRB(left, top, right, bottom)); - resetVolatility(); + path_.addOval(SkRect::MakeLTRB(left, top, right, bottom)); } void CanvasPath::addArc(float left, @@ -228,20 +193,17 @@ void CanvasPath::addArc(float left, float bottom, float startAngle, float sweepAngle) { - mutable_path().addArc(SkRect::MakeLTRB(left, top, right, bottom), - startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI); - resetVolatility(); + path_.addArc(SkRect::MakeLTRB(left, top, right, bottom), + startAngle * 180.0 / M_PI, sweepAngle * 180.0 / M_PI); } void CanvasPath::addPolygon(const tonic::Float32List& points, bool close) { - mutable_path().addPoly(reinterpret_cast(points.data()), - points.num_elements() / 2, close); - resetVolatility(); + path_.addPoly(reinterpret_cast(points.data()), + points.num_elements() / 2, close); } void CanvasPath::addRRect(const RRect& rrect) { - mutable_path().addRRect(rrect.sk_rrect); - resetVolatility(); + path_.addRRect(rrect.sk_rrect); } void CanvasPath::addPath(CanvasPath* path, double dx, double dy) { @@ -249,8 +211,7 @@ void CanvasPath::addPath(CanvasPath* path, double dx, double dy) { Dart_ThrowException(ToDart("Path.addPath called with non-genuine Path.")); return; } - mutable_path().addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode); - resetVolatility(); + path_.addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode); } void CanvasPath::addPathWithMatrix(CanvasPath* path, @@ -266,9 +227,8 @@ void CanvasPath::addPathWithMatrix(CanvasPath* path, SkMatrix matrix = ToSkMatrix(matrix4); matrix.setTranslateX(matrix.getTranslateX() + dx); matrix.setTranslateY(matrix.getTranslateY() + dy); - mutable_path().addPath(path->path(), matrix, SkPath::kAppend_AddPathMode); + path_.addPath(path->path(), matrix, SkPath::kAppend_AddPathMode); matrix4.Release(); - resetVolatility(); } void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) { @@ -277,8 +237,7 @@ void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) { ToDart("Path.extendWithPath called with non-genuine Path.")); return; } - mutable_path().addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode); - resetVolatility(); + path_.addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode); } void CanvasPath::extendWithPathAndMatrix(CanvasPath* path, @@ -294,43 +253,37 @@ void CanvasPath::extendWithPathAndMatrix(CanvasPath* path, SkMatrix matrix = ToSkMatrix(matrix4); matrix.setTranslateX(matrix.getTranslateX() + dx); matrix.setTranslateY(matrix.getTranslateY() + dy); - mutable_path().addPath(path->path(), matrix, SkPath::kExtend_AddPathMode); + path_.addPath(path->path(), matrix, SkPath::kExtend_AddPathMode); matrix4.Release(); - resetVolatility(); } void CanvasPath::close() { - mutable_path().close(); - resetVolatility(); + path_.close(); } void CanvasPath::reset() { - mutable_path().reset(); - resetVolatility(); + path_.reset(); } bool CanvasPath::contains(double x, double y) { - return path().contains(x, y); + return path_.contains(x, y); } void CanvasPath::shift(Dart_Handle path_handle, double dx, double dy) { fml::RefPtr path = CanvasPath::Create(path_handle); - auto& other_mutable_path = path->mutable_path(); - mutable_path().offset(dx, dy, &other_mutable_path); - resetVolatility(); + path_.offset(dx, dy, &path->path_); } void CanvasPath::transform(Dart_Handle path_handle, tonic::Float64List& matrix4) { fml::RefPtr path = CanvasPath::Create(path_handle); - auto& other_mutable_path = path->mutable_path(); - mutable_path().transform(ToSkMatrix(matrix4), &other_mutable_path); + path_.transform(ToSkMatrix(matrix4), &path->path_); matrix4.Release(); } tonic::Float32List CanvasPath::getBounds() { tonic::Float32List rect(Dart_NewTypedData(Dart_TypedData_kFloat32, 4)); - const SkRect& bounds = path().getBounds(); + const SkRect& bounds = path_.getBounds(); rect[0] = bounds.left(); rect[1] = bounds.top(); rect[2] = bounds.right(); @@ -340,22 +293,21 @@ tonic::Float32List CanvasPath::getBounds() { bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) { return Op(path1->path(), path2->path(), static_cast(operation), - &tracked_path_->path); - resetVolatility(); + &path_); } void CanvasPath::clone(Dart_Handle path_handle) { fml::RefPtr path = CanvasPath::Create(path_handle); // per Skia docs, this will create a fast copy // data is shared until the source path or dest path are mutated - path->mutable_path() = this->path(); + path->path_ = path_; } // This is doomed to be called too early, since Paths are mutable. // However, it can help for some of the clone/shift/transform type methods // where the resultant path will initially have a meaningful size. size_t CanvasPath::GetAllocationSize() const { - return sizeof(CanvasPath) + path().approximateBytesUsed(); + return sizeof(CanvasPath) + path_.approximateBytesUsed(); } } // namespace flutter diff --git a/lib/ui/painting/path.h b/lib/ui/painting/path.h index 3c05cdd140837..dbd6d7a0d1cc2 100644 --- a/lib/ui/painting/path.h +++ b/lib/ui/painting/path.h @@ -7,7 +7,6 @@ #include "flutter/lib/ui/dart_wrapper.h" #include "flutter/lib/ui/painting/rrect.h" -#include "flutter/lib/ui/volatile_path_tracker.h" #include "third_party/skia/include/core/SkPath.h" #include "third_party/skia/include/pathops/SkPathOps.h" #include "third_party/tonic/typed_data/typed_list.h" @@ -37,7 +36,7 @@ class CanvasPath : public RefCountedDartWrappable { static fml::RefPtr CreateFrom(Dart_Handle path_handle, const SkPath& src) { fml::RefPtr path = CanvasPath::Create(path_handle); - path->tracked_path_->path = src; + path->path_ = src; return path; } @@ -109,24 +108,16 @@ class CanvasPath : public RefCountedDartWrappable { bool op(CanvasPath* path1, CanvasPath* path2, int operation); void clone(Dart_Handle path_handle); - const SkPath& path() const { return tracked_path_->path; } + const SkPath& path() const { return path_; } size_t GetAllocationSize() const override; static void RegisterNatives(tonic::DartLibraryNatives* natives); - virtual void ReleaseDartWrappableReference() const override; - private: CanvasPath(); - std::shared_ptr path_tracker_; - std::shared_ptr tracked_path_; - - // Must be called whenever the path is created or mutated. - void resetVolatility(); - - SkPath& mutable_path() { return tracked_path_->path; } + SkPath path_; }; } // namespace flutter diff --git a/lib/ui/painting/path_unittests.cc b/lib/ui/painting/path_unittests.cc deleted file mode 100644 index 277e9e5b1e5da..0000000000000 --- a/lib/ui/painting/path_unittests.cc +++ /dev/null @@ -1,185 +0,0 @@ -// 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/lib/ui/painting/path.h" - -#include - -#include "flutter/common/task_runners.h" -#include "flutter/fml/synchronization/waitable_event.h" -#include "flutter/runtime/dart_vm.h" -#include "flutter/shell/common/shell_test.h" -#include "flutter/shell/common/thread_host.h" -#include "flutter/testing/testing.h" - -namespace flutter { -namespace testing { - -TEST_F(ShellTest, PathVolatilityOldPathsBecomeNonVolatile) { - auto message_latch = std::make_shared(); - - auto native_validate_path = [message_latch](Dart_NativeArguments args) { - auto handle = Dart_GetNativeArgument(args, 0); - intptr_t peer = 0; - Dart_Handle result = Dart_GetNativeInstanceField( - handle, tonic::DartWrappable::kPeerIndex, &peer); - EXPECT_FALSE(Dart_IsError(result)); - CanvasPath* path = reinterpret_cast(peer); - EXPECT_TRUE(path); - EXPECT_TRUE(path->path().isVolatile()); - std::shared_ptr tracker = - UIDartState::Current()->GetVolatilePathTracker(); - EXPECT_TRUE(tracker); - - for (int i = 0; i < VolatilePathTracker::kFramesOfVolatility; i++) { - EXPECT_TRUE(path->path().isVolatile()); - tracker->OnFrame(); - } - EXPECT_FALSE(path->path().isVolatile()); - message_latch->Signal(); - }; - - Settings settings = CreateSettingsForFixture(); - TaskRunners task_runners("test", // label - GetCurrentTaskRunner(), // platform - CreateNewThread(), // raster - CreateNewThread(), // ui - CreateNewThread() // io - ); - - AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path)); - - std::unique_ptr shell = - CreateShell(std::move(settings), std::move(task_runners)); - - ASSERT_TRUE(shell->IsSetup()); - auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("createPath"); - - shell->RunEngine(std::move(configuration), [](auto result) { - ASSERT_EQ(result, Engine::RunStatus::Success); - }); - - message_latch->Wait(); - - DestroyShell(std::move(shell), std::move(task_runners)); -} - -TEST_F(ShellTest, PathVolatilityGCRemovesPathFromTracker) { - auto message_latch = std::make_shared(); - - auto native_validate_path = [message_latch](Dart_NativeArguments args) { - auto handle = Dart_GetNativeArgument(args, 0); - intptr_t peer = 0; - Dart_Handle result = Dart_GetNativeInstanceField( - handle, tonic::DartWrappable::kPeerIndex, &peer); - EXPECT_FALSE(Dart_IsError(result)); - CanvasPath* path = reinterpret_cast(peer); - EXPECT_TRUE(path); - EXPECT_TRUE(path->path().isVolatile()); - std::shared_ptr tracker = - UIDartState::Current()->GetVolatilePathTracker(); - EXPECT_TRUE(tracker); - - static_assert(VolatilePathTracker::kFramesOfVolatility > 1); - EXPECT_TRUE(path->path().isVolatile()); - tracker->OnFrame(); - EXPECT_TRUE(path->path().isVolatile()); - - // simulate GC - path->ReleaseDartWrappableReference(); - - tracker->OnFrame(); - // Because the path got GC'd, it was removed from the cache and we're the - // only one holding it. - EXPECT_TRUE(path->path().isVolatile()); - - message_latch->Signal(); - }; - - Settings settings = CreateSettingsForFixture(); - TaskRunners task_runners("test", // label - GetCurrentTaskRunner(), // platform - CreateNewThread(), // raster - CreateNewThread(), // ui - CreateNewThread() // io - ); - - AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path)); - - std::unique_ptr shell = - CreateShell(std::move(settings), std::move(task_runners)); - - ASSERT_TRUE(shell->IsSetup()); - auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("createPath"); - - shell->RunEngine(std::move(configuration), [](auto result) { - ASSERT_EQ(result, Engine::RunStatus::Success); - }); - - message_latch->Wait(); - - DestroyShell(std::move(shell), std::move(task_runners)); -} - -// Screen diffing tests use deterministic rendering. Allowing a path to be -// volatile or not for an individual frame can result in minor pixel differences -// that cause the test to fail. -// If deterministic rendering is enabled, the tracker should be disabled and -// paths should always be non-volatile. -TEST_F(ShellTest, DeterministicRenderingDisablesPathVolatility) { - auto message_latch = std::make_shared(); - - auto native_validate_path = [message_latch](Dart_NativeArguments args) { - auto handle = Dart_GetNativeArgument(args, 0); - intptr_t peer = 0; - Dart_Handle result = Dart_GetNativeInstanceField( - handle, tonic::DartWrappable::kPeerIndex, &peer); - EXPECT_FALSE(Dart_IsError(result)); - CanvasPath* path = reinterpret_cast(peer); - EXPECT_TRUE(path); - EXPECT_FALSE(path->path().isVolatile()); - std::shared_ptr tracker = - UIDartState::Current()->GetVolatilePathTracker(); - EXPECT_TRUE(tracker); - EXPECT_FALSE(tracker->enabled()); - - for (int i = 0; i < VolatilePathTracker::kFramesOfVolatility; i++) { - tracker->OnFrame(); - EXPECT_FALSE(path->path().isVolatile()); - } - EXPECT_FALSE(path->path().isVolatile()); - message_latch->Signal(); - }; - - Settings settings = CreateSettingsForFixture(); - settings.skia_deterministic_rendering_on_cpu = true; - TaskRunners task_runners("test", // label - GetCurrentTaskRunner(), // platform - CreateNewThread(), // raster - CreateNewThread(), // ui - CreateNewThread() // io - ); - - AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path)); - - std::unique_ptr shell = - CreateShell(std::move(settings), std::move(task_runners)); - - ASSERT_TRUE(shell->IsSetup()); - auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("createPath"); - - shell->RunEngine(std::move(configuration), [](auto result) { - ASSERT_EQ(result, Engine::RunStatus::Success); - }); - - message_latch->Wait(); - - DestroyShell(std::move(shell), std::move(task_runners)); -} - -} // namespace testing -} // namespace flutter diff --git a/lib/ui/ui_benchmarks.cc b/lib/ui/ui_benchmarks.cc index 739885710d7a2..b0f09e373607a 100644 --- a/lib/ui/ui_benchmarks.cc +++ b/lib/ui/ui_benchmarks.cc @@ -4,7 +4,6 @@ #include "flutter/benchmarking/benchmarking.h" #include "flutter/common/settings.h" -#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_message_response_dart.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/shell/common/thread_host.h" @@ -67,54 +66,7 @@ static void BM_PlatformMessageResponseDartComplete(benchmark::State& state) { } } -static void BM_PathVolatilityTracker(benchmark::State& state) { - ThreadHost thread_host("test", - ThreadHost::Type::Platform | ThreadHost::Type::RASTER | - 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()); - - VolatilePathTracker tracker(task_runners.GetUITaskRunner(), true); - - while (state.KeepRunning()) { - std::vector> paths; - constexpr int path_count = 1000; - for (int i = 0; i < path_count; i++) { - auto path = std::make_shared(); - path->path = SkPath(); - path->path.setIsVolatile(true); - paths.push_back(std::move(path)); - } - - fml::AutoResetWaitableEvent latch; - task_runners.GetUITaskRunner()->PostTask([&]() { - for (auto path : paths) { - tracker.Insert(path); - } - latch.Signal(); - }); - - latch.Wait(); - - task_runners.GetUITaskRunner()->PostTask([&]() { tracker.OnFrame(); }); - - for (int i = 0; i < path_count - 10; ++i) { - tracker.Erase(paths[i]); - } - - task_runners.GetUITaskRunner()->PostTask([&]() { tracker.OnFrame(); }); - - latch.Reset(); - task_runners.GetUITaskRunner()->PostTask([&]() { latch.Signal(); }); - latch.Wait(); - } -} - BENCHMARK(BM_PlatformMessageResponseDartComplete) ->Unit(benchmark::kMicrosecond); -BENCHMARK(BM_PathVolatilityTracker)->Unit(benchmark::kMillisecond); - } // namespace flutter diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index 3def189206afc..e5514b21f956d 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -28,7 +28,6 @@ UIDartState::UIDartState( UnhandledExceptionCallback unhandled_exception_callback, std::shared_ptr isolate_name_server, bool is_root_isolate, - std::shared_ptr volatile_path_tracker, bool enable_skparagraph) : task_runners_(std::move(task_runners)), add_callback_(std::move(add_callback)), @@ -38,7 +37,6 @@ UIDartState::UIDartState( io_manager_(std::move(io_manager)), skia_unref_queue_(std::move(skia_unref_queue)), image_decoder_(std::move(image_decoder)), - volatile_path_tracker_(std::move(volatile_path_tracker)), advisory_script_uri_(std::move(advisory_script_uri)), advisory_script_entrypoint_(std::move(advisory_script_entrypoint)), logger_prefix_(std::move(logger_prefix)), @@ -110,11 +108,6 @@ fml::RefPtr UIDartState::GetSkiaUnrefQueue() const { return skia_unref_queue_; } -std::shared_ptr UIDartState::GetVolatilePathTracker() - const { - return volatile_path_tracker_; -} - void UIDartState::ScheduleMicrotask(Dart_Handle closure) { if (tonic::LogIfError(closure) || !Dart_IsClosure(closure)) { return; diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index 0a3e44a97ca78..d05ad9f81167d 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -20,7 +20,6 @@ #include "flutter/lib/ui/isolate_name_server/isolate_name_server.h" #include "flutter/lib/ui/painting/image_decoder.h" #include "flutter/lib/ui/snapshot_delegate.h" -#include "flutter/lib/ui/volatile_path_tracker.h" #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/skia/include/gpu/GrDirectContext.h" #include "third_party/tonic/dart_microtask_queue.h" @@ -60,8 +59,6 @@ class UIDartState : public tonic::DartState { fml::RefPtr GetSkiaUnrefQueue() const; - std::shared_ptr GetVolatilePathTracker() const; - fml::WeakPtr GetSnapshotDelegate() const; fml::WeakPtr GetHintFreedDelegate() const; @@ -105,7 +102,6 @@ class UIDartState : public tonic::DartState { UnhandledExceptionCallback unhandled_exception_callback, std::shared_ptr isolate_name_server, bool is_root_isolate_, - std::shared_ptr volatile_path_tracker, bool enable_skparagraph); ~UIDartState() override; @@ -128,7 +124,6 @@ class UIDartState : public tonic::DartState { fml::WeakPtr io_manager_; fml::RefPtr skia_unref_queue_; fml::WeakPtr image_decoder_; - std::shared_ptr volatile_path_tracker_; const std::string advisory_script_uri_; const std::string advisory_script_entrypoint_; const std::string logger_prefix_; diff --git a/lib/ui/volatile_path_tracker.cc b/lib/ui/volatile_path_tracker.cc deleted file mode 100644 index b82012bfa30a8..0000000000000 --- a/lib/ui/volatile_path_tracker.cc +++ /dev/null @@ -1,85 +0,0 @@ -// 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/lib/ui/volatile_path_tracker.h" - -namespace flutter { - -VolatilePathTracker::VolatilePathTracker( - fml::RefPtr ui_task_runner, - bool enabled) - : ui_task_runner_(ui_task_runner), enabled_(enabled) {} - -void VolatilePathTracker::Insert(std::shared_ptr path) { - FML_DCHECK(ui_task_runner_->RunsTasksOnCurrentThread()); - FML_DCHECK(path); - FML_DCHECK(path->path.isVolatile()); - if (!enabled_) { - path->path.setIsVolatile(false); - return; - } - paths_.insert(path); -} - -void VolatilePathTracker::Erase(std::shared_ptr path) { - if (!enabled_) { - return; - } - FML_DCHECK(path); - if (ui_task_runner_->RunsTasksOnCurrentThread()) { - paths_.erase(path); - return; - } - - std::scoped_lock lock(paths_to_remove_mutex_); - needs_drain_ = true; - paths_to_remove_.push_back(path); -} - -void VolatilePathTracker::OnFrame() { - FML_DCHECK(ui_task_runner_->RunsTasksOnCurrentThread()); - if (!enabled_) { - return; - } - std::string total_count = std::to_string(paths_.size()); - TRACE_EVENT1("flutter", "VolatilePathTracker::OnFrame", "total_count", - total_count.c_str()); - - Drain(); - - std::set> surviving_paths_; - for (const std::shared_ptr& path : paths_) { - path->frame_count++; - if (path->frame_count >= kFramesOfVolatility) { - path->path.setIsVolatile(false); - path->tracking_volatility = false; - } else { - surviving_paths_.insert(path); - } - } - paths_.swap(surviving_paths_); - std::string post_removal_count = std::to_string(paths_.size()); - TRACE_EVENT_INSTANT1("flutter", "VolatilePathTracker::OnFrame", - "remaining_count", post_removal_count.c_str()); -} - -void VolatilePathTracker::Drain() { - if (needs_drain_) { - TRACE_EVENT0("flutter", "VolatilePathTracker::Drain"); - std::deque> paths_to_remove; - { - std::scoped_lock lock(paths_to_remove_mutex_); - paths_to_remove.swap(paths_to_remove_); - needs_drain_ = false; - } - std::string count = std::to_string(paths_to_remove.size()); - TRACE_EVENT_INSTANT1("flutter", "VolatilePathTracker::Drain", "count", - count.c_str()); - for (auto& path : paths_to_remove) { - paths_.erase(path); - } - } -} - -} // namespace flutter diff --git a/lib/ui/volatile_path_tracker.h b/lib/ui/volatile_path_tracker.h deleted file mode 100644 index 40f28311a008a..0000000000000 --- a/lib/ui/volatile_path_tracker.h +++ /dev/null @@ -1,82 +0,0 @@ -// 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. - -#ifndef FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_ -#define FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_ - -#include -#include -#include - -#include "flutter/fml/macros.h" -#include "flutter/fml/task_runner.h" -#include "flutter/fml/trace_event.h" -#include "third_party/skia/include/core/SkPath.h" - -namespace flutter { - -/// A cache for paths drawn from dart:ui. -/// -/// Whenever a flutter::CanvasPath is created, it must Insert an entry into -/// this cache. Whenever a frame is drawn, the shell must call OnFrame. The -/// cache will flip the volatility bit on the SkPath and remove it from the -/// cache. If the Dart object is released, Erase must be called to avoid -/// tracking a path that is no longer referenced in Dart code. -/// -/// Enabling this cache may cause difficult to predict minor pixel differences -/// when paths are rendered. If deterministic rendering is needed, e.g. for a -/// screen diffing test, this class will not cache any paths and will -/// automatically set the volatility of the path to false. -class VolatilePathTracker { - public: - /// The fields of this struct must only accessed on the UI task runner. - struct TrackedPath { - bool tracking_volatility = false; - int frame_count = 0; - SkPath path; - }; - - VolatilePathTracker(fml::RefPtr ui_task_runner, - bool enabled); - - static constexpr int kFramesOfVolatility = 2; - - // Starts tracking a path. - // Must be called from the UI task runner. - // - // Callers should only insert paths that are currently volatile. - void Insert(std::shared_ptr path); - - // Removes a path from tracking. - // - // May be called from any thread. - void Erase(std::shared_ptr path); - - // Called by the shell at the end of a frame after notifying Dart about idle - // time. - // - // This method will flip the volatility bit to false for any paths that have - // survived the |kFramesOfVolatility|. - // - // Must be called from the UI task runner. - void OnFrame(); - - bool enabled() const { return enabled_; } - - private: - fml::RefPtr ui_task_runner_; - std::atomic_bool needs_drain_ = false; - std::mutex paths_to_remove_mutex_; - std::deque> paths_to_remove_; - std::set> paths_; - bool enabled_ = true; - - void Drain(); - - FML_DISALLOW_COPY_AND_ASSIGN(VolatilePathTracker); -}; - -} // namespace flutter - -#endif // FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 9cb2fce5e91a6..6ea5a2256cf43 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -92,8 +92,7 @@ std::weak_ptr DartIsolate::SpawnIsolate( GetIOManager(), GetSkiaUnrefQueue(), GetImageDecoder(), advisory_script_uri, advisory_script_entrypoint, flags, isolate_create_callback, isolate_shutdown_callback, dart_entrypoint, - dart_entrypoint_library, std::move(isolate_configration), - GetVolatilePathTracker(), this); + dart_entrypoint_library, std::move(isolate_configration), this); } std::weak_ptr DartIsolate::CreateRunningRootIsolate( @@ -114,7 +113,6 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( std::optional dart_entrypoint, std::optional dart_entrypoint_library, std::unique_ptr isolate_configration, - std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate) { if (!isolate_snapshot) { FML_LOG(ERROR) << "Invalid isolate snapshot."; @@ -142,8 +140,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( advisory_script_entrypoint, // isolate_flags, // isolate_create_callback, // - isolate_shutdown_callback, // - std::move(volatile_path_tracker) // + isolate_shutdown_callback // ) .lock(); @@ -176,8 +173,8 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( } if (settings.root_isolate_create_callback) { - // Isolate callbacks always occur in isolate scope and before user code has - // had a chance to run. + // Isolate callbacks always occur in isolate scope and before user code + // has had a chance to run. tonic::DartState::Scope scope(isolate.get()); settings.root_isolate_create_callback(*isolate.get()); } @@ -215,7 +212,6 @@ std::weak_ptr DartIsolate::CreateRootIsolate( Flags flags, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, - std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate) { TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate"); @@ -235,17 +231,16 @@ std::weak_ptr DartIsolate::CreateRootIsolate( auto isolate_data = std::make_unique>( std::shared_ptr(new DartIsolate( - settings, // settings - task_runners, // task runners - std::move(snapshot_delegate), // snapshot delegate - std::move(hint_freed_delegate), // hint freed delegate - std::move(io_manager), // IO manager - std::move(unref_queue), // Skia unref queue - std::move(image_decoder), // Image Decoder - advisory_script_uri, // advisory URI - advisory_script_entrypoint, // advisory entrypoint - true, // is_root_isolate - std::move(volatile_path_tracker) // volatile path tracker + settings, // settings + task_runners, // task runners + std::move(snapshot_delegate), // snapshot delegate + std::move(hint_freed_delegate), // hint freed delegate + std::move(io_manager), // IO manager + std::move(unref_queue), // Skia unref queue + std::move(image_decoder), // Image Decoder + advisory_script_uri, // advisory URI + advisory_script_entrypoint, // advisory entrypoint + true // is_root_isolate ))); DartErrorString error; @@ -286,18 +281,16 @@ std::weak_ptr DartIsolate::CreateRootIsolate( return (*root_isolate_data)->GetWeakIsolatePtr(); } -DartIsolate::DartIsolate( - const Settings& settings, - TaskRunners task_runners, - fml::WeakPtr snapshot_delegate, - fml::WeakPtr hint_freed_delegate, - fml::WeakPtr io_manager, - fml::RefPtr unref_queue, - fml::WeakPtr image_decoder, - std::string advisory_script_uri, - std::string advisory_script_entrypoint, - bool is_root_isolate, - std::shared_ptr volatile_path_tracker) +DartIsolate::DartIsolate(const Settings& settings, + TaskRunners task_runners, + fml::WeakPtr snapshot_delegate, + fml::WeakPtr hint_freed_delegate, + fml::WeakPtr io_manager, + fml::RefPtr unref_queue, + fml::WeakPtr image_decoder, + std::string advisory_script_uri, + std::string advisory_script_entrypoint, + bool is_root_isolate) : UIDartState(std::move(task_runners), settings.task_observer_add, settings.task_observer_remove, @@ -312,7 +305,6 @@ DartIsolate::DartIsolate( settings.unhandled_exception_callback, DartVMRef::GetIsolateNameServer(), is_root_isolate, - std::move(volatile_path_tracker), settings.enable_skparagraph), may_insecurely_connect_to_all_domains_( settings.may_insecurely_connect_to_all_domains), @@ -733,8 +725,8 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, bool DartIsolate::Shutdown() { TRACE_EVENT0("flutter", "DartIsolate::Shutdown"); // This call may be re-entrant since Dart_ShutdownIsolate can invoke the - // cleanup callback which deletes the embedder side object of the dart isolate - // (a.k.a. this). + // cleanup callback which deletes the embedder side object of the dart + // isolate (a.k.a. this). if (phase_ == Phase::Shutdown) { return false; } @@ -762,7 +754,8 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( if (!vm_data) { *error = fml::strdup( - "Could not access VM data to initialize isolates. This may be because " + "Could not access VM data to initialize isolates. This may be " + "because " "the VM has initialized shutdown on another thread already."); return nullptr; } @@ -780,8 +773,8 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( #if (FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DEBUG) // TODO(68663): The service isolate in debug mode is always launched without - // sound null safety. Fix after the isolate snapshot data is created with the - // right flags. + // sound null safety. Fix after the isolate snapshot data is created with + // the right flags. flags->null_safety = vm_data->GetIsolateSnapshot()->IsNullSafetyEnabled(nullptr); #endif @@ -801,8 +794,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint DartIsolate::Flags{flags}, // flags nullptr, // isolate create callback - nullptr, // isolate shutdown callback - nullptr // volatile path tracker + nullptr // isolate shutdown callback ); std::shared_ptr service_isolate = weak_service_isolate.lock(); @@ -870,8 +862,9 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( // The VM attempts to start the VM service for us on |Dart_Initialize|. In // such a case, the callback data will be null and the script URI will be // DART_VM_SERVICE_ISOLATE_NAME. In such cases, we just create the service - // isolate like normal but dont hold a reference to it at all. We also start - // this isolate since we will never again reference it from the engine. + // isolate like normal but dont hold a reference to it at all. We also + // start this isolate since we will never again reference it from the + // engine. return DartCreateAndStartServiceIsolate(package_root, // package_config, // flags, // @@ -913,8 +906,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( fml::WeakPtr{}, // image_decoder advisory_script_uri, // advisory_script_uri advisory_script_entrypoint, // advisory_script_entrypoint - false, // is_root_isolate - nullptr))); // volatile path tracker + false))); // is_root_isolate Dart_Isolate vm_isolate = CreateDartIsolateGroup( std::move(isolate_group_data), std::move(isolate_data), flags, error, @@ -971,8 +963,7 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, (*isolate_group_data)->GetAdvisoryScriptURI(), // advisory_script_uri (*isolate_group_data) ->GetAdvisoryScriptEntrypoint(), // advisory_script_entrypoint - false, // is_root_isolate - nullptr))); // volatile path tracker + false))); // is_root_isolate // root isolate should have been created via CreateRootIsolate if (!InitializeIsolate(*embedder_isolate, isolate, error)) { @@ -1006,7 +997,8 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( return nullptr; } - // Ownership of the isolate data objects has been transferred to the Dart VM. + // Ownership of the isolate data objects has been transferred to the Dart + // VM. std::shared_ptr embedder_isolate(*isolate_data); isolate_group_data.release(); isolate_data.release(); @@ -1036,9 +1028,9 @@ bool DartIsolate::InitializeIsolate( return false; } - // Root isolates will be setup by the engine and the service isolate (which is - // also a root isolate) by the utility routines in the VM. However, secondary - // isolates will be run by the VM if they are marked as runnable. + // Root isolates will be setup by the engine and the service isolate (which + // is also a root isolate) by the utility routines in the VM. However, + // secondary isolates will be run by the VM if they are marked as runnable. if (!embedder_isolate->IsRootIsolate()) { auto child_isolate_preparer = embedder_isolate->GetIsolateGroupData().GetChildIsolatePreparer(); diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index b4705ad9c7b76..a42ec378c3ea0 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -233,7 +233,6 @@ class DartIsolate : public UIDartState { std::optional dart_entrypoint, std::optional dart_entrypoint_library, std::unique_ptr isolate_configration, - std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate = nullptr); //---------------------------------------------------------------------------- @@ -464,7 +463,6 @@ class DartIsolate : public UIDartState { Flags flags, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, - std::shared_ptr volatile_path_tracker, const DartIsolate* spawning_isolate = nullptr); DartIsolate(const Settings& settings, @@ -476,8 +474,7 @@ class DartIsolate : public UIDartState { fml::WeakPtr image_decoder, std::string advisory_script_uri, std::string advisory_script_entrypoint, - bool is_root_isolate, - std::shared_ptr volatile_path_tracker); + bool is_root_isolate); [[nodiscard]] bool Initialize(Dart_Isolate isolate); diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 3b25c02dec0ae..92589d8663ea7 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -68,8 +68,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration), // isolate configuration - nullptr // Volatile path tracker + std::move(isolate_configuration) // isolate configuration ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); @@ -172,8 +171,7 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration), // isolate configuration - nullptr // Volatile path tracker + std::move(isolate_configuration) // isolate configuration ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); @@ -431,8 +429,7 @@ TEST_F(DartIsolateTest, CanCreateServiceIsolate) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration), // isolate configuration - nullptr // Volatile path tracker + std::move(isolate_configuration) // isolate configuration ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); @@ -532,8 +529,7 @@ TEST_F(DartIsolateTest, InvalidLoadingUnitFails) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration), // isolate configuration - nullptr // volatile path tracker + std::move(isolate_configuration) // isolate configuration ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate); diff --git a/runtime/dart_lifecycle_unittests.cc b/runtime/dart_lifecycle_unittests.cc index 09de19f6e1687..8576b3fb11292 100644 --- a/runtime/dart_lifecycle_unittests.cc +++ b/runtime/dart_lifecycle_unittests.cc @@ -73,8 +73,7 @@ static std::shared_ptr CreateAndRunRootIsolate( settings.isolate_shutdown_callback, // isolate shutdown callback, entrypoint, // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration), // isolate configuration - nullptr // Volatile path tracker + std::move(isolate_configuration) // isolate configuration ) .lock(); diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 42a6ea6bc9d67..55dd1e736315c 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -37,8 +37,7 @@ RuntimeController::RuntimeController( const PlatformData& p_platform_data, const fml::closure& p_isolate_create_callback, const fml::closure& p_isolate_shutdown_callback, - std::shared_ptr p_persistent_isolate_data, - std::shared_ptr p_volatile_path_tracker) + std::shared_ptr p_persistent_isolate_data) : client_(p_client), vm_(p_vm), isolate_snapshot_(std::move(p_isolate_snapshot)), @@ -54,8 +53,7 @@ RuntimeController::RuntimeController( platform_data_(std::move(p_platform_data)), isolate_create_callback_(p_isolate_create_callback), isolate_shutdown_callback_(p_isolate_shutdown_callback), - persistent_isolate_data_(std::move(p_persistent_isolate_data)), - volatile_path_tracker_(std::move(p_volatile_path_tracker)) {} + persistent_isolate_data_(std::move(p_persistent_isolate_data)) {} std::unique_ptr RuntimeController::Spawn( RuntimeDelegate& client, @@ -70,8 +68,7 @@ std::unique_ptr RuntimeController::Spawn( hint_freed_delegate_, io_manager_, unref_queue_, image_decoder_, advisory_script_uri, advisory_script_entrypoint, idle_notification_callback, platform_data_, isolate_create_callback, - isolate_shutdown_callback, persistent_isolate_data, - volatile_path_tracker_); + isolate_shutdown_callback, persistent_isolate_data); result->spawning_isolate_ = root_isolate_; return result; } @@ -114,8 +111,7 @@ std::unique_ptr RuntimeController::Clone() const { platform_data_, // isolate_create_callback_, // isolate_shutdown_callback_, // - persistent_isolate_data_, // - volatile_path_tracker_ // + persistent_isolate_data_ // )); } @@ -391,8 +387,7 @@ bool RuntimeController::LaunchRootIsolate( isolate_shutdown_callback_, // dart_entrypoint, // dart_entrypoint_library, // - std::move(isolate_configuration), // - volatile_path_tracker_ // + std::move(isolate_configuration) // ) .lock(); diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 09ff677814e17..4a836902c296f 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -15,7 +15,6 @@ #include "flutter/lib/ui/io_manager.h" #include "flutter/lib/ui/text/font_collection.h" #include "flutter/lib/ui/ui_dart_state.h" -#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/lib/ui/window/pointer_data_packet.h" #include "flutter/runtime/dart_vm.h" @@ -111,8 +110,6 @@ class RuntimeController : public PlatformConfigurationClient { /// @param[in] persistent_isolate_data Unstructured persistent read-only /// data that the root isolate can /// access in a synchronous manner. - /// @param[in] volatile_path_tracker Cache for tracking path - /// volatility. /// RuntimeController( RuntimeDelegate& client, @@ -130,8 +127,7 @@ class RuntimeController : public PlatformConfigurationClient { const PlatformData& platform_data, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, - std::shared_ptr persistent_isolate_data, - std::shared_ptr volatile_path_tracker); + std::shared_ptr persistent_isolate_data); //---------------------------------------------------------------------------- /// @brief Create a RuntimeController that shares as many resources as @@ -614,7 +610,6 @@ class RuntimeController : public PlatformConfigurationClient { const fml::closure isolate_create_callback_; const fml::closure isolate_shutdown_callback_; std::shared_ptr persistent_isolate_data_; - std::shared_ptr volatile_path_tracker_; PlatformConfiguration* GetPlatformConfigurationIfAvailable(); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index c25511da130e0..416ce577a7861 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -68,8 +68,7 @@ Engine::Engine(Delegate& delegate, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker) + fml::WeakPtr snapshot_delegate) : Engine(delegate, dispatcher_maker, vm.GetConcurrentWorkerTaskRunner(), @@ -95,8 +94,7 @@ Engine::Engine(Delegate& delegate, platform_data, // platform data settings_.isolate_create_callback, // isolate create callback settings_.isolate_shutdown_callback, // isolate shutdown callback - settings_.persistent_isolate_data, // persistent isolate data - std::move(volatile_path_tracker) // volatile path tracker + settings_.persistent_isolate_data // persistent isolate data ); } diff --git a/shell/common/engine.h b/shell/common/engine.h index a5581611e5c31..bbdd4d0b69bfc 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -19,7 +19,6 @@ #include "flutter/lib/ui/semantics/semantics_node.h" #include "flutter/lib/ui/snapshot_delegate.h" #include "flutter/lib/ui/text/font_collection.h" -#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/runtime/dart_vm.h" @@ -352,8 +351,7 @@ class Engine final : public RuntimeDelegate, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker); + fml::WeakPtr snapshot_delegate); //---------------------------------------------------------------------------- /// @brief Create a Engine that shares as many resources as diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 435138d7ce7af..cebcd4b8f8f1c 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -51,8 +51,7 @@ std::unique_ptr CreateEngine( std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker) { + fml::WeakPtr snapshot_delegate) { return std::make_unique(delegate, // dispatcher_maker, // vm, // @@ -63,8 +62,7 @@ std::unique_ptr CreateEngine( std::move(animator), // io_manager, // unref_queue, // - snapshot_delegate, // - volatile_path_tracker); + snapshot_delegate); } } // namespace @@ -82,11 +80,8 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( return nullptr; } - auto shell = std::unique_ptr( - new Shell(std::move(vm), task_runners, settings, - std::make_shared( - task_runners.GetUITaskRunner(), - !settings.skia_deterministic_rendering_on_cpu))); + auto shell = + std::unique_ptr(new Shell(std::move(vm), task_runners, settings)); // Create the rasterizer on the raster thread. std::promise> rasterizer_promise; @@ -182,18 +177,17 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( std::move(vsync_waiter)); engine_promise.set_value( - on_create_engine(*shell, // - dispatcher_maker, // - *shell->GetDartVM(), // - std::move(isolate_snapshot), // - task_runners, // - platform_data, // - shell->GetSettings(), // - std::move(animator), // - weak_io_manager_future.get(), // - unref_queue_future.get(), // - snapshot_delegate_future.get(), // - shell->volatile_path_tracker_)); + on_create_engine(*shell, // + dispatcher_maker, // + *shell->GetDartVM(), // + std::move(isolate_snapshot), // + task_runners, // + platform_data, // + shell->GetSettings(), // + std::move(animator), // + weak_io_manager_future.get(), // + unref_queue_future.get(), // + snapshot_delegate_future.get())); })); if (!shell->Setup(std::move(platform_view), // @@ -355,15 +349,11 @@ std::unique_ptr Shell::Create( return shell; } -Shell::Shell(DartVMRef vm, - TaskRunners task_runners, - Settings settings, - std::shared_ptr volatile_path_tracker) +Shell::Shell(DartVMRef vm, TaskRunners task_runners, Settings settings) : task_runners_(std::move(task_runners)), settings_(std::move(settings)), vm_(std::move(vm)), is_gpu_disabled_sync_switch_(new fml::SyncSwitch()), - volatile_path_tracker_(std::move(volatile_path_tracker)), weak_factory_gpu_(nullptr), weak_factory_(this) { FML_CHECK(vm_) << "Must have access to VM to create a shell."; @@ -491,8 +481,7 @@ std::unique_ptr Shell::Spawn( Settings settings, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker) { + fml::WeakPtr snapshot_delegate) { return engine->Spawn(/*delegate=*/delegate, /*dispatcher_maker=*/dispatcher_maker, /*settings=*/settings, @@ -1099,7 +1088,6 @@ void Shell::OnAnimatorNotifyIdle(int64_t deadline) { if (engine_) { engine_->NotifyIdle(deadline); - volatile_path_tracker_->OnFrame(); } } diff --git a/shell/common/shell.h b/shell/common/shell.h index 3749d4727bca7..4c9b688eb2ae0 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -27,7 +27,6 @@ #include "flutter/fml/time/time_point.h" #include "flutter/lib/ui/semantics/custom_accessibility_action.h" #include "flutter/lib/ui/semantics/semantics_node.h" -#include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/service_protocol.h" @@ -109,8 +108,7 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr animator, fml::WeakPtr io_manager, fml::RefPtr unref_queue, - fml::WeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker)> + fml::WeakPtr snapshot_delegate)> EngineCreateCallback; //---------------------------------------------------------------------------- @@ -412,7 +410,6 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr rasterizer_; // on raster task runner std::unique_ptr io_manager_; // on IO task runner std::shared_ptr is_gpu_disabled_sync_switch_; - std::shared_ptr volatile_path_tracker_; fml::WeakPtr weak_engine_; // to be shared across threads fml::TaskRunnerAffineWeakPtr @@ -464,10 +461,7 @@ class Shell final : public PlatformView::Delegate, sk_sp shared_resource_context_; - Shell(DartVMRef vm, - TaskRunners task_runners, - Settings settings, - std::shared_ptr volatile_path_tracker); + Shell(DartVMRef vm, TaskRunners task_runners, Settings settings); static std::unique_ptr CreateShellOnPlatformThread( DartVMRef vm, diff --git a/testing/dart_isolate_runner.cc b/testing/dart_isolate_runner.cc index 86f6c66892e9f..43d77c0165c28 100644 --- a/testing/dart_isolate_runner.cc +++ b/testing/dart_isolate_runner.cc @@ -56,8 +56,7 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( std::string entrypoint, const std::vector& args, const std::string& fixtures_path, - fml::WeakPtr io_manager, - std::shared_ptr volatile_path_tracker) { + fml::WeakPtr io_manager) { FML_CHECK(task_runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); if (!vm_ref) { @@ -127,8 +126,7 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( settings.isolate_shutdown_callback, // isolate shutdown callback entrypoint, // entrypoint std::nullopt, // library - std::move(isolate_configuration), // isolate configuration - std::move(volatile_path_tracker) // volatile path tracker + std::move(isolate_configuration) // isolate configuration ) .lock(); @@ -148,15 +146,14 @@ std::unique_ptr RunDartCodeInIsolate( std::string entrypoint, const std::vector& args, const std::string& fixtures_path, - fml::WeakPtr io_manager, - std::shared_ptr volatile_path_tracker) { + fml::WeakPtr io_manager) { std::unique_ptr result; fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask( task_runners.GetUITaskRunner(), fml::MakeCopyable([&]() mutable { result = RunDartCodeInIsolateOnUITaskRunner( vm_ref, settings, task_runners, entrypoint, args, fixtures_path, - io_manager, std::move(volatile_path_tracker)); + io_manager); latch.Signal(); })); latch.Wait(); diff --git a/testing/dart_isolate_runner.h b/testing/dart_isolate_runner.h index ab6029e38b02b..83c91a1e5c3a8 100644 --- a/testing/dart_isolate_runner.h +++ b/testing/dart_isolate_runner.h @@ -42,16 +42,14 @@ class AutoIsolateShutdown { FML_DISALLOW_COPY_AND_ASSIGN(AutoIsolateShutdown); }; -void RunDartCodeInIsolate( - DartVMRef& vm_ref, - std::unique_ptr& result, - const Settings& settings, - const TaskRunners& task_runners, - std::string entrypoint, - const std::vector& args, - const std::string& fixtures_path, - fml::WeakPtr io_manager = {}, - std::shared_ptr volatile_path_tracker = nullptr); +void RunDartCodeInIsolate(DartVMRef& vm_ref, + std::unique_ptr& result, + const Settings& settings, + const TaskRunners& task_runners, + std::string entrypoint, + const std::vector& args, + const std::string& fixtures_path, + fml::WeakPtr io_manager = {}); std::unique_ptr RunDartCodeInIsolate( DartVMRef& vm_ref, @@ -60,8 +58,7 @@ std::unique_ptr RunDartCodeInIsolate( std::string entrypoint, const std::vector& args, const std::string& fixtures_path, - fml::WeakPtr io_manager = {}, - std::shared_ptr volatile_path_tracker = nullptr); + fml::WeakPtr io_manager = {}); } // namespace testing } // namespace flutter diff --git a/third_party/tonic/tests/dart_state_unittest.cc b/third_party/tonic/tests/dart_state_unittest.cc index 8a91cc20b41ae..3d053de40e75b 100644 --- a/third_party/tonic/tests/dart_state_unittest.cc +++ b/third_party/tonic/tests/dart_state_unittest.cc @@ -44,8 +44,7 @@ TEST_F(DartState, IsShuttingDown) { settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint std::nullopt, // dart entrypoint library - std::move(isolate_configuration), // isolate configuration - nullptr // Volatile path tracker + std::move(isolate_configuration) // isolate configuration ); auto root_isolate = weak_isolate.lock(); ASSERT_TRUE(root_isolate);