diff --git a/DEPS b/DEPS index bc07e16223358..a45788ea7f4c8 100644 --- a/DEPS +++ b/DEPS @@ -18,7 +18,7 @@ vars = { 'llvm_git': 'https://llvm.googlesource.com', # OCMock is for testing only so there is no google clone 'ocmock_git': 'https://github.com/erikdoe/ocmock.git', - 'skia_revision': '030e21befbc98fe0e509560e4ce2a92505cc3c5b', + 'skia_revision': 'a4cce5236dcf5af8b3b8725559df76877802f209', # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. @@ -960,7 +960,7 @@ deps = { 'packages': [ { 'package': 'fuchsia/sdk/core/linux-amd64', - 'version': 'VcFEJiUUTYwkhEAlJ3KJjUXFFFwsKWYjZH00EOgSXJEC' + 'version': 'sD8HRA4JmXczujkqOVt6Ya0R5zAFyiM0TqOj1OcqAXEC' } ], 'condition': 'host_os == "linux" and not download_fuchsia_sdk', diff --git a/ci/licenses_golden/licenses_skia b/ci/licenses_golden/licenses_skia index 23fe4761e9f5a..0bd1322dcfd13 100644 --- a/ci/licenses_golden/licenses_skia +++ b/ci/licenses_golden/licenses_skia @@ -1,4 +1,4 @@ -Signature: 84d66ee8859e0c7c7f9c9dd0dfdf984f +Signature: 0d3bfc822abf779517a9eef059fd6c81 ==================================================================================================== LIBRARY: etc1 diff --git a/ci/licenses_golden/tool_signature b/ci/licenses_golden/tool_signature index 42b033e5febc0..124a0c03d3462 100644 --- a/ci/licenses_golden/tool_signature +++ b/ci/licenses_golden/tool_signature @@ -1,2 +1,2 @@ -Signature: 69d986a7b2275b798b574b6aea2c1b78 +Signature: 4b8678841cc9a099770862d08fb15c47 diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index b646c6dbe0bfc..3191257a14d44 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -206,12 +206,23 @@ void CanRenderTiledTexture(AiksTest* aiks_test, canvas.DrawRect(Rect::MakeXYWH(0, 0, 600, 600), paint); } - // Should not change the image. - PathBuilder path_builder; - path_builder.AddCircle({150, 150}, 150); - path_builder.AddRoundedRect(Rect::MakeLTRB(300, 300, 600, 600), 10); - paint.style = Paint::Style::kFill; - canvas.DrawPath(path_builder.TakePath(), paint); + { + // Should not change the image. + PathBuilder path_builder; + path_builder.AddCircle({150, 150}, 150); + path_builder.AddRoundedRect(Rect::MakeLTRB(300, 300, 600, 600), 10); + paint.style = Paint::Style::kFill; + canvas.DrawPath(path_builder.TakePath(), paint); + } + + { + // Should not change the image. Tests the Convex short-cut code. + PathBuilder path_builder; + path_builder.AddCircle({150, 450}, 150); + path_builder.SetConvexity(Convexity::kConvex); + paint.style = Paint::Style::kFill; + canvas.DrawPath(path_builder.TakePath(), paint); + } ASSERT_TRUE(aiks_test->OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } @@ -3744,6 +3755,29 @@ TEST_P(AiksTest, VerticesGeometryUVPositionData) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +// Regression test for https://github.com/flutter/flutter/issues/135441 . +TEST_P(AiksTest, VerticesGeometryUVPositionDataWithTranslate) { + Canvas canvas; + Paint paint; + auto texture = CreateTextureForFixture("table_mountain_nx.png"); + + paint.color_source = ColorSource::MakeImage( + texture, Entity::TileMode::kClamp, Entity::TileMode::kClamp, {}, + Matrix::MakeTranslation({100.0, 100.0})); + + auto vertices = {Point(0, 0), Point(texture->GetSize().width, 0), + Point(0, texture->GetSize().height)}; + std::vector indices = {0u, 1u, 2u}; + std::vector texture_coordinates = {}; + std::vector vertex_colors = {}; + auto geometry = std::make_shared( + vertices, indices, texture_coordinates, vertex_colors, + Rect::MakeLTRB(0, 0, 1, 1), VerticesGeometry::VertexMode::kTriangleStrip); + + canvas.DrawVertices(geometry, BlendMode::kSourceOver, paint); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, ClearBlendWithBlur) { Canvas canvas; Paint white; diff --git a/impeller/base/base_unittests.cc b/impeller/base/base_unittests.cc index 5edfdff6ccf1b..d869420f09aaf 100644 --- a/impeller/base/base_unittests.cc +++ b/impeller/base/base_unittests.cc @@ -76,5 +76,72 @@ TEST(StringsTest, CanSPrintF) { ASSERT_EQ(SPrintF("%sx%.2f", "Hello", 12.122222), "Hellox12.12"); } +struct CVTest { + Mutex mutex; + ConditionVariable cv; + uint32_t rando_ivar IPLR_GUARDED_BY(mutex) = 0; +}; + +TEST(ConditionVariableTest, WaitUntil) { + CVTest test; + // test.rando_ivar = 12; // <--- Static analysis error + for (size_t i = 0; i < 2; ++i) { + test.mutex.Lock(); // <--- Static analysis error without this. + auto result = test.cv.WaitUntil( + test.mutex, + std::chrono::high_resolution_clock::now() + + std::chrono::milliseconds{10}, + [&]() IPLR_REQUIRES(test.mutex) { + test.rando_ivar = 12; // <-- Static analysics error without the + // IPLR_REQUIRES on the pred. + return false; + }); + test.mutex.Unlock(); + ASSERT_FALSE(result); + } + Lock lock(test.mutex); // <--- Static analysis error without this. + // The predicate never returns true. So return has to be due to a non-spurious + // wake. + ASSERT_EQ(test.rando_ivar, 12u); +} + +TEST(ConditionVariableTest, WaitFor) { + CVTest test; + // test.rando_ivar = 12; // <--- Static analysis error + for (size_t i = 0; i < 2; ++i) { + test.mutex.Lock(); // <--- Static analysis error without this. + auto result = test.cv.WaitFor( + test.mutex, std::chrono::milliseconds{10}, + [&]() IPLR_REQUIRES(test.mutex) { + test.rando_ivar = 12; // <-- Static analysics error without the + // IPLR_REQUIRES on the pred. + return false; + }); + test.mutex.Unlock(); + ASSERT_FALSE(result); + } + Lock lock(test.mutex); // <--- Static analysis error without this. + // The predicate never returns true. So return has to be due to a non-spurious + // wake. + ASSERT_EQ(test.rando_ivar, 12u); +} + +TEST(ConditionVariableTest, WaitForever) { + CVTest test; + // test.rando_ivar = 12; // <--- Static analysis error + for (size_t i = 0; i < 2; ++i) { + test.mutex.Lock(); // <--- Static analysis error without this. + test.cv.Wait(test.mutex, [&]() IPLR_REQUIRES(test.mutex) { + test.rando_ivar = 12; // <-- Static analysics error without + // the IPLR_REQUIRES on the pred. + return true; + }); + test.mutex.Unlock(); + } + Lock lock(test.mutex); // <--- Static analysis error without this. + // The wake only happens when the predicate returns true. + ASSERT_EQ(test.rando_ivar, 12u); +} + } // namespace testing } // namespace impeller diff --git a/impeller/base/thread.h b/impeller/base/thread.h index 4ab2c7310b3c9..0f8dfd20bbef1 100644 --- a/impeller/base/thread.h +++ b/impeller/base/thread.h @@ -4,6 +4,9 @@ #pragma once +#include +#include +#include #include #include #include @@ -14,6 +17,8 @@ namespace impeller { +class ConditionVariable; + class IPLR_CAPABILITY("mutex") Mutex { public: Mutex() = default; @@ -25,6 +30,8 @@ class IPLR_CAPABILITY("mutex") Mutex { void Unlock() IPLR_RELEASE() { mutex_.unlock(); } private: + friend class ConditionVariable; + std::mutex mutex_; Mutex(const Mutex&) = delete; @@ -124,4 +131,134 @@ class IPLR_SCOPED_CAPABILITY WriterLock { WriterLock& operator=(WriterLock&&) = delete; }; +//------------------------------------------------------------------------------ +/// @brief A condition variable exactly similar to the one in libcxx with +/// two major differences: +/// +/// * On the Wait, WaitFor, and WaitUntil calls, static analysis +/// annotation are respected. +/// * There is no ability to wait on a condition variable and also +/// be susceptible to spurious wakes. This is because the +/// predicate is mandatory. +/// +class ConditionVariable { + public: + ConditionVariable() = default; + + ~ConditionVariable() = default; + + ConditionVariable(const ConditionVariable&) = delete; + + ConditionVariable& operator=(const ConditionVariable&) = delete; + + void NotifyOne() { cv_.notify_one(); } + + void NotifyAll() { cv_.notify_all(); } + + using Predicate = std::function; + + //---------------------------------------------------------------------------- + /// @brief Atomically unlocks the mutex and waits on the condition + /// variable up to a specified time point. Spurious wakes may + /// happen before the time point is reached. In such cases the + /// predicate is invoked and it must return `false` for the wait + /// to continue. The predicate will be invoked with the mutex + /// locked. + /// + /// @note Since the predicate is invoked with the mutex locked, if it + /// accesses other guarded resources, the predicate itself must be + /// decorated with the IPLR_REQUIRES directive. For instance, + /// + /// ```c++ + /// [] () IPLR_REQUIRES(mutex) { + /// return my_guarded_resource.should_stop_waiting; + /// } + /// ``` + /// + /// @param mutex The mutex. + /// @param[in] time_point The time point to wait to. + /// @param[in] should_stop_waiting The predicate invoked on spurious wakes. + /// Must return false for the wait to + /// continue. + /// + /// @tparam Clock The clock type. + /// @tparam Duration The duration type. + /// + /// @return The value of the predicate at the end of the wait. + /// + template + bool WaitUntil(Mutex& mutex, + const std::chrono::time_point& time_point, + const Predicate& should_stop_waiting) IPLR_REQUIRES(mutex) { + std::unique_lock lock(mutex.mutex_, std::adopt_lock); + return cv_.wait_until(lock, time_point, should_stop_waiting); + } + + //---------------------------------------------------------------------------- + /// @brief Atomically unlocks the mutex and waits on the condition + /// variable for a designated duration. Spurious wakes may happen + /// before the time point is reached. In such cases the predicate + /// is invoked and it must return `false` for the wait to + /// continue. The predicate will be invoked with the mutex locked. + /// + /// @note Since the predicate is invoked with the mutex locked, if it + /// accesses other guarded resources, the predicate itself must be + /// decorated with the IPLR_REQUIRES directive. For instance, + /// + /// ```c++ + /// [] () IPLR_REQUIRES(mutex) { + /// return my_guarded_resource.should_stop_waiting; + /// } + /// ``` + /// + /// @param mutex The mutex. + /// @param[in] duration The duration to wait for. + /// @param[in] should_stop_waiting The predicate invoked on spurious wakes. + /// Must return false for the wait to + /// continue. + /// + /// @tparam Representation The duration representation type. + /// @tparam Period The duration period type. + /// + /// @return The value of the predicate at the end of the wait. + /// + template + bool WaitFor(Mutex& mutex, + const std::chrono::duration& duration, + const Predicate& should_stop_waiting) IPLR_REQUIRES(mutex) { + return WaitUntil(mutex, std::chrono::steady_clock::now() + duration, + should_stop_waiting); + } + + //---------------------------------------------------------------------------- + /// @brief Atomically unlocks the mutex and waits on the condition + /// variable indefinitely till the predicate determines that the + /// wait must end. Spurious wakes may happen before the time point + /// is reached. In such cases the predicate is invoked and it must + /// return `false` for the wait to continue. The predicate will be + /// invoked with the mutex locked. + /// + /// @note Since the predicate is invoked with the mutex locked, if it + /// accesses other guarded resources, the predicate itself must be + /// decorated with the IPLR_REQUIRES directive. For instance, + /// + /// ```c++ + /// [] () IPLR_REQUIRES(mutex) { + /// return my_guarded_resource.should_stop_waiting; + /// } + /// ``` + /// + /// @param mutex The mutex + /// @param[in] should_stop_waiting The should stop waiting + /// + void Wait(Mutex& mutex, const Predicate& should_stop_waiting) + IPLR_REQUIRES(mutex) { + std::unique_lock lock(mutex.mutex_, std::adopt_lock); + cv_.wait(lock, should_stop_waiting); + } + + private: + std::condition_variable cv_; +}; + } // namespace impeller diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index 1153572f25c08..4cd025aef0422 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -82,6 +82,9 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( RenderPass& pass) { using VS = TextureFillVertexShader; + auto uv_transform = + texture_coverage.GetNormalizingTransform() * effect_transform; + if (path_.GetFillType() == FillType::kNonZero && // path_.IsConvex()) { auto [points, indices] = TessellateConvex( @@ -93,9 +96,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( for (auto i = 0u; i < points.size(); i++) { VS::PerVertexData data; data.position = points[i]; - data.texture_coords = effect_transform * - (points[i] - texture_coverage.origin) / - texture_coverage.size; + data.texture_coords = uv_transform * points[i]; vertex_builder.AppendVertex(data); } for (auto i = 0u; i < indices.size(); i++) { @@ -116,16 +117,14 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( auto tesselation_result = renderer.GetTessellator()->Tessellate( path_.GetFillType(), path_.CreatePolyline(entity.GetTransformation().GetMaxBasisLength()), - [&vertex_builder, &texture_coverage, &effect_transform]( + [&vertex_builder, &uv_transform]( const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { for (auto i = 0u; i < vertices_count * 2; i += 2) { VS::PerVertexData data; Point vtx = {vertices[i], vertices[i + 1]}; data.position = vtx; - data.texture_coords = effect_transform * - (vtx - texture_coverage.origin) / - texture_coverage.size; + data.texture_coords = uv_transform * vtx; vertex_builder.AppendVertex(data); } FML_DCHECK(vertex_builder.GetVertexCount() == vertices_count); diff --git a/impeller/entity/geometry/geometry.cc b/impeller/entity/geometry/geometry.cc index cff1770f134fb..29d880a86c86b 100644 --- a/impeller/entity/geometry/geometry.cc +++ b/impeller/entity/geometry/geometry.cc @@ -75,12 +75,13 @@ GeometryResult ComputeUVGeometryForRect(Rect source_rect, RenderPass& pass) { auto& host_buffer = pass.GetTransientsBuffer(); + auto uv_transform = + texture_coverage.GetNormalizingTransform() * effect_transform; std::vector data(8); auto points = source_rect.GetPoints(); for (auto i = 0u, j = 0u; i < 8; i += 2, j++) { data[i] = points[j]; - data[i + 1] = effect_transform * (points[j] - texture_coverage.origin) / - texture_coverage.size; + data[i + 1] = uv_transform * points[j]; } return GeometryResult{ diff --git a/impeller/entity/geometry/vertices_geometry.cc b/impeller/entity/geometry/vertices_geometry.cc index 23a1c32a5f0b1..42af1e49c0a46 100644 --- a/impeller/entity/geometry/vertices_geometry.cc +++ b/impeller/entity/geometry/vertices_geometry.cc @@ -227,8 +227,8 @@ GeometryResult VerticesGeometry::GetPositionUVBuffer( auto index_count = indices_.size(); auto vertex_count = vertices_.size(); - auto size = texture_coverage.size; - auto origin = texture_coverage.origin; + auto uv_transform = + texture_coverage.GetNormalizingTransform() * effect_transform; auto has_texture_coordinates = HasTextureCoordinates(); std::vector vertex_data(vertex_count); { @@ -236,9 +236,7 @@ GeometryResult VerticesGeometry::GetPositionUVBuffer( auto vertex = vertices_[i]; auto texture_coord = has_texture_coordinates ? texture_coordinates_[i] : vertices_[i]; - auto uv = - effect_transform * Point((texture_coord.x - origin.x) / size.width, - (texture_coord.y - origin.y) / size.height); + auto uv = uv_transform * texture_coord; // From experimentation we need to clamp these values to < 1.0 or else // there can be flickering. vertex_data[i] = { diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h index fa33757318d17..4c5fbcefffc12 100644 --- a/impeller/geometry/rect.h +++ b/impeller/geometry/rect.h @@ -214,6 +214,35 @@ struct TRect { return TRect::MakePointBounds(points.begin(), points.end()).value(); } + /// @brief Constructs a Matrix that will map all points in the coordinate + /// space of the rectangle into a new normalized coordinate space + /// where the upper left corner of the rectangle maps to (0, 0) + /// and the lower right corner of the rectangle maps to (1, 1). + /// + /// Empty and non-finite rectangles will return a zero-scaling + /// transform that maps all points to (0, 0). + constexpr Matrix GetNormalizingTransform() const { + if (!IsEmpty()) { + Scalar sx = 1.0 / size.width; + Scalar sy = 1.0 / size.height; + Scalar tx = origin.x * -sx; + Scalar ty = origin.y * -sy; + + // Exclude NaN and infinities and either scale underflowing to zero + if (sx != 0.0 && sy != 0.0 && 0.0 * sx * sy * tx * ty == 0.0) { + // clang-format off + return Matrix( sx, 0.0f, 0.0f, 0.0f, + 0.0f, sy, 0.0f, 0.0f, + 0.0f, 0.0f, 1.0f, 0.0f, + tx, ty, 0.0f, 1.0f); + // clang-format on + } + } + + // Map all coordinates to the origin. + return Matrix::MakeScale({0.0f, 0.0f, 1.0f}); + } + constexpr TRect Union(const TRect& o) const { auto this_ltrb = GetLTRB(); auto other_ltrb = o.GetLTRB(); diff --git a/impeller/geometry/rect_unittests.cc b/impeller/geometry/rect_unittests.cc index 27bedfab38f09..c9abe94b1244b 100644 --- a/impeller/geometry/rect_unittests.cc +++ b/impeller/geometry/rect_unittests.cc @@ -14,14 +14,14 @@ namespace testing { TEST(RectTest, RectOriginSizeGetters) { { Rect r = Rect::MakeOriginSize({10, 20}, {50, 40}); - ASSERT_EQ(r.GetOrigin(), Point(10, 20)); - ASSERT_EQ(r.GetSize(), Size(50, 40)); + EXPECT_EQ(r.GetOrigin(), Point(10, 20)); + EXPECT_EQ(r.GetSize(), Size(50, 40)); } { Rect r = Rect::MakeLTRB(10, 20, 50, 40); - ASSERT_EQ(r.GetOrigin(), Point(10, 20)); - ASSERT_EQ(r.GetSize(), Size(40, 20)); + EXPECT_EQ(r.GetOrigin(), Point(10, 20)); + EXPECT_EQ(r.GetSize(), Size(40, 20)); } } @@ -44,14 +44,152 @@ TEST(RectTest, RectMakeSize) { Size s(100, 200); IRect r = IRect::MakeSize(s); IRect expected = IRect::MakeLTRB(0, 0, 100, 200); - ASSERT_EQ(r, expected); + EXPECT_EQ(r, expected); } { ISize s(100, 200); IRect r = IRect::MakeSize(s); IRect expected = IRect::MakeLTRB(0, 0, 100, 200); - ASSERT_EQ(r, expected); + EXPECT_EQ(r, expected); + } +} + +TEST(RectTest, RectGetNormalizingTransform) { + { + // Checks for expected matrix values + + auto r = Rect::MakeXYWH(100, 200, 200, 400); + + EXPECT_EQ(r.GetNormalizingTransform(), + Matrix::MakeScale({0.005, 0.0025, 1.0}) * + Matrix::MakeTranslation({-100, -200})); + } + + { + // Checks for expected transformation of points relative to the rect + + auto r = Rect::MakeLTRB(300, 500, 400, 700); + auto m = r.GetNormalizingTransform(); + + // The 4 corners of the rect => (0, 0) to (1, 1) + EXPECT_EQ(m * Point(300, 500), Point(0, 0)); + EXPECT_EQ(m * Point(400, 500), Point(1, 0)); + EXPECT_EQ(m * Point(400, 700), Point(1, 1)); + EXPECT_EQ(m * Point(300, 700), Point(0, 1)); + + // The center => (0.5, 0.5) + EXPECT_EQ(m * Point(350, 600), Point(0.5, 0.5)); + + // Outside the 4 corners => (-1, -1) to (2, 2) + EXPECT_EQ(m * Point(200, 300), Point(-1, -1)); + EXPECT_EQ(m * Point(500, 300), Point(2, -1)); + EXPECT_EQ(m * Point(500, 900), Point(2, 2)); + EXPECT_EQ(m * Point(200, 900), Point(-1, 2)); + } + + { + // Checks for behavior with empty rects + + auto zero = Matrix::MakeScale({0.0, 0.0, 1.0}); + + // Empty for width and/or height == 0 + EXPECT_EQ(Rect::MakeXYWH(10, 10, 0, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, 0).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 0, 0).GetNormalizingTransform(), zero); + + // Empty for width and/or height < 0 + EXPECT_EQ(Rect::MakeXYWH(10, 10, -1, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, -1).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, -1, -1).GetNormalizingTransform(), zero); + } + + { + // Checks for behavior with non-finite rects + + auto z = Matrix::MakeScale({0.0, 0.0, 1.0}); + auto nan = std::numeric_limits::quiet_NaN(); + auto inf = std::numeric_limits::infinity(); + + // Non-finite for width and/or height == nan + EXPECT_EQ(Rect::MakeXYWH(10, 10, nan, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, nan).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, nan, nan).GetNormalizingTransform(), z); + + // Non-finite for width and/or height == inf + EXPECT_EQ(Rect::MakeXYWH(10, 10, inf, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, inf).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, inf, inf).GetNormalizingTransform(), z); + + // Non-finite for width and/or height == -inf + EXPECT_EQ(Rect::MakeXYWH(10, 10, -inf, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, -inf).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, -inf, -inf).GetNormalizingTransform(), z); + + // Non-finite for origin X and/or Y == nan + EXPECT_EQ(Rect::MakeXYWH(nan, 10, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, nan, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(nan, nan, 10, 10).GetNormalizingTransform(), z); + + // Non-finite for origin X and/or Y == inf + EXPECT_EQ(Rect::MakeXYWH(inf, 10, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, inf, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(inf, inf, 10, 10).GetNormalizingTransform(), z); + + // Non-finite for origin X and/or Y == -inf + EXPECT_EQ(Rect::MakeXYWH(-inf, 10, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, -inf, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(-inf, -inf, 10, 10).GetNormalizingTransform(), z); + } +} + +TEST(RectTest, IRectGetNormalizingTransform) { + { + // Checks for expected matrix values + + auto r = IRect::MakeXYWH(100, 200, 200, 400); + + EXPECT_EQ(r.GetNormalizingTransform(), + Matrix::MakeScale({0.005, 0.0025, 1.0}) * + Matrix::MakeTranslation({-100, -200})); + } + + { + // Checks for expected transformation of points relative to the rect + + auto r = IRect::MakeLTRB(300, 500, 400, 700); + auto m = r.GetNormalizingTransform(); + + // The 4 corners of the rect => (0, 0) to (1, 1) + EXPECT_EQ(m * Point(300, 500), Point(0, 0)); + EXPECT_EQ(m * Point(400, 500), Point(1, 0)); + EXPECT_EQ(m * Point(400, 700), Point(1, 1)); + EXPECT_EQ(m * Point(300, 700), Point(0, 1)); + + // The center => (0.5, 0.5) + EXPECT_EQ(m * Point(350, 600), Point(0.5, 0.5)); + + // Outside the 4 corners => (-1, -1) to (2, 2) + EXPECT_EQ(m * Point(200, 300), Point(-1, -1)); + EXPECT_EQ(m * Point(500, 300), Point(2, -1)); + EXPECT_EQ(m * Point(500, 900), Point(2, 2)); + EXPECT_EQ(m * Point(200, 900), Point(-1, 2)); + } + + { + // Checks for behavior with empty rects + + auto zero = Matrix::MakeScale({0.0, 0.0, 1.0}); + + // Empty for width and/or height == 0 + EXPECT_EQ(IRect::MakeXYWH(10, 10, 0, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, 10, 0).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, 0, 0).GetNormalizingTransform(), zero); + + // Empty for width and/or height < 0 + EXPECT_EQ(IRect::MakeXYWH(10, 10, -1, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, 10, -1).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, -1, -1).GetNormalizingTransform(), zero); } } diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index ce20224042438..85fca6d417605 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:js_interop'; import 'dart:math' as math; @@ -25,7 +26,7 @@ const bool _debugLogPointerEvents = false; const bool _debugLogFlutterEvents = false; /// The signature of a callback that handles pointer events. -typedef _PointerDataCallback = void Function(Iterable); +typedef _PointerDataCallback = void Function(DomEvent event, List); // The mask for the bitfield of event buttons. Buttons not contained in this // mask are cut off. @@ -103,11 +104,14 @@ class PointerBinding { } } + final ClickDebouncer clickDebouncer = ClickDebouncer(); + /// Performs necessary clean up for PointerBinding including removing event listeners /// and clearing the existing pointer state void dispose() { _adapter.clearListeners(); _pointerDataConverter.clearPointerState(); + clickDebouncer.reset(); } final DomElement flutterViewElement; @@ -156,20 +160,259 @@ class PointerBinding { // TODO(dit): remove old API fallbacks, https://github.com/flutter/flutter/issues/116141 _BaseAdapter _createAdapter() { if (_detector.hasPointerEvents) { - return _PointerAdapter(_onPointerData, flutterViewElement, _pointerDataConverter, _keyboardConverter); + return _PointerAdapter(clickDebouncer.onPointerData, flutterViewElement, _pointerDataConverter, _keyboardConverter); } // Fallback for Safari Mobile < 13. To be removed. if (_detector.hasTouchEvents) { - return _TouchAdapter(_onPointerData, flutterViewElement, _pointerDataConverter, _keyboardConverter); + return _TouchAdapter(clickDebouncer.onPointerData, flutterViewElement, _pointerDataConverter, _keyboardConverter); } // Fallback for Safari Desktop < 13. To be removed. if (_detector.hasMouseEvents) { - return _MouseAdapter(_onPointerData, flutterViewElement, _pointerDataConverter, _keyboardConverter); + return _MouseAdapter(clickDebouncer.onPointerData, flutterViewElement, _pointerDataConverter, _keyboardConverter); } throw UnsupportedError('This browser does not support pointer, touch, or mouse events.'); } +} + +@visibleForTesting +typedef QueuedEvent = ({ DomEvent event, Duration timeStamp, List data }); + +@visibleForTesting +typedef DebounceState = ({ + DomElement target, + Timer timer, + List queue, +}); + +/// Disambiguates taps and clicks that are produced both by the framework from +/// `pointerdown`/`pointerup` events and those detected as DOM "click" events by +/// the browser. +/// +/// The implementation is waiting for a `pointerdown`, and as soon as it sees +/// one stops forwarding pointer events to the framework, and instead queues +/// them in a list. The queuing process stops as soon as one of the following +/// two conditions happens first: +/// +/// * 200ms passes after the `pointerdown` event. Most clicks, even slow ones, +/// are typically done by then. Importantly, screen readers simulate clicks +/// much faster than 200ms. So if the timer expires, it is likely the user is +/// not interested in producing a click, so the debouncing process stops and +/// all queued events are forwarded to the framework. If, for example, a +/// tappable node is inside a scrollable viewport, the events can be +/// intrepreted by the framework to initiate scrolling. +/// * A `click` event arrives. If the event queue has not been flushed to the +/// framework, the event is forwarded to the framework as a +/// `SemanticsAction.tap`, and all the pointer events are dropped. If, by the +/// time the click event arrives, the queue was flushed (but no more than 50ms +/// ago), then the click event is dropped instead under the assumption that +/// the flushed pointer events are interpreted by the framework as the desired +/// gesture. +/// +/// This mechanism is in place to deal with https://github.com/flutter/flutter/issues/130162. +class ClickDebouncer { + DebounceState? _state; + + @visibleForTesting + DebounceState? get debugState => _state; + + // The timestamp of the last "pointerup" DOM event that was flushed. + // + // Not to be confused with the time when it was flushed. The two may be far + // apart because the flushing can happen after a delay due to timer, or events + // that happen after the said "pointerup". + Duration? _lastFlushedPointerUpTimeStamp; + + /// Returns true if the debouncer has a non-empty queue of pointer events that + /// were withheld from the framework. + /// + /// This value is normally false, and it flips to true when the first + /// pointerdown is observed that lands on a tappable semantics node, denoted + /// by the presence of the `flt-tappable` attribute. + bool get isDebouncing => _state != null; + + /// Processes a pointer event. + /// + /// If semantics are off, simply forwards the event to the framework. + /// + /// If currently debouncing events (see [isDebouncing]), adds the event to + /// the debounce queue, unless the target of the event is different from the + /// target that initiated the debouncing process, in which case stops + /// debouncing and flushes pointer events to the framework. + /// + /// If the event is a `pointerdown` and the target is `flt-tappable`, begins + /// debouncing events. + /// + /// In all other situations forwards the event to the framework. + void onPointerData(DomEvent event, List data) { + if (!EnginePlatformDispatcher.instance.semanticsEnabled) { + _sendToFramework(event, data); + return; + } + + if (isDebouncing) { + _debounce(event, data); + } else if (event.type == 'pointerdown') { + _startDebouncing(event, data); + } else { + _sendToFramework(event, data); + } + } + + /// Notifies the debouncer of the browser-detected "click" DOM event. + /// + /// Forwards the event to the framework, unless it is deduplicated because + /// the corresponding pointer down/up events were recently flushed to the + /// framework already. + void onClick(DomEvent click, int semanticsNodeId, bool isListening) { + assert(click.type == 'click'); + + if (!isDebouncing) { + // There's no pending queue of pointer events that are being debounced. It + // is a standalone click event. Unless pointer down/up were flushed + // recently and if the node is currently listening to event, forward to + // the framework. + if (isListening && _shouldSendClickEventToFramework(click)) { + _sendSemanticsTapToFramework(click, semanticsNodeId); + } + return; + } + + if (isListening) { + // There's a pending queue of pointer events. Prefer sending the tap action + // instead of pointer events, because the pointer events may not land on the + // combined semantic node and miss the click/tap. + final DebounceState state = _state!; + _state = null; + state.timer.cancel(); + _sendSemanticsTapToFramework(click, semanticsNodeId); + } else { + // The semantic node is not listening to taps. Flush the pointer events + // for the framework to figure out what to do with them. It's possible + // the framework is interested in gestures other than taps. + _flush(); + } + } + + void _sendSemanticsTapToFramework(DomEvent click, int semanticsNodeId) { + // Tappable nodes can be nested inside other tappable nodes. If a click + // lands on an inner element and is allowed to propagate, it will also + // land on the ancestor tappable, leading to both the descendant and the + // ancestor sending SemanticsAction.tap to the framework, creating a double + // tap/click, which is wrong. More details: + // + // https://github.com/flutter/flutter/issues/134842 + click.stopPropagation(); + + EnginePlatformDispatcher.instance.invokeOnSemanticsAction( + semanticsNodeId, ui.SemanticsAction.tap, null); + } + + void _startDebouncing(DomEvent event, List data) { + assert( + _state == null, + 'Cannot start debouncing. Already debouncing.' + ); + assert( + event.type == 'pointerdown', + 'Click debouncing must begin with a pointerdown' + ); + + final DomEventTarget? target = event.target; + if (target is DomElement && target.hasAttribute('flt-tappable')) { + _state = ( + target: target, + // The 200ms duration was chosen empirically by testing tapping, mouse + // clicking, trackpad tapping and clicking, as well as the following + // screen readers: TalkBack on Android, VoiceOver on macOS, Narrator/ + // NVDA/JAWS on Windows. 200ms seemed to hit the sweet spot by + // satisfying the following: + // * It was short enough that delaying the `pointerdown` still allowed + // drag gestures to begin reasonably soon (e.g. scrolling). + // * It was long enough to register taps and clicks. + // * It was successful at detecting taps generated by all tested + // screen readers. + timer: Timer(const Duration(milliseconds: 200), _onTimerExpired), + queue: [( + event: event, + timeStamp: _BaseAdapter._eventTimeStampToDuration(event.timeStamp!), + data: data, + )], + ); + } else { + // The event landed on an non-tappable target. Assume this won't lead to + // double clicks and forward the event to the framework. + _sendToFramework(event, data); + } + } + + void _debounce(DomEvent event, List data) { + assert( + _state != null, + 'Cannot debounce event. Debouncing state not established by _startDebouncing.' + ); - void _onPointerData(Iterable data) { + final DebounceState state = _state!; + state.queue.add(( + event: event, + timeStamp: _BaseAdapter._eventTimeStampToDuration(event.timeStamp!), + data: data, + )); + + // It's only interesting to debounce clicks when both `pointerdown` and + // `pointerup` land on the same element. + if (event.type == 'pointerup') { + // TODO(yjbanov): this is a bit mouthful, but see https://github.com/dart-lang/sdk/issues/53070 + final DomEventTarget? eventTarget = event.target; + final DomElement stateTarget = state.target; + final bool targetChanged = eventTarget != stateTarget; + if (targetChanged) { + _flush(); + } + } + } + + void _onTimerExpired() { + if (!isDebouncing) { + return; + } + _flush(); + } + + // If the click event happens soon after the last `pointerup` event that was + // already flushed to the framework, the click event is dropped to avoid + // double click. + bool _shouldSendClickEventToFramework(DomEvent click) { + final Duration? lastFlushedPointerUpTimeStamp = _lastFlushedPointerUpTimeStamp; + + if (lastFlushedPointerUpTimeStamp == null) { + // We haven't seen a pointerup. It's standalone click event. Let it through. + return true; + } + + final Duration clickTimeStamp = _BaseAdapter._eventTimeStampToDuration(click.timeStamp!); + final Duration delta = clickTimeStamp - lastFlushedPointerUpTimeStamp; + return delta >= const Duration(milliseconds: 50); + } + + void _flush() { + assert(_state != null); + + final DebounceState state = _state!; + state.timer.cancel(); + + final List aggregateData = []; + for (final QueuedEvent queuedEvent in state.queue) { + if (queuedEvent.event.type == 'pointerup') { + _lastFlushedPointerUpTimeStamp = queuedEvent.timeStamp; + } + aggregateData.addAll(queuedEvent.data); + } + + _sendToFramework(null, aggregateData); + _state = null; + } + + void _sendToFramework(DomEvent? event, List data) { final ui.PointerDataPacket packet = ui.PointerDataPacket(data: data.toList()); if (_debugLogFlutterEvents) { for(final ui.PointerData datum in data) { @@ -178,6 +421,16 @@ class PointerBinding { } EnginePlatformDispatcher.instance.invokeOnPointerDataPacket(packet); } + + /// Cancels any pending debounce process and forgets anything that happened so + /// far. + /// + /// This object can be used as if it was just initialized. + void reset() { + _state?.timer.cancel(); + _state = null; + _lastFlushedPointerUpTimeStamp = null; + } } class PointerSupportDetector { @@ -197,53 +450,47 @@ class _Listener { required this.event, required this.target, required this.handler, - required this.isNative, }); - /// Registers a listener for the given [event] on [target] using the Dart-to-JS API. + /// Registers a listener for the given `event` on a `target`. + /// + /// If `passive` is null uses the default behavior determined by the event + /// type. If `passive` is true, marks the handler as non-blocking for the + /// built-in browser behavior. This means the browser will not wait for the + /// handler to finish execution before performing the default action + /// associated with this event. If `passive` is false, the browser will wait + /// for the handler to finish execution before performing the respective + /// action. factory _Listener.register({ required String event, required DomEventTarget target, required DartDomEventListener handler, + bool? passive, }) { final DomEventListener jsHandler = createDomEventListener(handler); - final _Listener listener = _Listener._( - event: event, - target: target, - handler: jsHandler, - isNative: false, - ); - target.addEventListener(event, jsHandler); - return listener; - } - /// Registers a listener for the given [event] on [target] using the native JS API. - factory _Listener.registerNative({ - required String event, - required DomEventTarget target, - required DomEventListener jsHandler, - bool passive = false, - }) { - final Map eventOptions = { - 'passive': passive, - }; + if (passive == null) { + target.addEventListener(event, jsHandler); + } else { + final Map eventOptions = { + 'passive': passive, + }; + target.addEventListenerWithOptions(event, jsHandler, eventOptions); + } + final _Listener listener = _Listener._( event: event, target: target, handler: jsHandler, - isNative: true, ); - target.addEventListenerWithOptions(event, jsHandler, eventOptions); + target.addEventListener(event, jsHandler); return listener; } final String event; - final DomEventTarget target; final DomEventListener handler; - final bool isNative; - void unregister() { target.removeEventListener(event, handler); } @@ -477,10 +724,11 @@ mixin _WheelEventListenerMixin on _BaseAdapter { } void _addWheelEventListener(DartDomEventListener handler) { - _listeners.add(_Listener.registerNative( + _listeners.add(_Listener.register( event: 'wheel', target: flutterViewElement, - jsHandler: createDomEventListener(handler), + handler: handler, + passive: false, )); } @@ -490,7 +738,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter { if (_debugLogPointerEvents) { print(event.type); } - _callback(_convertWheelEventToPointerData(event)); + _callback(e, _convertWheelEventToPointerData(event)); // Prevent default so mouse wheel event doesn't get converted to // a scroll event that semantic nodes would process. // @@ -739,7 +987,7 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { buttons: event.buttons!.toInt(), ); _convertEventsToPointerData(data: pointerData, event: event, details: down); - _callback(pointerData); + _callback(event, pointerData); }); // Why `domWindow` you ask? See this fiddle: https://jsfiddle.net/ditman/7towxaqp @@ -756,7 +1004,7 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { final _SanitizedDetails move = sanitizer.sanitizeMoveEvent(buttons: event.buttons!.toInt()); _convertEventsToPointerData(data: pointerData, event: event, details: move); } - _callback(pointerData); + _callback(event, pointerData); }); _addPointerEventListener(flutterViewElement, 'pointerleave', (DomPointerEvent event) { @@ -766,7 +1014,7 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { final _SanitizedDetails? details = sanitizer.sanitizeLeaveEvent(buttons: event.buttons!.toInt()); if (details != null) { _convertEventsToPointerData(data: pointerData, event: event, details: details); - _callback(pointerData); + _callback(event, pointerData); } }, checkModifiers: false); @@ -779,7 +1027,7 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { _removePointerIfUnhoverable(event); if (details != null) { _convertEventsToPointerData(data: pointerData, event: event, details: details); - _callback(pointerData); + _callback(event, pointerData); } } }); @@ -795,7 +1043,7 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { final _SanitizedDetails details = _getSanitizer(device).sanitizeCancelEvent(); _removePointerIfUnhoverable(event); _convertEventsToPointerData(data: pointerData, event: event, details: details); - _callback(pointerData); + _callback(event, pointerData); } }, checkModifiers: false); @@ -934,7 +1182,7 @@ class _TouchAdapter extends _BaseAdapter { ); } } - _callback(pointerData); + _callback(event, pointerData); }); _addTouchEventListener(flutterViewElement, 'touchmove', (DomTouchEvent event) { @@ -953,7 +1201,7 @@ class _TouchAdapter extends _BaseAdapter { ); } } - _callback(pointerData); + _callback(event, pointerData); }); _addTouchEventListener(flutterViewElement, 'touchend', (DomTouchEvent event) { @@ -975,7 +1223,7 @@ class _TouchAdapter extends _BaseAdapter { ); } } - _callback(pointerData); + _callback(event, pointerData); }); _addTouchEventListener(flutterViewElement, 'touchcancel', (DomTouchEvent event) { @@ -994,7 +1242,7 @@ class _TouchAdapter extends _BaseAdapter { ); } } - _callback(pointerData); + _callback(event, pointerData); }); } @@ -1091,7 +1339,7 @@ class _MouseAdapter extends _BaseAdapter with _WheelEventListenerMixin { buttons: event.buttons!.toInt(), ); _convertEventsToPointerData(data: pointerData, event: event, details: sanitizedDetails); - _callback(pointerData); + _callback(event, pointerData); }); // Why `domWindow` you ask? See this fiddle: https://jsfiddle.net/ditman/7towxaqp @@ -1103,7 +1351,7 @@ class _MouseAdapter extends _BaseAdapter with _WheelEventListenerMixin { } final _SanitizedDetails move = _sanitizer.sanitizeMoveEvent(buttons: event.buttons!.toInt()); _convertEventsToPointerData(data: pointerData, event: event, details: move); - _callback(pointerData); + _callback(event, pointerData); }); _addMouseEventListener(flutterViewElement, 'mouseleave', (DomMouseEvent event) { @@ -1111,7 +1359,7 @@ class _MouseAdapter extends _BaseAdapter with _WheelEventListenerMixin { final _SanitizedDetails? details = _sanitizer.sanitizeLeaveEvent(buttons: event.buttons!.toInt()); if (details != null) { _convertEventsToPointerData(data: pointerData, event: event, details: details); - _callback(pointerData); + _callback(event, pointerData); } }); @@ -1121,7 +1369,7 @@ class _MouseAdapter extends _BaseAdapter with _WheelEventListenerMixin { final _SanitizedDetails? sanitizedDetails = _sanitizer.sanitizeUpEvent(buttons: event.buttons?.toInt()); if (sanitizedDetails != null) { _convertEventsToPointerData(data: pointerData, event: event, details: sanitizedDetails); - _callback(pointerData); + _callback(event, pointerData); } }); diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 82d9a2d874659..b27c9233f10a2 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -426,6 +426,7 @@ abstract class PrimaryRoleManager { /// Initializes a role for a [semanticsObject] that includes basic /// functionality for focus, labels, live regions, and route names. PrimaryRoleManager.withBasics(this.role, this.semanticsObject) { + element = _initElement(createElement(), semanticsObject); addFocusManagement(); addLiveRegion(); addRouteName(); @@ -438,9 +439,11 @@ abstract class PrimaryRoleManager { /// Use this constructor for highly specialized cases where /// [RoleManager.withBasics] does not work, for example when the default focus /// management intereferes with the widget's functionality. - PrimaryRoleManager.blank(this.role, this.semanticsObject); + PrimaryRoleManager.blank(this.role, this.semanticsObject) { + element = _initElement(createElement(), semanticsObject); + } - late final DomElement element = _initElement(createElement(), semanticsObject); + late final DomElement element; /// The primary role identifier. final PrimaryRole role; @@ -2028,8 +2031,9 @@ class EngineSemanticsOwner { } /// Receives DOM events from the pointer event system to correlate with the - /// semantics events; returns true if the event should be forwarded to the - /// framework. + /// semantics events. + /// + /// Returns true if the event should be forwarded to the framework. /// /// The browser sends us both raw pointer events and gestures from /// [SemanticsObject.element]s. There could be three possibilities: diff --git a/lib/web_ui/lib/src/engine/semantics/tappable.dart b/lib/web_ui/lib/src/engine/semantics/tappable.dart index cba0fafeb7650..d3612ae80348c 100644 --- a/lib/web_ui/lib/src/engine/semantics/tappable.dart +++ b/lib/web_ui/lib/src/engine/semantics/tappable.dart @@ -2,12 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; -import '../dom.dart'; -import '../platform_dispatcher.dart'; -import 'semantics.dart'; - /// Sets the "button" ARIA role. class Button extends PrimaryRoleManager { Button(SemanticsObject semanticsObject) : super.withBasics(PrimaryRole.button, semanticsObject) { @@ -34,42 +31,45 @@ class Button extends PrimaryRoleManager { /// click as [ui.SemanticsAction.tap]. class Tappable extends RoleManager { Tappable(SemanticsObject semanticsObject, PrimaryRoleManager owner) - : super(Role.tappable, semanticsObject, owner); + : super(Role.tappable, semanticsObject, owner) { + _clickListener = createDomEventListener((DomEvent click) { + PointerBinding.instance!.clickDebouncer.onClick( + click, + semanticsObject.id, + _isListening, + ); + }); + owner.element.addEventListener('click', _clickListener); + } DomEventListener? _clickListener; + bool _isListening = false; @override void update() { - if (_clickListener == null) { - _clickListener = createDomEventListener((DomEvent event) { - // Stop dom from reacting since it will be handled entirely on the - // flutter side. - event.preventDefault(); - if (!semanticsObject.isTappable || semanticsObject.enabledState() == EnabledState.disabled) { - return; - } - if (semanticsObject.owner.gestureMode != GestureMode.browserGestures) { - return; - } - EnginePlatformDispatcher.instance.invokeOnSemanticsAction( - semanticsObject.id, ui.SemanticsAction.tap, null); - }); - owner.addEventListener('click', _clickListener); + final bool wasListening = _isListening; + _isListening = semanticsObject.enabledState() != EnabledState.disabled && semanticsObject.isTappable; + if (wasListening != _isListening) { + _updateAttribute(); } } - void _stopListening() { - if (_clickListener == null) { - return; + void _updateAttribute() { + // The `flt-tappable` attribute marks the element for the ClickDebouncer to + // to know that it should debounce click events on this element. The + // contract is that the element that has this attribute is also the element + // that receives pointer and "click" events. + if (_isListening) { + owner.element.setAttribute('flt-tappable', ''); + } else { + owner.element.removeAttribute('flt-tappable'); } - - owner.removeEventListener('click', _clickListener); - _clickListener = null; } @override void dispose() { + owner.removeEventListener('click', _clickListener); + _clickListener = null; super.dispose(); - _stopListening(); } } diff --git a/lib/web_ui/test/common/matchers.dart b/lib/web_ui/test/common/matchers.dart index a2bd9fe66102a..51698843d66a8 100644 --- a/lib/web_ui/test/common/matchers.dart +++ b/lib/web_ui/test/common/matchers.dart @@ -227,23 +227,23 @@ enum HtmlComparisonMode { /// Rewrites [htmlContent] by removing irrelevant style attributes. /// -/// If [throwOnUnusedAttributes] is `true`, throws instead of rewriting. Set -/// [throwOnUnusedAttributes] to `true` to check that expected HTML strings do +/// If [throwOnUnusedStyleProperties] is `true`, throws instead of rewriting. Set +/// [throwOnUnusedStyleProperties] to `true` to check that expected HTML strings do /// not contain irrelevant attributes. It is ok for actual HTML to contain all /// kinds of attributes. They only need to be filtered out before testing. String canonicalizeHtml( String htmlContent, { HtmlComparisonMode mode = HtmlComparisonMode.nonLayoutOnly, - bool throwOnUnusedAttributes = false, - List? ignoredAttributes, + bool throwOnUnusedStyleProperties = false, + List? ignoredStyleProperties, }) { if (htmlContent.trim().isEmpty) { return ''; } - String? unusedAttribute(String name) { - if (throwOnUnusedAttributes) { - fail('Provided HTML contains style attribute "$name" which ' + String? unusedStyleProperty(String name) { + if (throwOnUnusedStyleProperties) { + fail('Provided HTML contains style property "$name" which ' 'is not used for comparison in the test. The HTML was:\n\n$htmlContent'); } @@ -293,17 +293,23 @@ String canonicalizeHtml( html_package.Element.tag(replacementTag); if (mode != HtmlComparisonMode.noAttributes) { - original.attributes.forEach((dynamic name, String value) { - if (name is! String) { - throw ArgumentError('"$name" should be String but was ${name.runtimeType}.'); - } + // Sort the attributes so tests are not sensitive to their order, which + // does not matter in terms of functionality. + final List attributeNames = original.attributes.keys.cast().toList(); + attributeNames.sort(); + for (final String name in attributeNames) { + final String value = original.attributes[name]!; if (name == 'style') { - return; + // The style attribute is handled separately because it contains substructure. + continue; } - if (name.startsWith('aria-')) { + + // These are the only attributes we're interested in testing. This list + // can change over time. + if (name.startsWith('aria-') || name.startsWith('flt-') || name == 'role') { replacement.attributes[name] = value; } - }); + } if (original.attributes.containsKey('style')) { final String styleValue = original.attributes['style']!; @@ -323,7 +329,7 @@ String canonicalizeHtml( if (parts.length == 2) { final String name = parts.first; - if (ignoredAttributes != null && ignoredAttributes.contains(name)) { + if (ignoredStyleProperties != null && ignoredStyleProperties.contains(name)) { return null; } @@ -337,7 +343,7 @@ String canonicalizeHtml( ].contains(name); if (isStaticAttribute) { - return unusedAttribute(name); + return unusedStyleProperty(name); } // Whether the attribute is set by the layout system. @@ -357,7 +363,7 @@ String canonicalizeHtml( if (forLayout && !isLayoutAttribute || !forLayout && isLayoutAttribute) { - return unusedAttribute(name); + return unusedStyleProperty(name); } } } @@ -372,7 +378,7 @@ String canonicalizeHtml( replacement.attributes['style'] = processedAttributes; } } - } else if (throwOnUnusedAttributes && original.attributes.isNotEmpty) { + } else if (throwOnUnusedStyleProperties && original.attributes.isNotEmpty) { fail('Provided HTML contains attributes. However, the comparison mode ' 'is $mode. The HTML was:\n\n$htmlContent'); } @@ -408,7 +414,7 @@ String canonicalizeHtml( void expectHtml(DomElement element, String expectedHtml, {HtmlComparisonMode mode = HtmlComparisonMode.nonLayoutOnly}) { expectedHtml = - canonicalizeHtml(expectedHtml, mode: mode, throwOnUnusedAttributes: true); + canonicalizeHtml(expectedHtml, mode: mode, throwOnUnusedStyleProperties: true); final String actualHtml = canonicalizeHtml(element.outerHTML!, mode: mode); expect(actualHtml, expectedHtml); } diff --git a/lib/web_ui/test/engine/pointer_binding_test.dart b/lib/web_ui/test/engine/pointer_binding_test.dart index ddcc1c83f523d..f936af366d891 100644 --- a/lib/web_ui/test/engine/pointer_binding_test.dart +++ b/lib/web_ui/test/engine/pointer_binding_test.dart @@ -3133,6 +3133,385 @@ void testMain() { packets.clear(); }, ); + + group('ClickDebouncer', () { + _testClickDebouncer(); + }); +} + +typedef CapturedSemanticsEvent = ({ + ui.SemanticsAction type, + int nodeId, +}); + +void _testClickDebouncer() { + final DateTime testTime = DateTime(2018, 12, 17); + late List pointerPackets; + late List semanticsActions; + late _PointerEventContext context; + late PointerBinding binding; + + void testWithSemantics( + String description, + Future Function() body, + ) { + test( + description, + () async { + EngineSemanticsOwner.instance + ..debugOverrideTimestampFunction(() => testTime) + ..semanticsEnabled = true; + await body(); + EngineSemanticsOwner.instance.semanticsEnabled = false; + }, + ); + } + + setUp(() { + context = _PointerEventContext(); + pointerPackets = []; + semanticsActions = []; + ui.window.onPointerDataPacket = (ui.PointerDataPacket packet) { + for (final ui.PointerData data in packet.data) { + pointerPackets.add(data.change); + } + }; + EnginePlatformDispatcher.instance.onSemanticsActionEvent = (ui.SemanticsActionEvent event) { + semanticsActions.add((type: event.type, nodeId: event.nodeId)); + }; + binding = PointerBinding.instance!; + binding.debugOverrideDetector(context); + binding.clickDebouncer.reset(); + }); + + tearDown(() { + binding.clickDebouncer.reset(); + }); + + test('Forwards to framework when semantics is off', () { + expect(EnginePlatformDispatcher.instance.semanticsEnabled, false); + expect(binding.clickDebouncer.isDebouncing, false); + binding.flutterViewElement.dispatchEvent(context.primaryDown()); + expect(pointerPackets, [ + ui.PointerChange.add, + ui.PointerChange.down, + ]); + expect(binding.clickDebouncer.isDebouncing, false); + expect(semanticsActions, isEmpty); + }); + + testWithSemantics('Forwards to framework when not debouncing', () async { + expect(EnginePlatformDispatcher.instance.semanticsEnabled, true); + expect(binding.clickDebouncer.isDebouncing, false); + + // This test DOM element is missing the `flt-tappable` attribute on purpose + // so that the debouncer does not debounce events and simply lets + // everything through. + final DomElement testElement = createDomElement('flt-semantics'); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + + testElement.dispatchEvent(context.primaryDown()); + testElement.dispatchEvent(context.primaryUp()); + expect(binding.clickDebouncer.isDebouncing, false); + + expect(pointerPackets, [ + ui.PointerChange.add, + ui.PointerChange.down, + ui.PointerChange.up, + ]); + expect(semanticsActions, isEmpty); + }); + + testWithSemantics('Accumulates pointer events starting from pointerdown', () async { + expect(EnginePlatformDispatcher.instance.semanticsEnabled, true); + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + + testElement.dispatchEvent(context.primaryDown()); + expect( + reason: 'Should start debouncing at first pointerdown', + binding.clickDebouncer.isDebouncing, + true, + ); + + testElement.dispatchEvent(context.primaryUp()); + expect( + reason: 'Should still be debouncing after pointerup', + binding.clickDebouncer.isDebouncing, + true, + ); + + expect( + reason: 'Events are withheld from the framework while debouncing', + pointerPackets, + [], + ); + expect( + binding.clickDebouncer.debugState!.target, + testElement, + ); + expect( + binding.clickDebouncer.debugState!.timer.isActive, + isTrue, + ); + expect( + binding.clickDebouncer.debugState!.queue.map((QueuedEvent e) => e.event.type), + ['pointerdown', 'pointerup'], + ); + + await Future.delayed(const Duration(milliseconds: 250)); + expect( + reason: 'Should stop debouncing after timer expires.', + binding.clickDebouncer.isDebouncing, + false, + ); + expect( + reason: 'Queued up events should be flushed to the framework.', + pointerPackets, + [ + ui.PointerChange.add, + ui.PointerChange.down, + ui.PointerChange.up, + ], + ); + expect(semanticsActions, isEmpty); + }); + + testWithSemantics('Flushes events to framework when target changes', () async { + expect(EnginePlatformDispatcher.instance.semanticsEnabled, true); + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + + testElement.dispatchEvent(context.primaryDown()); + expect( + reason: 'Should start debouncing at first pointerdown', + binding.clickDebouncer.isDebouncing, + true, + ); + + final DomElement newTarget = createDomElement('flt-semantics'); + newTarget.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(newTarget); + newTarget.dispatchEvent(context.primaryUp()); + + expect( + reason: 'Should stop debouncing when target changes.', + binding.clickDebouncer.isDebouncing, + false, + ); + expect( + reason: 'The state should be cleaned up after stopping debouncing.', + binding.clickDebouncer.debugState, + isNull, + ); + expect( + reason: 'Queued up events should be flushed to the framework.', + pointerPackets, + [ + ui.PointerChange.add, + ui.PointerChange.down, + ui.PointerChange.up, + ], + ); + expect(semanticsActions, isEmpty); + }); + + testWithSemantics('Forwards click to framework when not debouncing but listening', () async { + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + + final DomEvent click = createDomMouseEvent( + 'click', + { + 'clientX': testElement.getBoundingClientRect().x, + 'clientY': testElement.getBoundingClientRect().y, + } + ); + + binding.clickDebouncer.onClick(click, 42, true); + expect(binding.clickDebouncer.isDebouncing, false); + expect(pointerPackets, isEmpty); + expect(semanticsActions, [ + (type: ui.SemanticsAction.tap, nodeId: 42) + ]); + }); + + testWithSemantics('Forwards click to framework when debouncing and listening', () async { + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + testElement.dispatchEvent(context.primaryDown()); + expect(binding.clickDebouncer.isDebouncing, true); + + final DomEvent click = createDomMouseEvent( + 'click', + { + 'clientX': testElement.getBoundingClientRect().x, + 'clientY': testElement.getBoundingClientRect().y, + } + ); + + binding.clickDebouncer.onClick(click, 42, true); + expect(pointerPackets, isEmpty); + expect(semanticsActions, [ + (type: ui.SemanticsAction.tap, nodeId: 42) + ]); + }); + + testWithSemantics('Dedupes click if debouncing but not listening', () async { + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + testElement.dispatchEvent(context.primaryDown()); + expect(binding.clickDebouncer.isDebouncing, true); + + final DomEvent click = createDomMouseEvent( + 'click', + { + 'clientX': testElement.getBoundingClientRect().x, + 'clientY': testElement.getBoundingClientRect().y, + } + ); + + binding.clickDebouncer.onClick(click, 42, false); + expect( + reason: 'When tappable declares that it is not listening to click events ' + 'the debouncer flushes the pointer events to the framework and ' + 'lets it sort it out.', + pointerPackets, + [ + ui.PointerChange.add, + ui.PointerChange.down, + ], + ); + expect(semanticsActions, isEmpty); + }); + + testWithSemantics('Dedupes click if pointer down/up flushed recently', () async { + expect(EnginePlatformDispatcher.instance.semanticsEnabled, true); + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + + testElement.dispatchEvent(context.primaryDown()); + + // Simulate the user holding the pointer down for some time before releasing, + // such that the pointerup event happens close to timer expiration. This + // will create the situation that the click event arrives just after the + // pointerup is flushed. Forwarding the click to the framework would look + // like a double-click, so the click event is deduped. + await Future.delayed(const Duration(milliseconds: 190)); + + testElement.dispatchEvent(context.primaryUp()); + expect(binding.clickDebouncer.isDebouncing, true); + expect( + reason: 'Timer has not expired yet', + pointerPackets, isEmpty, + ); + + // Wait for the timer to expire to make sure pointer events are flushed. + await Future.delayed(const Duration(milliseconds: 20)); + + expect( + reason: 'Queued up events should be flushed to the framework because the ' + 'time expired before the click event arrived.', + pointerPackets, + [ + ui.PointerChange.add, + ui.PointerChange.down, + ui.PointerChange.up, + ], + ); + + final DomEvent click = createDomMouseEvent( + 'click', + { + 'clientX': testElement.getBoundingClientRect().x, + 'clientY': testElement.getBoundingClientRect().y, + } + ); + binding.clickDebouncer.onClick(click, 42, true); + + expect( + reason: 'Because the DOM click event was deduped.', + semanticsActions, + isEmpty, + ); + }); + + + testWithSemantics('Forwards click if enough time passed after the last flushed pointerup', () async { + expect(EnginePlatformDispatcher.instance.semanticsEnabled, true); + expect(binding.clickDebouncer.isDebouncing, false); + + final DomElement testElement = createDomElement('flt-semantics'); + testElement.setAttribute('flt-tappable', ''); + EnginePlatformDispatcher.instance.implicitView!.dom.semanticsHost.appendChild(testElement); + + testElement.dispatchEvent(context.primaryDown()); + + // Simulate the user holding the pointer down for some time before releasing, + // such that the pointerup event happens close to timer expiration. This + // makes it possible for the click to arrive early. However, this test in + // particular will delay the click to check that the delay is checked + // correctly. The inverse situation was already tested in the previous test. + await Future.delayed(const Duration(milliseconds: 190)); + + testElement.dispatchEvent(context.primaryUp()); + expect(binding.clickDebouncer.isDebouncing, true); + expect( + reason: 'Timer has not expired yet', + pointerPackets, isEmpty, + ); + + // Wait for the timer to expire to make sure pointer events are flushed. + await Future.delayed(const Duration(milliseconds: 100)); + + expect( + reason: 'Queued up events should be flushed to the framework because the ' + 'time expired before the click event arrived.', + pointerPackets, + [ + ui.PointerChange.add, + ui.PointerChange.down, + ui.PointerChange.up, + ], + ); + + final DomEvent click = createDomMouseEvent( + 'click', + { + 'clientX': testElement.getBoundingClientRect().x, + 'clientY': testElement.getBoundingClientRect().y, + } + ); + binding.clickDebouncer.onClick(click, 42, true); + + expect( + reason: 'The DOM click should still be sent to the framework because it ' + 'happened far enough from the last pointerup that it is unlikely ' + 'to be a duplicate.', + semanticsActions, + [ + (type: ui.SemanticsAction.tap, nodeId: 42) + ], + ); + }); } class MockSafariPointerEventWorkaround implements SafariPointerEventWorkaround { diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index f9abdde052297..8534fe009b77c 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -390,7 +390,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); @@ -400,7 +400,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); @@ -410,7 +410,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); @@ -433,7 +433,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); @@ -448,7 +448,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); expect(existingParent, tree[1]!.element.parent); @@ -482,7 +482,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); @@ -492,7 +492,7 @@ void _testEngineSemanticsOwner() { expectSemanticsTree(''' - + '''); @@ -1440,7 +1440,7 @@ void _testIncrementables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); final SemanticsObject node = semantics().debugSemanticsTree![0]!; @@ -1473,7 +1473,7 @@ void _testIncrementables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); final DomHTMLInputElement input = @@ -1506,7 +1506,7 @@ void _testIncrementables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); final DomHTMLInputElement input = @@ -1541,7 +1541,7 @@ void _testIncrementables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); semantics().semanticsEnabled = false; @@ -1684,7 +1684,7 @@ void _testCheckables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); final SemanticsObject node = semantics().debugSemanticsTree![0]!; @@ -1742,7 +1742,7 @@ void _testCheckables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); semantics().semanticsEnabled = false; @@ -1768,7 +1768,7 @@ void _testCheckables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); semantics().semanticsEnabled = false; @@ -1818,7 +1818,7 @@ void _testCheckables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); semantics().semanticsEnabled = false; @@ -1845,7 +1845,7 @@ void _testCheckables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); semantics().semanticsEnabled = false; @@ -1897,7 +1897,7 @@ void _testCheckables() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - + '''); semantics().semanticsEnabled = false; @@ -1970,7 +1970,7 @@ void _testTappable() { tester.apply(); expectSemanticsTree(''' - + '''); final SemanticsObject node = semantics().debugSemanticsTree![0]!; @@ -2031,14 +2031,14 @@ void _testTappable() { ''); updateTappable(enabled: true); - expectSemanticsTree(''); + expectSemanticsTree(''); updateTappable(enabled: false); expectSemanticsTree( ''); updateTappable(enabled: true); - expectSemanticsTree(''); + expectSemanticsTree(''); semantics().semanticsEnabled = false; }); @@ -2111,6 +2111,89 @@ void _testTappable() { semantics().semanticsEnabled = false; }); + + // Regression test for: https://github.com/flutter/flutter/issues/134842 + // + // If the click event is allowed to propagate through the hierarchy, then both + // the descendant and the parent will generate a SemanticsAction.tap, causing + // a double-tap to happen on the framework side. + test('inner tappable overrides ancestor tappable', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final List capturedActions = []; + EnginePlatformDispatcher.instance.onSemanticsActionEvent = (ui.SemanticsActionEvent event) { + capturedActions.add((event.nodeId, event.type, event.arguments)); + }; + + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + isFocusable: true, + hasTap: true, + hasEnabledState: true, + isEnabled: true, + isButton: true, + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + children: [ + tester.updateNode( + id: 1, + isFocusable: true, + hasTap: true, + hasEnabledState: true, + isEnabled: true, + isButton: true, + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ), + ], + ); + tester.apply(); + + expectSemanticsTree(''' + + + + + +'''); + + // Tap on the outer element + { + final DomElement element = tester.getSemanticsObject(0).element; + final DomRect rect = element.getBoundingClientRect(); + + element.dispatchEvent(createDomMouseEvent('click', { + 'clientX': (rect.left + (rect.right - rect.left) / 2).floor(), + 'clientY': (rect.top + (rect.bottom - rect.top) / 2).floor(), + })); + + expect(capturedActions, [ + (0, ui.SemanticsAction.tap, null), + ]); + } + + // Tap on the inner element + { + capturedActions.clear(); + final DomElement element = tester.getSemanticsObject(1).element; + final DomRect rect = element.getBoundingClientRect(); + + element.dispatchEvent(createDomMouseEvent('click', { + 'bubbles': true, + 'clientX': (rect.left + (rect.right - rect.left) / 2).floor(), + 'clientY': (rect.top + (rect.bottom - rect.top) / 2).floor(), + })); + + // The click on the inner element should not propagate to the parent to + // avoid sending a second SemanticsAction.tap action to the framework. + expect(capturedActions, [ + (1, ui.SemanticsAction.tap, null), + ]); + } + + semantics().semanticsEnabled = false; + }); } void _testImage() { @@ -2675,11 +2758,11 @@ void _testDialog() { tester.apply(); expectSemanticsTree(''' - + - + @@ -2768,7 +2851,7 @@ void _testDialog() { - + @@ -2905,9 +2988,9 @@ void _testFocusable() { } expectSemanticsTree(''' - + - + '''); diff --git a/lib/web_ui/test/engine/semantics/semantics_tester.dart b/lib/web_ui/test/engine/semantics/semantics_tester.dart index f0c61e92ca541..8a40c918f6d42 100644 --- a/lib/web_ui/test/engine/semantics/semantics_tester.dart +++ b/lib/web_ui/test/engine/semantics/semantics_tester.dart @@ -353,9 +353,9 @@ class SemanticsTester { /// Verifies the HTML structure of the current semantics tree. void expectSemanticsTree(String semanticsHtml) { - const List ignoredAttributes = ['pointer-events']; + const List ignoredStyleProperties = ['pointer-events']; expect( - canonicalizeHtml(rootElement.querySelector('flt-semantics')!.outerHTML!, ignoredAttributes: ignoredAttributes), + canonicalizeHtml(rootElement.querySelector('flt-semantics')!.outerHTML!, ignoredStyleProperties: ignoredStyleProperties), canonicalizeHtml(semanticsHtml), ); } diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 820a8629b4350..6db1564f58469 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -1027,12 +1027,12 @@ private static PointerCoords parsePointerCoords(Object rawCoords, float density) coords.orientation = (float) (double) coordsList.get(0); coords.pressure = (float) (double) coordsList.get(1); coords.size = (float) (double) coordsList.get(2); - coords.toolMajor = (float) (double) coordsList.get(3) * density; - coords.toolMinor = (float) (double) coordsList.get(4) * density; - coords.touchMajor = (float) (double) coordsList.get(5) * density; - coords.touchMinor = (float) (double) coordsList.get(6) * density; - coords.x = (float) (double) coordsList.get(7) * density; - coords.y = (float) (double) coordsList.get(8) * density; + coords.toolMajor = (float) ((double) coordsList.get(3) * density); + coords.toolMinor = (float) ((double) coordsList.get(4) * density); + coords.touchMajor = (float) ((double) coordsList.get(5) * density); + coords.touchMinor = (float) ((double) coordsList.get(6) * density); + coords.x = (float) ((double) coordsList.get(7) * density); + coords.y = (float) ((double) coordsList.get(8) * density); return coords; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 4aa073de93506..264ef06d162a5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -25,6 +25,7 @@ #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/embedder/embedder_engine.h" #include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" +#include "flutter/testing/stream_capture.h" #include "flutter/testing/test_dart_native_resolver.h" #include "gtest/gtest.h" @@ -122,7 +123,7 @@ + (void)registerWithRegistrar:(id)registrar { TEST_F(FlutterEngineTest, CanLaunch) { FlutterEngine* engine = GetFlutterEngine(); EXPECT_TRUE([engine runWithEntrypoint:@"main"]); - EXPECT_TRUE(engine.running); + ASSERT_TRUE(engine.running); } TEST_F(FlutterEngineTest, HasNonNullExecutableName) { @@ -189,23 +190,19 @@ + (void)registerWithRegistrar:(id)registrar { CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { latch.Signal(); })); // Replace stdout stream buffer with our own. - std::stringstream buffer; - std::streambuf* old_buffer = std::cout.rdbuf(); - std::cout.rdbuf(buffer.rdbuf()); + StreamCapture stdout_capture(&std::cout); // Launch the test entrypoint. FlutterEngine* engine = GetFlutterEngine(); EXPECT_TRUE([engine runWithEntrypoint:@"canLogToStdout"]); - EXPECT_TRUE(engine.running); + ASSERT_TRUE(engine.running); latch.Wait(); - // Restore old stdout stream buffer. - std::cout.rdbuf(old_buffer); + stdout_capture.Stop(); // Verify hello world was written to stdout. - std::string logs = buffer.str(); - EXPECT_TRUE(logs.find("Hello logging") != std::string::npos); + EXPECT_TRUE(stdout_capture.GetOutput().find("Hello logging") != std::string::npos); } // TODO(cbracken): Needs deflaking. https://github.com/flutter/flutter/issues/124677 @@ -227,7 +224,7 @@ + (void)registerWithRegistrar:(id)registrar { // Launch the test entrypoint. EXPECT_TRUE([engine runWithEntrypoint:@"backgroundTest"]); - EXPECT_TRUE(engine.running); + ASSERT_TRUE(engine.running); FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil @@ -257,7 +254,7 @@ + (void)registerWithRegistrar:(id)registrar { // Launch the test entrypoint. EXPECT_TRUE([engine runWithEntrypoint:@"backgroundTest"]); - EXPECT_TRUE(engine.running); + ASSERT_TRUE(engine.running); FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil @@ -479,7 +476,7 @@ + (void)registerWithRegistrar:(id)registrar { FlutterEngine* engine = GetFlutterEngine(); EXPECT_TRUE([engine runWithEntrypoint:@"nativeCallback"]); - EXPECT_TRUE(engine.running); + ASSERT_TRUE(engine.running); latch.Wait(); ASSERT_TRUE(latch_called); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h b/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h index b52524cd9dec5..a0a58d4516d82 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h @@ -5,6 +5,8 @@ #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h" #import + +#include "flutter/testing/autoreleasepool_test.h" #include "flutter/testing/test_dart_native_resolver.h" #include "gtest/gtest.h" @@ -36,6 +38,30 @@ class FlutterEngineTest : public ::testing::Test { // Returns a mock FlutterEngine that is able to work in environments // without a real pasteboard. +// +// Callers MUST call [mockEngine shutDownEngine] when finished with the returned engine. id CreateMockFlutterEngine(NSString* pasteboardString); +class MockFlutterEngineTest : public AutoreleasePoolTest { + public: + MockFlutterEngineTest(); + + void SetUp() override; + void TearDown() override; + + id GetMockEngine() { return engine_mock_; } + + void ShutDownEngine(); + + ~MockFlutterEngineTest() { + [engine_mock_ shutDownEngine]; + [engine_mock_ stopMocking]; + } + + private: + id engine_mock_; + + FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterEngineTest); +}; + } // namespace flutter::testing diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm index 8a535aa5e22e1..5e7ba44471ef1 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm @@ -67,4 +67,21 @@ id CreateMockFlutterEngine(NSString* pasteboardString) { } } +MockFlutterEngineTest::MockFlutterEngineTest() = default; + +void MockFlutterEngineTest::SetUp() { + engine_mock_ = CreateMockFlutterEngine(@""); +} + +void MockFlutterEngineTest::TearDown() { + [engine_mock_ shutDownEngine]; + [engine_mock_ stopMocking]; + engine_mock_ = nil; +} + +void MockFlutterEngineTest::ShutDownEngine() { + [engine_mock_ shutDownEngine]; + engine_mock_ = nil; +} + } // namespace flutter::testing diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index e5f5befd2ff2a..93579f5396d19 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -16,7 +16,8 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTestUtils.h" #include "flutter/shell/platform/embedder/test_utils/key_codes.g.h" -#import "flutter/testing/testing.h" +#include "flutter/testing/autoreleasepool_test.h" +#include "flutter/testing/testing.h" #pragma mark - Test Helper Classes @@ -92,18 +93,18 @@ - (void)mouseUp:(NSEvent*)event { @end @interface FlutterViewControllerTestObjC : NSObject -- (bool)testKeyEventsAreSentToFramework; -- (bool)testKeyEventsArePropagatedIfNotHandled; -- (bool)testKeyEventsAreNotPropagatedIfHandled; -- (bool)testCtrlTabKeyEventIsPropagated; -- (bool)testKeyEquivalentIsPassedToTextInputPlugin; -- (bool)testFlagsChangedEventsArePropagatedIfNotHandled; -- (bool)testKeyboardIsRestartedOnEngineRestart; -- (bool)testTrackpadGesturesAreSentToFramework; -- (bool)testMouseDownUpEventsSentToNextResponder; -- (bool)testModifierKeysAreSynthesizedOnMouseMove; -- (bool)testViewWillAppearCalledMultipleTimes; -- (bool)testFlutterViewIsConfigured; +- (bool)testKeyEventsAreSentToFramework:(id)mockEngine; +- (bool)testKeyEventsArePropagatedIfNotHandled:(id)mockEngine; +- (bool)testKeyEventsAreNotPropagatedIfHandled:(id)mockEngine; +- (bool)testCtrlTabKeyEventIsPropagated:(id)mockEngine; +- (bool)testKeyEquivalentIsPassedToTextInputPlugin:(id)mockEngine; +- (bool)testFlagsChangedEventsArePropagatedIfNotHandled:(id)mockEngine; +- (bool)testKeyboardIsRestartedOnEngineRestart:(id)mockEngine; +- (bool)testTrackpadGesturesAreSentToFramework:(id)mockEngine; +- (bool)testMouseDownUpEventsSentToNextResponder:(id)mockEngine; +- (bool)testModifierKeysAreSynthesizedOnMouseMove:(id)mockEngine; +- (bool)testViewWillAppearCalledMultipleTimes:(id)mockEngine; +- (bool)testFlutterViewIsConfigured:(id)mockEngine; - (bool)testLookupKeyAssets; - (bool)testLookupKeyAssetsWithPackage; - (bool)testViewControllerIsReleased; @@ -173,7 +174,27 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, #pragma mark - gtest tests -TEST(FlutterViewController, HasViewThatHidesOtherViewsInAccessibility) { +// AutoreleasePoolTest subclass that exists simply to provide more specific naming. +class FlutterViewControllerTest : public AutoreleasePoolTest { + public: + FlutterViewControllerTest() = default; + ~FlutterViewControllerTest() = default; + + private: + FML_DISALLOW_COPY_AND_ASSIGN(FlutterViewControllerTest); +}; + +// MockFlutterEngineTest subclass that exists simply to provide more specific naming. +class FlutterViewControllerMockEngineTest : public MockFlutterEngineTest { + public: + FlutterViewControllerMockEngineTest() = default; + ~FlutterViewControllerMockEngineTest() = default; + + private: + FML_DISALLOW_COPY_AND_ASSIGN(FlutterViewControllerMockEngineTest); +}; + +TEST_F(FlutterViewControllerTest, HasViewThatHidesOtherViewsInAccessibility) { FlutterViewController* viewControllerMock = CreateMockViewController(); [viewControllerMock loadView]; @@ -194,13 +215,13 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, EXPECT_EQ(accessibilityChildren[0], viewControllerMock.flutterView); } -TEST(FlutterViewController, FlutterViewAcceptsFirstMouse) { +TEST_F(FlutterViewControllerTest, FlutterViewAcceptsFirstMouse) { FlutterViewController* viewControllerMock = CreateMockViewController(); [viewControllerMock loadView]; EXPECT_EQ([viewControllerMock.flutterView acceptsFirstMouse:nil], YES); } -TEST(FlutterViewController, ReparentsPluginWhenAccessibilityDisabled) { +TEST_F(FlutterViewControllerTest, ReparentsPluginWhenAccessibilityDisabled) { FlutterEngine* engine = CreateTestEngine(); FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil @@ -226,7 +247,7 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, EXPECT_TRUE(viewController.textInputPlugin.superview == viewController.view); } -TEST(FlutterViewController, CanSetMouseTrackingModeBeforeViewLoaded) { +TEST_F(FlutterViewControllerTest, CanSetMouseTrackingModeBeforeViewLoaded) { NSString* fixtures = @(testing::GetFixturesPath()); FlutterDartProject* project = [[FlutterDartProject alloc] initWithAssetsPath:fixtures @@ -236,64 +257,84 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, ASSERT_EQ(viewController.mouseTrackingMode, kFlutterMouseTrackingModeInActiveApp); } -TEST(FlutterViewControllerTest, TestKeyEventsAreSentToFramework) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyEventsAreSentToFramework]); +TEST_F(FlutterViewControllerMockEngineTest, TestKeyEventsAreSentToFramework) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyEventsAreSentToFramework:mockEngine]); } -TEST(FlutterViewControllerTest, TestKeyEventsArePropagatedIfNotHandled) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyEventsArePropagatedIfNotHandled]); +TEST_F(FlutterViewControllerMockEngineTest, TestKeyEventsArePropagatedIfNotHandled) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testKeyEventsArePropagatedIfNotHandled:mockEngine]); } -TEST(FlutterViewControllerTest, TestKeyEventsAreNotPropagatedIfHandled) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyEventsAreNotPropagatedIfHandled]); +TEST_F(FlutterViewControllerMockEngineTest, TestKeyEventsAreNotPropagatedIfHandled) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testKeyEventsAreNotPropagatedIfHandled:mockEngine]); } -TEST(FlutterViewControllerTest, TestCtrlTabKeyEventIsPropagated) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testCtrlTabKeyEventIsPropagated]); +TEST_F(FlutterViewControllerMockEngineTest, TestCtrlTabKeyEventIsPropagated) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testCtrlTabKeyEventIsPropagated:mockEngine]); } -TEST(FlutterViewControllerTest, TestKeyEquivalentIsPassedToTextInputPlugin) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyEquivalentIsPassedToTextInputPlugin]); +TEST_F(FlutterViewControllerMockEngineTest, TestKeyEquivalentIsPassedToTextInputPlugin) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] + testKeyEquivalentIsPassedToTextInputPlugin:mockEngine]); } -TEST(FlutterViewControllerTest, TestFlagsChangedEventsArePropagatedIfNotHandled) { - ASSERT_TRUE( - [[FlutterViewControllerTestObjC alloc] testFlagsChangedEventsArePropagatedIfNotHandled]); +TEST_F(FlutterViewControllerMockEngineTest, TestFlagsChangedEventsArePropagatedIfNotHandled) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] + testFlagsChangedEventsArePropagatedIfNotHandled:mockEngine]); } -TEST(FlutterViewControllerTest, TestKeyboardIsRestartedOnEngineRestart) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyboardIsRestartedOnEngineRestart]); +TEST_F(FlutterViewControllerMockEngineTest, TestKeyboardIsRestartedOnEngineRestart) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testKeyboardIsRestartedOnEngineRestart:mockEngine]); } -TEST(FlutterViewControllerTest, TestTrackpadGesturesAreSentToFramework) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testTrackpadGesturesAreSentToFramework]); +TEST_F(FlutterViewControllerMockEngineTest, TestTrackpadGesturesAreSentToFramework) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testTrackpadGesturesAreSentToFramework:mockEngine]); } -TEST(FlutterViewControllerTest, TestMouseDownUpEventsSentToNextResponder) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testMouseDownUpEventsSentToNextResponder]); +TEST_F(FlutterViewControllerMockEngineTest, TestMouseDownUpEventsSentToNextResponder) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testMouseDownUpEventsSentToNextResponder:mockEngine]); } -TEST(FlutterViewControllerTest, TestModifierKeysAreSynthesizedOnMouseMove) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testModifierKeysAreSynthesizedOnMouseMove]); +TEST_F(FlutterViewControllerMockEngineTest, TestModifierKeysAreSynthesizedOnMouseMove) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testModifierKeysAreSynthesizedOnMouseMove:mockEngine]); } -TEST(FlutterViewControllerTest, testViewWillAppearCalledMultipleTimes) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testViewWillAppearCalledMultipleTimes]); +TEST_F(FlutterViewControllerMockEngineTest, testViewWillAppearCalledMultipleTimes) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE( + [[FlutterViewControllerTestObjC alloc] testViewWillAppearCalledMultipleTimes:mockEngine]); } -TEST(FlutterViewControllerTest, testFlutterViewIsConfigured) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testFlutterViewIsConfigured]); +TEST_F(FlutterViewControllerMockEngineTest, testFlutterViewIsConfigured) { + id mockEngine = GetMockEngine(); + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testFlutterViewIsConfigured:mockEngine]); } -TEST(FlutterViewControllerTest, testLookupKeyAssets) { +TEST_F(FlutterViewControllerTest, testLookupKeyAssets) { ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testLookupKeyAssets]); } -TEST(FlutterViewControllerTest, testLookupKeyAssetsWithPackage) { +TEST_F(FlutterViewControllerTest, testLookupKeyAssetsWithPackage) { ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testLookupKeyAssetsWithPackage]); } -TEST(FlutterViewControllerTest, testViewControllerIsReleased) { +TEST_F(FlutterViewControllerTest, testViewControllerIsReleased) { ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testViewControllerIsReleased]); } @@ -303,8 +344,7 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, @implementation FlutterViewControllerTestObjC -- (bool)testKeyEventsAreSentToFramework { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testKeyEventsAreSentToFramework:(id)engineMock { id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -342,8 +382,7 @@ - (bool)testKeyEventsAreSentToFramework { } // Regression test for https://github.com/flutter/flutter/issues/122084. -- (bool)testCtrlTabKeyEventIsPropagated { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testCtrlTabKeyEventIsPropagated:(id)engineMock { __block bool called = false; __block FlutterKeyEvent last_event; OCMStub([[engineMock ignoringNonObjectArgs] sendKeyEvent:FlutterKeyEvent {} @@ -387,8 +426,7 @@ - (bool)testCtrlTabKeyEventIsPropagated { return true; } -- (bool)testKeyEquivalentIsPassedToTextInputPlugin { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testKeyEquivalentIsPassedToTextInputPlugin:(id)engineMock { __block bool called = false; __block FlutterKeyEvent last_event; OCMStub([[engineMock ignoringNonObjectArgs] sendKeyEvent:FlutterKeyEvent {} @@ -438,8 +476,7 @@ - (bool)testKeyEquivalentIsPassedToTextInputPlugin { return true; } -- (bool)testKeyEventsArePropagatedIfNotHandled { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testKeyEventsArePropagatedIfNotHandled:(id)engineMock { id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -494,9 +531,7 @@ - (bool)testKeyEventsArePropagatedIfNotHandled { return true; } -- (bool)testFlutterViewIsConfigured { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); - +- (bool)testFlutterViewIsConfigured:(id)engineMock { FlutterRenderer* renderer_ = [[FlutterRenderer alloc] initWithFlutterEngine:engineMock]; OCMStub([engineMock renderer]).andReturn(renderer_); @@ -515,8 +550,7 @@ - (bool)testFlutterViewIsConfigured { return true; } -- (bool)testFlagsChangedEventsArePropagatedIfNotHandled { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testFlagsChangedEventsArePropagatedIfNotHandled:(id)engineMock { id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -569,8 +603,7 @@ - (bool)testFlagsChangedEventsArePropagatedIfNotHandled { return true; } -- (bool)testKeyEventsAreNotPropagatedIfHandled { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testKeyEventsAreNotPropagatedIfHandled:(id)engineMock { id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -625,8 +658,7 @@ - (bool)testKeyEventsAreNotPropagatedIfHandled { return true; } -- (bool)testKeyboardIsRestartedOnEngineRestart { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testKeyboardIsRestartedOnEngineRestart:(id)engineMock { id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -687,8 +719,7 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event } } -- (bool)testTrackpadGesturesAreSentToFramework { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testTrackpadGesturesAreSentToFramework:(id)engineMock { // Need to return a real renderer to allow view controller to load. FlutterRenderer* renderer_ = [[FlutterRenderer alloc] initWithFlutterEngine:engineMock]; OCMStub([engineMock renderer]).andReturn(renderer_); @@ -983,8 +1014,7 @@ - (bool)testTrackpadGesturesAreSentToFramework { return true; } -- (bool)testViewWillAppearCalledMultipleTimes { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testViewWillAppearCalledMultipleTimes:(id)engineMock { FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock nibName:@"" bundle:nil]; @@ -1020,7 +1050,7 @@ static void SwizzledNoop(id self, SEL _cmd) {} // See: https://github.com/flutter/flutter/issues/115015 // See: http://www.openradar.me/FB12050037 // See: https://developer.apple.com/documentation/appkit/nsresponder/1524634-mousedown -- (bool)testMouseDownUpEventsSentToNextResponder { +- (bool)testMouseDownUpEventsSentToNextResponder:(id)engineMock { // The root cause of the above bug is NSResponder mouseDown/mouseUp methods that don't correctly // walk the responder chain calling the appropriate method on the next responder under certain // conditions. Simulate this by swizzling out the default implementations and replacing them with @@ -1032,7 +1062,6 @@ - (bool)testMouseDownUpEventsSentToNextResponder { IMP origMouseUp = method_setImplementation(mouseUp, noopImp); // Verify that mouseDown/mouseUp trigger mouseDown/mouseUp calls on FlutterViewController. - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); MouseEventFlutterViewController* viewController = [[MouseEventFlutterViewController alloc] initWithEngine:engineMock nibName:@"" bundle:nil]; FlutterView* view = (FlutterView*)[viewController view]; @@ -1057,8 +1086,7 @@ - (bool)testMouseDownUpEventsSentToNextResponder { return true; } -- (bool)testModifierKeysAreSynthesizedOnMouseMove { - id engineMock = flutter::testing::CreateMockFlutterEngine(@""); +- (bool)testModifierKeysAreSynthesizedOnMouseMove:(id)engineMock { id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -1177,10 +1205,11 @@ - (bool)testViewControllerIsReleased { bundle:nil]; [viewController loadView]; weakController = viewController; + + [engineMock shutDownEngine]; } EXPECT_EQ(weakController, nil); - return true; } diff --git a/shell/platform/windows/angle_surface_manager.h b/shell/platform/windows/angle_surface_manager.h index 2f6302b9659ed..d50dc2a7f1e64 100644 --- a/shell/platform/windows/angle_surface_manager.h +++ b/shell/platform/windows/angle_surface_manager.h @@ -65,7 +65,7 @@ class AngleSurfaceManager { // Binds |egl_context_| to the current rendering thread and to the draw and // read surfaces returning a boolean result reflecting success. - bool MakeCurrent(); + virtual bool MakeCurrent(); // Unbinds the current EGL context from the current thread. bool ClearCurrent(); diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index 8059a1ed29e03..e66ccea5f45b3 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -12,6 +12,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::AnyNumber; using testing::Invoke; using testing::Return; @@ -133,8 +134,7 @@ TEST(FlutterWindowTest, OnBitmapSurfaceUpdated) { // when the DPI scale is 100% (96 DPI). TEST(FlutterWindowTest, OnCursorRectUpdatedRegularDPI) { MockFlutterWindow win32window; - ON_CALL(win32window, GetDpiScale()).WillByDefault(Return(1.0)); - EXPECT_CALL(win32window, GetDpiScale()).Times(1); + EXPECT_CALL(win32window, GetDpiScale()).WillOnce(Return(1.0)); Rect cursor_rect(Point(10, 20), Size(30, 40)); EXPECT_CALL(win32window, UpdateCursorRect(cursor_rect)).Times(1); @@ -147,8 +147,7 @@ TEST(FlutterWindowTest, OnCursorRectUpdatedRegularDPI) { // when the DPI scale is 150% (144 DPI). TEST(FlutterWindowTest, OnCursorRectUpdatedHighDPI) { MockFlutterWindow win32window; - ON_CALL(win32window, GetDpiScale()).WillByDefault(Return(1.5)); - EXPECT_CALL(win32window, GetDpiScale()).Times(1); + EXPECT_CALL(win32window, GetDpiScale()).WillOnce(Return(1.5)); Rect expected_cursor_rect(Point(15, 30), Size(45, 60)); EXPECT_CALL(win32window, UpdateCursorRect(expected_cursor_rect)).Times(1); @@ -160,7 +159,9 @@ TEST(FlutterWindowTest, OnCursorRectUpdatedHighDPI) { TEST(FlutterWindowTest, OnPointerStarSendsDeviceType) { FlutterWindow win32window(100, 100); MockWindowBindingHandlerDelegate delegate; + EXPECT_CALL(delegate, OnWindowStateEvent).Times(AnyNumber()); win32window.SetView(&delegate); + // Move EXPECT_CALL(delegate, OnPointerMove(10.0, 10.0, kFlutterPointerDeviceKindMouse, @@ -259,6 +260,7 @@ TEST(FlutterWindowTest, OnPointerStarSendsDeviceType) { TEST(FlutterWindowTest, OnScrollCallsGetScrollOffsetMultiplier) { MockFlutterWindow win32window; MockWindowBindingHandlerDelegate delegate; + EXPECT_CALL(win32window, OnWindowStateEvent).Times(AnyNumber()); win32window.SetView(&delegate); ON_CALL(win32window, GetScrollOffsetMultiplier()) @@ -277,6 +279,7 @@ TEST(FlutterWindowTest, OnScrollCallsGetScrollOffsetMultiplier) { TEST(FlutterWindowTest, OnWindowRepaint) { MockFlutterWindow win32window; MockWindowBindingHandlerDelegate delegate; + EXPECT_CALL(win32window, OnWindowStateEvent).Times(AnyNumber()); win32window.SetView(&delegate); EXPECT_CALL(delegate, OnWindowRepaint()).Times(1); @@ -287,6 +290,7 @@ TEST(FlutterWindowTest, OnWindowRepaint) { TEST(FlutterWindowTest, OnThemeChange) { MockFlutterWindow win32window; MockWindowBindingHandlerDelegate delegate; + EXPECT_CALL(win32window, OnWindowStateEvent).Times(AnyNumber()); win32window.SetView(&delegate); EXPECT_CALL(delegate, OnHighContrastChanged).Times(1); @@ -308,9 +312,9 @@ TEST(FlutterWindowTest, AccessibilityNodeWithoutView) { TEST(FlutterWindowTest, AlertNode) { std::unique_ptr win32window = std::make_unique(); - ON_CALL(*win32window, GetPlatformWindow()).WillByDefault(Return(nullptr)); - ON_CALL(*win32window, GetAxFragmentRootDelegate()) - .WillByDefault(Return(nullptr)); + EXPECT_CALL(*win32window.get(), GetAxFragmentRootDelegate()) + .WillRepeatedly(Return(nullptr)); + EXPECT_CALL(*win32window.get(), OnWindowStateEvent).Times(AnyNumber()); MockFlutterWindowsView view(std::move(win32window)); std::wstring message = L"Test alert"; EXPECT_CALL(view, NotifyWinEventWrapper(_, ax::mojom::Event::kAlert)) @@ -337,21 +341,22 @@ TEST(FlutterWindowTest, AlertNode) { TEST(FlutterWindowTest, LifecycleFocusMessages) { MockFlutterWindow win32window; - ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() { - return reinterpret_cast(1); - }); + EXPECT_CALL(win32window, GetPlatformWindow) + .WillRepeatedly(Return(reinterpret_cast(1))); MockWindowBindingHandlerDelegate delegate; - win32window.SetView(&delegate); WindowStateEvent last_event; - ON_CALL(delegate, OnWindowStateEvent) - .WillByDefault([&last_event](HWND hwnd, WindowStateEvent event) { + EXPECT_CALL(delegate, OnWindowStateEvent) + .WillRepeatedly([&last_event](HWND hwnd, WindowStateEvent event) { last_event = event; }); - ON_CALL(win32window, OnWindowStateEvent) - .WillByDefault([&](WindowStateEvent event) { + EXPECT_CALL(win32window, OnWindowStateEvent) + .WillRepeatedly([&](WindowStateEvent event) { win32window.FlutterWindow::OnWindowStateEvent(event); }); + EXPECT_CALL(win32window, OnResize).Times(AnyNumber()); + + win32window.SetView(&delegate); win32window.InjectWindowMessage(WM_SIZE, 0, 0); EXPECT_EQ(last_event, WindowStateEvent::kHide); @@ -368,13 +373,13 @@ TEST(FlutterWindowTest, LifecycleFocusMessages) { TEST(FlutterWindowTest, CachedLifecycleMessage) { MockFlutterWindow win32window; - ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() { - return reinterpret_cast(1); - }); - ON_CALL(win32window, OnWindowStateEvent) - .WillByDefault([&](WindowStateEvent event) { + EXPECT_CALL(win32window, GetPlatformWindow) + .WillRepeatedly(Return(reinterpret_cast(1))); + EXPECT_CALL(win32window, OnWindowStateEvent) + .WillRepeatedly([&](WindowStateEvent event) { win32window.FlutterWindow::OnWindowStateEvent(event); }); + EXPECT_CALL(win32window, OnResize).Times(1); // Restore win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1)); @@ -385,8 +390,8 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) { MockWindowBindingHandlerDelegate delegate; bool focused = false; bool restored = false; - ON_CALL(delegate, OnWindowStateEvent) - .WillByDefault([&](HWND hwnd, WindowStateEvent event) { + EXPECT_CALL(delegate, OnWindowStateEvent) + .WillRepeatedly([&](HWND hwnd, WindowStateEvent event) { if (event == WindowStateEvent::kFocus) { focused = true; } else if (event == WindowStateEvent::kShow) { @@ -403,18 +408,19 @@ TEST(FlutterWindowTest, PosthumousWindowMessage) { MockWindowBindingHandlerDelegate delegate; int msg_count = 0; HWND hwnd; - ON_CALL(delegate, OnWindowStateEvent) - .WillByDefault([&](HWND hwnd, WindowStateEvent event) { msg_count++; }); + EXPECT_CALL(delegate, OnWindowStateEvent) + .WillRepeatedly([&](HWND hwnd, WindowStateEvent event) { msg_count++; }); { MockFlutterWindow win32window(false); - ON_CALL(win32window, GetPlatformWindow).WillByDefault([&]() { + EXPECT_CALL(win32window, GetPlatformWindow).WillRepeatedly([&]() { return win32window.FlutterWindow::GetPlatformWindow(); }); - ON_CALL(win32window, OnWindowStateEvent) - .WillByDefault([&](WindowStateEvent event) { + EXPECT_CALL(win32window, OnWindowStateEvent) + .WillRepeatedly([&](WindowStateEvent event) { win32window.FlutterWindow::OnWindowStateEvent(event); }); + EXPECT_CALL(win32window, OnResize).Times(AnyNumber()); win32window.SetView(&delegate); win32window.InitializeChild("Title", 1, 1); hwnd = win32window.GetPlatformWindow(); diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 281663415d505..63c791b109932 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -640,9 +640,8 @@ TEST_F(FlutterWindowsEngineTest, AlertPlatformMessage) { std::make_unique<::testing::NiceMock>(); ui::AXPlatformNodeDelegateBase parent_delegate; AlertPlatformNodeDelegate delegate(parent_delegate); - ON_CALL(*window_binding_handler, GetAlertDelegate).WillByDefault([&delegate] { - return &delegate; - }); + EXPECT_CALL(*window_binding_handler, GetAlertDelegate) + .WillRepeatedly(Return(&delegate)); MockFlutterWindowsView view(std::move(window_binding_handler)); view.SetEngine(engine.get()); @@ -660,9 +659,9 @@ TEST_F(FlutterWindowsEngineTest, AlertPlatformMessage) { }); bool did_call = false; - ON_CALL(view, NotifyWinEventWrapper) - .WillByDefault([&did_call](ui::AXPlatformNodeWin* node, - ax::mojom::Event event) { did_call = true; }); + EXPECT_CALL(view, NotifyWinEventWrapper) + .WillOnce([&did_call](ui::AXPlatformNodeWin* node, + ax::mojom::Event event) { did_call = true; }); engine->UpdateSemanticsEnabled(true); engine->Run(); @@ -712,13 +711,13 @@ TEST_F(FlutterWindowsEngineTest, TestExit) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, Quit) - .WillByDefault( - [&finished](std::optional hwnd, std::optional wparam, - std::optional lparam, - UINT exit_code) { finished = exit_code == 0; }); - ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; }); - EXPECT_CALL(*handler, Quit).Times(1); + EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed)); + EXPECT_CALL(*handler, Quit) + .WillOnce([&finished](std::optional hwnd, + std::optional wparam, + std::optional lparam, + UINT exit_code) { finished = exit_code == 0; }); + EXPECT_CALL(*handler, IsLastWindowOfProcess).WillRepeatedly(Return(true)); modifier.SetLifecycleManager(std::move(handler)); engine->lifecycle_manager()->BeginProcessingExit(); @@ -738,7 +737,6 @@ TEST_F(FlutterWindowsEngineTest, TestExit) { TEST_F(FlutterWindowsEngineTest, TestExitCancel) { FlutterWindowsEngineBuilder builder{GetContext()}; builder.SetDartEntrypoint("exitTestCancel"); - bool finished = false; bool did_call = false; auto engine = builder.Build(); @@ -750,12 +748,8 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, Quit) - .WillByDefault([&finished](std::optional hwnd, - std::optional wparam, - std::optional lparam, - UINT exit_code) { finished = true; }); - ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; }); + EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed)); + EXPECT_CALL(*handler, IsLastWindowOfProcess).WillRepeatedly(Return(true)); EXPECT_CALL(*handler, Quit).Times(0); modifier.SetLifecycleManager(std::move(handler)); engine->lifecycle_manager()->BeginProcessingExit(); @@ -783,10 +777,11 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) { while (!did_call) { engine->task_runner()->ProcessTasks(); } - - EXPECT_FALSE(finished); } +// TODO(loicsharma): This test is passing incorrectly on the first +// WM_CLOSE message when instead it should pass on the second WM_CLOSE message. +// https://github.com/flutter/flutter/issues/137963 TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) { FlutterWindowsEngineBuilder builder{GetContext()}; builder.SetDartEntrypoint("exitTestExit"); @@ -801,18 +796,18 @@ TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - auto& handler_obj = *handler; - ON_CALL(handler_obj, IsLastWindowOfProcess).WillByDefault([]() { - return true; - }); - ON_CALL(handler_obj, Quit) - .WillByDefault( - [&handler_obj](std::optional hwnd, std::optional wparam, + EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed)); + // TODO(loicsharma): These should be `EXPECT_CALL`s + // https://github.com/flutter/flutter/issues/137963 + ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault(Return(true)); + ON_CALL(*handler, Quit) + .WillByDefault([handler_ptr = handler.get()]( + std::optional hwnd, std::optional wparam, std::optional lparam, UINT exit_code) { - handler_obj.WindowsLifecycleManager::Quit(hwnd, wparam, lparam, - exit_code); - }); - ON_CALL(handler_obj, DispatchMessage) + handler_ptr->WindowsLifecycleManager::Quit(hwnd, wparam, lparam, + exit_code); + }); + ON_CALL(*handler, DispatchMessage) .WillByDefault( [&engine](HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) { engine->window_proc_delegate_manager()->OnTopLevelWindowProc( @@ -863,7 +858,8 @@ TEST_F(FlutterWindowsEngineTest, TestExitCloseMultiWindow) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([&finished]() { + EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed)); + EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce([&finished]() { finished = true; return false; }); @@ -913,10 +909,7 @@ TEST_F(FlutterWindowsEngineTest, EnableApplicationLifecycle) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { - return false; - }); - EXPECT_CALL(*handler, IsLastWindowOfProcess).Times(1); + EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(false)); modifier.SetLifecycleManager(std::move(handler)); engine->lifecycle_manager()->BeginProcessingExit(); @@ -936,10 +929,7 @@ TEST_F(FlutterWindowsEngineTest, ApplicationLifecycleExternalWindow) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { - return false; - }); - EXPECT_CALL(*handler, IsLastWindowOfProcess).Times(1); + EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(false)); modifier.SetLifecycleManager(std::move(handler)); engine->lifecycle_manager()->BeginProcessingExit(); @@ -1065,8 +1055,8 @@ TEST_F(FlutterWindowsEngineTest, EnableLifecycleState) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, SetLifecycleState) - .WillByDefault([handler_ptr = handler.get()](AppLifecycleState state) { + EXPECT_CALL(*handler, SetLifecycleState) + .WillRepeatedly([handler_ptr = handler.get()](AppLifecycleState state) { handler_ptr->WindowsLifecycleManager::SetLifecycleState(state); }); modifier.SetLifecycleManager(std::move(handler)); @@ -1118,8 +1108,8 @@ TEST_F(FlutterWindowsEngineTest, LifecycleStateToFrom) { EngineModifier modifier(engine.get()); modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; auto handler = std::make_unique(engine.get()); - ON_CALL(*handler, SetLifecycleState) - .WillByDefault([handler_ptr = handler.get()](AppLifecycleState state) { + EXPECT_CALL(*handler, SetLifecycleState) + .WillRepeatedly([handler_ptr = handler.get()](AppLifecycleState state) { handler_ptr->WindowsLifecycleManager::SetLifecycleState(state); }); handler->begin_processing_callback = [&]() { enabled_lifecycle = true; }; @@ -1167,6 +1157,7 @@ TEST_F(FlutterWindowsEngineTest, ChannelListenedTo) { bool lifecycle_began = false; auto handler = std::make_unique(engine.get()); + EXPECT_CALL(*handler, SetLifecycleState).Times(1); handler->begin_processing_callback = [&]() { lifecycle_began = true; }; modifier.SetLifecycleManager(std::move(handler)); diff --git a/shell/platform/windows/flutter_windows_unittests.cc b/shell/platform/windows/flutter_windows_unittests.cc index 38d613301bc80..bab9d89b01523 100644 --- a/shell/platform/windows/flutter_windows_unittests.cc +++ b/shell/platform/windows/flutter_windows_unittests.cc @@ -15,6 +15,7 @@ #include "flutter/shell/platform/windows/testing/windows_test.h" #include "flutter/shell/platform/windows/testing/windows_test_config_builder.h" #include "flutter/shell/platform/windows/testing/windows_test_context.h" +#include "flutter/testing/stream_capture.h" #include "gtest/gtest.h" #include "third_party/tonic/converter/dart_converter.h" @@ -45,27 +46,20 @@ TEST_F(WindowsTest, LaunchMain) { // Verify there is no unexpected output from launching main. TEST_F(WindowsTest, LaunchMainHasNoOutput) { // Replace stdout & stderr stream buffers with our own. - std::stringstream cout_buffer; - std::stringstream cerr_buffer; - std::streambuf* old_cout_buffer = std::cout.rdbuf(); - std::streambuf* old_cerr_buffer = std::cerr.rdbuf(); - std::cout.rdbuf(cout_buffer.rdbuf()); - std::cerr.rdbuf(cerr_buffer.rdbuf()); + StreamCapture stdout_capture(&std::cout); + StreamCapture stderr_capture(&std::cerr); auto& context = GetContext(); WindowsConfigBuilder builder(context); ViewControllerPtr controller{builder.Run()}; ASSERT_NE(controller, nullptr); - // Restore original stdout & stderr stream buffer. - std::cout.rdbuf(old_cout_buffer); - std::cerr.rdbuf(old_cerr_buffer); + stdout_capture.Stop(); + stderr_capture.Stop(); // Verify stdout & stderr have no output. - std::string cout = cout_buffer.str(); - std::string cerr = cerr_buffer.str(); - EXPECT_TRUE(cout.empty()); - EXPECT_TRUE(cerr.empty()); + EXPECT_TRUE(stdout_capture.GetOutput().empty()); + EXPECT_TRUE(stderr_capture.GetOutput().empty()); } // Verify we can successfully launch a custom entry point. diff --git a/shell/platform/windows/flutter_windows_view_unittests.cc b/shell/platform/windows/flutter_windows_view_unittests.cc index 859ffd1bd484a..6f4d5fc85650f 100644 --- a/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/shell/platform/windows/flutter_windows_view_unittests.cc @@ -131,6 +131,7 @@ class MockAngleSurfaceManager : public AngleSurfaceManager { (override)); MOCK_METHOD(void, DestroySurface, (), (override)); + MOCK_METHOD(bool, MakeCurrent, (), (override)); MOCK_METHOD(void, SetVSyncEnabled, (bool), (override)); private: @@ -1302,7 +1303,9 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) { FlutterWindowsView view(std::move(window_binding_handler)); InSequence s; + EXPECT_CALL(*surface_manager.get(), MakeCurrent).WillOnce(Return(true)); EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(true)).Times(1); + EXPECT_CALL(*surface_manager.get(), MakeCurrent).WillOnce(Return(true)); EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(false)).Times(1); EXPECT_CALL(*engine.get(), Stop).Times(1); diff --git a/shell/platform/windows/system_utils_unittests.cc b/shell/platform/windows/system_utils_unittests.cc index a4a9bcda114b8..1b53ed7748dc8 100644 --- a/shell/platform/windows/system_utils_unittests.cc +++ b/shell/platform/windows/system_utils_unittests.cc @@ -25,8 +25,8 @@ TEST(SystemUtils, GetPreferredLanguageInfo) { TEST(SystemUtils, GetPreferredLanguages) { MockWindowsProcTable proc_table; - ON_CALL(proc_table, GetThreadPreferredUILanguages) - .WillByDefault( + EXPECT_CALL(proc_table, GetThreadPreferredUILanguages) + .WillRepeatedly( [](DWORD flags, PULONG count, PZZWSTR languages, PULONG size) { // Languages string ends in a double-null. static const wchar_t lang[] = L"en-US\0"; diff --git a/testing/BUILD.gn b/testing/BUILD.gn index 5a56313330d6a..cdc185aac29ac 100644 --- a/testing/BUILD.gn +++ b/testing/BUILD.gn @@ -25,6 +25,8 @@ source_set("testing_lib") { "mock_canvas.h", "post_task_sync.cc", "post_task_sync.h", + "stream_capture.cc", + "stream_capture.h", "test_args.cc", "test_args.h", "test_timeout_listener.cc", diff --git a/testing/stream_capture.cc b/testing/stream_capture.cc new file mode 100644 index 0000000000000..ed02282f4dc19 --- /dev/null +++ b/testing/stream_capture.cc @@ -0,0 +1,31 @@ +// 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/testing/stream_capture.h" + +namespace flutter { +namespace testing { + +StreamCapture::StreamCapture(std::ostream* ostream) + : ostream_(ostream), old_buffer_(ostream_->rdbuf()) { + ostream_->rdbuf(buffer_.rdbuf()); +} + +StreamCapture::~StreamCapture() { + Stop(); +} + +void StreamCapture::Stop() { + if (old_buffer_) { + ostream_->rdbuf(old_buffer_); + old_buffer_ = nullptr; + } +} + +std::string StreamCapture::GetOutput() const { + return buffer_.str(); +} + +} // namespace testing +} // namespace flutter diff --git a/testing/stream_capture.h b/testing/stream_capture.h new file mode 100644 index 0000000000000..d40a38045d255 --- /dev/null +++ b/testing/stream_capture.h @@ -0,0 +1,47 @@ +// 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_TESTING_STREAM_CAPTURE_H_ +#define FLUTTER_TESTING_STREAM_CAPTURE_H_ + +#include +#include +#include + +namespace flutter { +namespace testing { + +// Temporarily replaces the specified stream's output buffer to capture output. +// +// Example: +// StreamCapture captured_stdout(&std::cout); +// ... code that writest to std::cout ... +// std::string output = captured_stdout.GetCapturedOutput(); +class StreamCapture { + public: + // Begins capturing output to the specified stream. + StreamCapture(std::ostream* ostream); + + // Stops capturing output to the specified stream, and restores the original + // output buffer, if |Stop| has not already been called. + ~StreamCapture(); + + // Stops capturing output to the specified stream, and restores the original + // output buffer. + void Stop(); + + // Returns any output written to the captured stream between construction and + // the first call to |Stop|, if any, or now. + std::string GetOutput() const; + + private: + std::ostream* ostream_; + std::stringstream buffer_; + std::streambuf* old_buffer_; +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_TESTING_STREAM_CAPTURE_H_ diff --git a/tools/licenses/lib/paths.dart b/tools/licenses/lib/paths.dart index 12a701a22be6a..b41d20e8048d1 100644 --- a/tools/licenses/lib/paths.dart +++ b/tools/licenses/lib/paths.dart @@ -96,11 +96,15 @@ final Set skippedPaths = { r'third_party/dart/pkg', // packages that don't become part of the binary (e.g. the analyzer) r'third_party/dart/runtime/bin/ffi_test', r'third_party/dart/runtime/docs', - r'third_party/dart/runtime/third_party/binary_size', // not linked in either - r'third_party/dart/runtime/third_party/d3', // Siva says "that is the charting library used by the binary size tool" + // TODO(aam): remove as a dup + r'third_party/dart/runtime/third_party/binary_size', + // TODO(aam): remove as a dup + r'third_party/dart/runtime/third_party/d3', r'third_party/dart/runtime/vm/service', r'third_party/dart/sdk/lib/html/doc', + r'third_party/dart/third_party/binary_size', // not linked in r'third_party/dart/third_party/binaryen', // not linked in + r'third_party/dart/third_party/d3', // Siva says "that is the charting library used by the binary size tool" r'third_party/dart/third_party/d8', // testing tool for dart2js r'third_party/dart/third_party/devtools', // not linked in r'third_party/dart/third_party/firefox_jsshell', // testing tool for dart2js