[flutter_releases] Flutter stable 2.10.3 Engine Cherrypicks

flow/layers/physical_shape_layer.cc

@@ -56,18 +56,24 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context,
   Layer::AutoPrerollSaveLayerState save =
       Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
 
-  SkRect child_paint_bounds;
+  SkRect child_paint_bounds = SkRect::MakeEmpty();
   PrerollChildren(context, matrix, &child_paint_bounds);
 
+  SkRect paint_bounds;
   if (elevation_ == 0) {
-    set_paint_bounds(path_.getBounds());
+    paint_bounds = path_.getBounds();
   } else {
     // We will draw the shadow in Paint(), so add some margin to the paint
-    // bounds to leave space for the shadow. We fill this whole region and clip
-    // children to it so we don't need to join the child paint bounds.
-    set_paint_bounds(DisplayListCanvasDispatcher::ComputeShadowBounds(
-        path_, elevation_, context->frame_device_pixel_ratio, matrix));
+    // bounds to leave space for the shadow.
+    paint_bounds = DisplayListCanvasDispatcher::ComputeShadowBounds(
+        path_, elevation_, context->frame_device_pixel_ratio, matrix);
   }
+
+  if (clip_behavior_ == Clip::none) {
+    paint_bounds.join(child_paint_bounds);
+  }
+
+  set_paint_bounds(paint_bounds);
 }
 
 void PhysicalShapeLayer::Paint(PaintContext& context) const {

flow/layers/physical_shape_layer_unittests.cc

@@ -66,7 +66,7 @@ TEST_F(PhysicalShapeLayerTest, NonEmptyLayer) {
                 0, MockCanvas::DrawPathData{layer_path, layer_paint}}}));
 }
 
-TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPath) {
+TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPathClip) {
   SkPath layer_path;
   layer_path.addRect(5.0f, 6.0f, 20.5f, 21.5f);
   SkPath child1_path;
@@ -83,18 +83,74 @@ TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPath) {
   auto layer =
       std::make_shared<PhysicalShapeLayer>(SK_ColorGREEN, SK_ColorBLACK,
                                            0.0f,  // elevation
-                                           layer_path, Clip::none);
+                                           layer_path, Clip::hardEdge);
   layer->Add(child1);
   layer->Add(child2);
 
-  SkRect child_paint_bounds;
+  SkRect child_paint_bounds = SkRect::MakeEmpty();
   layer->Preroll(preroll_context(), SkMatrix());
   child_paint_bounds.join(child1->paint_bounds());
   child_paint_bounds.join(child2->paint_bounds());
   EXPECT_EQ(layer->paint_bounds(), layer_path.getBounds());
   EXPECT_NE(layer->paint_bounds(), child_paint_bounds);
   EXPECT_TRUE(layer->needs_painting(paint_context()));
 
+  SkPaint layer_paint;
+  layer_paint.setColor(SK_ColorGREEN);
+  layer_paint.setAntiAlias(true);
+  SkPaint child1_paint;
+  child1_paint.setColor(SK_ColorRED);
+  child1_paint.setAntiAlias(true);
+  SkPaint child2_paint;
+  child2_paint.setColor(SK_ColorBLUE);
+  child2_paint.setAntiAlias(true);
+  layer->Paint(paint_context());
+  EXPECT_EQ(mock_canvas().draw_calls(),
+            std::vector({
+                MockCanvas::DrawCall{
+                    0, MockCanvas::DrawPathData{layer_path, layer_paint}},
+                MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
+                MockCanvas::DrawCall{
+                    1, MockCanvas::ClipRectData{layer_path.getBounds(),
+                                                SkClipOp::kIntersect}},
+                MockCanvas::DrawCall{
+                    1, MockCanvas::DrawPathData{child1_path, child1_paint}},
+                MockCanvas::DrawCall{
+                    1, MockCanvas::DrawPathData{child2_path, child2_paint}},
+                MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
+            }));
+}
+
+TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPathNoClip) {
+  SkPath layer_path;
+  layer_path.addRect(5.0f, 6.0f, 20.5f, 21.5f);
+  SkPath child1_path;
+  child1_path.addRect(4, 0, 12, 12).close();
+  SkPath child2_path;
+  child2_path.addRect(3, 2, 5, 15).close();
+  auto child1 = std::make_shared<PhysicalShapeLayer>(SK_ColorRED, SK_ColorBLACK,
+                                                     0.0f,  // elevation
+                                                     child1_path, Clip::none);
+  auto child2 =
+      std::make_shared<PhysicalShapeLayer>(SK_ColorBLUE, SK_ColorBLACK,
+                                           0.0f,  // elevation
+                                           child2_path, Clip::none);
+  auto layer =
+      std::make_shared<PhysicalShapeLayer>(SK_ColorGREEN, SK_ColorBLACK,
+                                           0.0f,  // elevation
+                                           layer_path, Clip::none);
+  layer->Add(child1);
+  layer->Add(child2);
+
+  SkRect total_bounds = SkRect::MakeEmpty();
+  layer->Preroll(preroll_context(), SkMatrix());
+  total_bounds.join(child1->paint_bounds());
+  total_bounds.join(child2->paint_bounds());
+  total_bounds.join(layer_path.getBounds());
+  EXPECT_NE(layer->paint_bounds(), layer_path.getBounds());
+  EXPECT_EQ(layer->paint_bounds(), total_bounds);
+  EXPECT_TRUE(layer->needs_painting(paint_context()));
+
   SkPaint layer_paint;
   layer_paint.setColor(SK_ColorGREEN);
   layer_paint.setAntiAlias(true);
@@ -174,10 +230,14 @@ TEST_F(PhysicalShapeLayerTest, ElevationComplex) {
     // On Fuchsia, the system compositor handles all elevated
     // PhysicalShapeLayers and their shadows , so we do not do any painting
     // there.
-    EXPECT_EQ(layers[i]->paint_bounds(),
-              (DisplayListCanvasDispatcher::ComputeShadowBounds(
-                  layer_path, initial_elevations[i], 1.0f /* pixel_ratio */,
-                  SkMatrix())));
+    SkRect paint_bounds = DisplayListCanvasDispatcher::ComputeShadowBounds(
+        layer_path, initial_elevations[i], 1.0f /* pixel_ratio */, SkMatrix());
+
+    // Without clipping the children will be painted as well
+    for (auto layer : layers[i]->layers()) {
+      paint_bounds.join(layer->paint_bounds());
+    }
+    EXPECT_EQ(layers[i]->paint_bounds(), paint_bounds);
     EXPECT_TRUE(layers[i]->needs_painting(paint_context()));
   }
 

shell/common/shell.cc

@@ -878,10 +878,13 @@ void Shell::OnPlatformViewDestroyed() {
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
                                     raster_task);
   latch.Wait();
-  // On Android, the external view embedder posts a task to the platform thread,
-  // and waits until it completes.
-  // As a result, the platform thread must not be blocked prior to calling
-  // this method.
+  // On Android, the external view embedder may post a task to the platform
+  // thread, and wait until it completes if overlay surfaces must be released.
+  // However, the platform thread might be blocked when Dart is initializing.
+  // In this situation, calling TeardownExternalViewEmbedder is safe because no
+  // platform views have been created before Flutter renders the first frame.
+  // Overall, the longer term plan is to remove this implementation once
+  // https://github.com/flutter/flutter/issues/96679 is fixed.
   rasterizer_->TeardownExternalViewEmbedder();
 }
 

shell/platform/android/external_view_embedder/external_view_embedder.cc

@@ -309,6 +309,9 @@ void AndroidExternalViewEmbedder::Teardown() {
 
 // |ExternalViewEmbedder|
 void AndroidExternalViewEmbedder::DestroySurfaces() {
+  if (!surface_pool_->HasLayers()) {
+    return;
+  }
   fml::AutoResetWaitableEvent latch;
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(),
                                     [&]() {

shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc

@@ -970,5 +970,16 @@ TEST(AndroidExternalViewEmbedder, Teardown) {
   embedder->Teardown();
 }
 
+TEST(AndroidExternalViewEmbedder, TeardownDoesNotCallJNIMethod) {
+  auto jni_mock = std::make_shared<JNIMock>();
+  auto android_context =
+      std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware);
+  auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
+      *android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
+
+  EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0);
+  embedder->Teardown();
+}
+
 }  // namespace testing
 }  // namespace flutter

shell/platform/android/external_view_embedder/surface_pool.cc

@@ -24,11 +24,11 @@ std::shared_ptr<OverlayLayer> SurfacePool::GetLayer(
     const AndroidContext& android_context,
     std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
     std::shared_ptr<AndroidSurfaceFactory> surface_factory) {
+  std::lock_guard lock(mutex_);
   // Destroy current layers in the pool if the frame size has changed.
   if (requested_frame_size_ != current_frame_size_) {
-    DestroyLayers(jni_facade);
+    DestroyLayersLocked(jni_facade);
   }
-
   intptr_t gr_context_key = reinterpret_cast<intptr_t>(gr_context);
   // Allocate a new surface if there isn't one available.
   if (available_layer_index_ >= layers_.size()) {
@@ -56,6 +56,7 @@ std::shared_ptr<OverlayLayer> SurfacePool::GetLayer(
     layer->gr_context_key = gr_context_key;
     layers_.push_back(layer);
   }
+
   std::shared_ptr<OverlayLayer> layer = layers_[available_layer_index_];
   // Since the surfaces are recycled, it's possible that the GrContext is
   // different.
@@ -73,19 +74,33 @@ std::shared_ptr<OverlayLayer> SurfacePool::GetLayer(
 }
 
 void SurfacePool::RecycleLayers() {
+  std::lock_guard lock(mutex_);
   available_layer_index_ = 0;
 }
 
+bool SurfacePool::HasLayers() {
+  std::lock_guard lock(mutex_);
+  return layers_.size() > 0;
+}
+
 void SurfacePool::DestroyLayers(
     std::shared_ptr<PlatformViewAndroidJNI> jni_facade) {
-  if (layers_.size() > 0) {
-    jni_facade->FlutterViewDestroyOverlaySurfaces();
+  std::lock_guard lock(mutex_);
+  DestroyLayersLocked(jni_facade);
+}
+
+void SurfacePool::DestroyLayersLocked(
+    std::shared_ptr<PlatformViewAndroidJNI> jni_facade) {
+  if (layers_.size() == 0) {
+    return;
   }
+  jni_facade->FlutterViewDestroyOverlaySurfaces();
   layers_.clear();
   available_layer_index_ = 0;
 }
 
 std::vector<std::shared_ptr<OverlayLayer>> SurfacePool::GetUnusedLayers() {
+  std::lock_guard lock(mutex_);
   std::vector<std::shared_ptr<OverlayLayer>> results;
   for (size_t i = available_layer_index_; i < layers_.size(); i++) {
     results.push_back(layers_[i]);
@@ -94,6 +109,7 @@ std::vector<std::shared_ptr<OverlayLayer>> SurfacePool::GetUnusedLayers() {
 }
 
 void SurfacePool::SetFrameSize(SkISize frame_size) {
+  std::lock_guard lock(mutex_);
   requested_frame_size_ = frame_size;
 }
 

shell/platform/android/external_view_embedder/surface_pool.h

@@ -5,6 +5,8 @@
 #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_
 #define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_
 
+#include <mutex>
+
 #include "flutter/flow/surface.h"
 #include "flutter/shell/platform/android/context/android_context.h"
 #include "flutter/shell/platform/android/surface/android_surface.h"
@@ -41,7 +43,6 @@ struct OverlayLayer {
   intptr_t gr_context_key;
 };
 
-// This class isn't thread safe.
 class SurfacePool {
  public:
   SurfacePool();
@@ -72,6 +73,9 @@ class SurfacePool {
   // then they are deallocated as soon as |GetLayer| is called.
   void SetFrameSize(SkISize frame_size);
 
+  // Returns true if the current pool has layers in use.
+  bool HasLayers();
+
  private:
   // The index of the entry in the layers_ vector that determines the beginning
   // of the unused layers. For example, consider the following vector:
@@ -95,6 +99,11 @@ class SurfacePool {
 
   // The frame size to be used by future layers.
   SkISize requested_frame_size_;
+
+  // Used to guard public methods.
+  std::mutex mutex_;
+
+  void DestroyLayersLocked(std::shared_ptr<PlatformViewAndroidJNI> jni_facade);
 };
 
 }  // namespace flutter

shell/platform/android/external_view_embedder/surface_pool_unittests.cc

@@ -62,6 +62,7 @@ TEST(SurfacePool, GetLayerAllocateOneLayer) {
   auto layer = pool->GetLayer(gr_context.get(), *android_context, jni_mock,
                               surface_factory);
 
+  ASSERT_TRUE(pool->HasLayers());
   ASSERT_NE(nullptr, layer);
   ASSERT_EQ(reinterpret_cast<intptr_t>(gr_context.get()),
             layer->gr_context_key);
@@ -96,6 +97,7 @@ TEST(SurfacePool, GetUnusedLayers) {
 
   pool->RecycleLayers();
 
+  ASSERT_TRUE(pool->HasLayers());
   ASSERT_EQ(1UL, pool->GetUnusedLayers().size());
   ASSERT_EQ(layer, pool->GetUnusedLayers()[0]);
 }
@@ -137,6 +139,7 @@ TEST(SurfacePool, GetLayerRecycle) {
   auto layer_2 = pool->GetLayer(gr_context_2.get(), *android_context, jni_mock,
                                 surface_factory);
 
+  ASSERT_TRUE(pool->HasLayers());
   ASSERT_NE(nullptr, layer_1);
   ASSERT_EQ(layer_1, layer_2);
   ASSERT_EQ(reinterpret_cast<intptr_t>(gr_context_2.get()),
@@ -176,6 +179,8 @@ TEST(SurfacePool, GetLayerAllocateTwoLayers) {
                                 surface_factory);
   auto layer_2 = pool->GetLayer(gr_context.get(), *android_context, jni_mock,
                                 surface_factory);
+
+  ASSERT_TRUE(pool->HasLayers());
   ASSERT_NE(nullptr, layer_1);
   ASSERT_NE(nullptr, layer_2);
   ASSERT_NE(layer_1, layer_2);
@@ -213,9 +218,11 @@ TEST(SurfacePool, DestroyLayers) {
   pool->GetLayer(gr_context.get(), *android_context, jni_mock, surface_factory);
 
   EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces());
+
+  ASSERT_TRUE(pool->HasLayers());
   pool->DestroyLayers(jni_mock);
 
-  pool->RecycleLayers();
+  ASSERT_FALSE(pool->HasLayers());
   ASSERT_TRUE(pool->GetUnusedLayers().empty());
 }
 
@@ -246,8 +253,12 @@ TEST(SurfacePool, DestroyLayersFrameSizeChanged) {
           ByMove(std::make_unique<PlatformViewAndroidJNI::OverlayMetadata>(
               0, window))));
 
+  ASSERT_FALSE(pool->HasLayers());
+
   pool->GetLayer(gr_context.get(), *android_context, jni_mock, surface_factory);
 
+  ASSERT_TRUE(pool->HasLayers());
+
   pool->SetFrameSize(SkISize::Make(20, 20));
   EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1);
   EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface())
@@ -258,6 +269,7 @@ TEST(SurfacePool, DestroyLayersFrameSizeChanged) {
   pool->GetLayer(gr_context.get(), *android_context, jni_mock, surface_factory);
 
   ASSERT_TRUE(pool->GetUnusedLayers().empty());
+  ASSERT_TRUE(pool->HasLayers());
 }
 
 }  // namespace testing