WeakPtrFactory should be destroyed before any other members. (#29402)

Author: 0xZOne

lib/ui/painting/image_decoder.h

@@ -57,7 +57,6 @@ class ImageDecoder {
  std::shared_ptr<fml::ConcurrentTaskRunner> concurrent_task_runner_;
  fml::WeakPtr<IOManager> io_manager_;
  fml::WeakPtrFactory<ImageDecoder> weak_factory_;
-
  FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoder);
 };
 

shell/common/engine.h

@@ -910,29 +910,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
  }
 
  private:
- Engine::Delegate& delegate_;
- const Settings settings_;
- std::unique_ptr<Animator> animator_;
- std::unique_ptr<RuntimeController> runtime_controller_;
-
- // The pointer_data_dispatcher_ depends on animator_ and runtime_controller_.
- // So it should be defined after them to ensure that pointer_data_dispatcher_
- // is destructed first.
- std::unique_ptr<PointerDataDispatcher> pointer_data_dispatcher_;
-
- std::string last_entry_point_;
- std::string last_entry_point_library_;
- std::string initial_route_;
- ViewportMetrics viewport_metrics_;
- std::shared_ptr<AssetManager> asset_manager_;
- bool activity_running_;
- bool have_surface_;
- std::shared_ptr<FontCollection> font_collection_;
- ImageDecoder image_decoder_;
- ImageGeneratorRegistry image_generator_registry_;
- TaskRunners task_runners_;
- fml::WeakPtrFactory<Engine> weak_factory_;
-
  // |RuntimeDelegate|
  std::string DefaultRouteName() override;
 
@@ -981,6 +958,28 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
 
  friend class testing::ShellTest;
 
+ Engine::Delegate& delegate_;
+ const Settings settings_;
+ std::unique_ptr<Animator> animator_;
+ std::unique_ptr<RuntimeController> runtime_controller_;
+
+ // The pointer_data_dispatcher_ depends on animator_ and runtime_controller_.
+ // So it should be defined after them to ensure that pointer_data_dispatcher_
+ // is destructed first.
+ std::unique_ptr<PointerDataDispatcher> pointer_data_dispatcher_;
+
+ std::string last_entry_point_;
+ std::string last_entry_point_library_;
+ std::string initial_route_;
+ ViewportMetrics viewport_metrics_;
+ std::shared_ptr<AssetManager> asset_manager_;
+ bool activity_running_;
+ bool have_surface_;
+ std::shared_ptr<FontCollection> font_collection_;
+ ImageDecoder image_decoder_;
+ ImageGeneratorRegistry image_generator_registry_;
+ TaskRunners task_runners_;
+ fml::WeakPtrFactory<Engine> weak_factory_; // Must be the last member.
  FML_DISALLOW_COPY_AND_ASSIGN(Engine);
 };
 

shell/common/platform_view.h

@@ -823,15 +823,14 @@ class PlatformView {
  const;
 
  protected:
+ // This is the only method called on the raster task runner.
+ virtual std::unique_ptr<Surface> CreateRenderingSurface();
+
  PlatformView::Delegate& delegate_;
  const TaskRunners task_runners_;
-
  PointerDataPacketConverter pointer_data_packet_converter_;
  SkISize size_;
- fml::WeakPtrFactory<PlatformView> weak_factory_;
-
- // This is the only method called on the raster task runner.
- virtual std::unique_ptr<Surface> CreateRenderingSurface();
+ fml::WeakPtrFactory<PlatformView> weak_factory_; // Must be the last member.
 
  private:
  FML_DISALLOW_COPY_AND_ASSIGN(PlatformView);

shell/common/pointer_data_dispatcher.h

@@ -147,20 +147,18 @@ class SmoothPointerDataDispatcher : public DefaultPointerDataDispatcher {
  virtual ~SmoothPointerDataDispatcher();
 
  private:
+ void DispatchPendingPacket();
+ void ScheduleSecondaryVsyncCallback();
+
  // If non-null, this will be a pending pointer data packet for the next frame
  // to consume. This is used to smooth out the irregular drag events delivery.
  // See also `DispatchPointerDataPacket` and input_events_unittests.cc.
  std::unique_ptr<PointerDataPacket> pending_packet_;
  int pending_trace_flow_id_ = -1;
-
  bool is_pointer_data_in_progress_ = false;
 
+ // WeakPtrFactory must be the last member.
  fml::WeakPtrFactory<SmoothPointerDataDispatcher> weak_factory_;
-
- void DispatchPendingPacket();
-
- void ScheduleSecondaryVsyncCallback();
-
  FML_DISALLOW_COPY_AND_ASSIGN(SmoothPointerDataDispatcher);
 };
 

shell/common/rasterizer.h

@@ -448,22 +448,6 @@ class Rasterizer final : public SnapshotDelegate {
  void DisableThreadMergerIfNeeded();
 
  private:
- Delegate& delegate_;
- std::unique_ptr<Surface> surface_;
- std::unique_ptr<SnapshotSurfaceProducer> snapshot_surface_producer_;
- std::unique_ptr<flutter::CompositorContext> compositor_context_;
- // This is the last successfully rasterized layer tree.
- std::unique_ptr<flutter::LayerTree> last_layer_tree_;
- // Set when we need attempt to rasterize the layer tree again. This layer_tree
- // has not successfully rasterized. This can happen due to the change in the
- // thread configuration. This will be inserted to the front of the pipeline.
- std::unique_ptr<flutter::LayerTree> resubmitted_layer_tree_;
- fml::closure next_frame_callback_;
- bool user_override_resource_cache_bytes_;
- std::optional<size_t> max_cache_bytes_;
- fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_;
- fml::TaskRunnerAffineWeakPtrFactory<Rasterizer> weak_factory_;
- std::shared_ptr<ExternalViewEmbedder> external_view_embedder_;
  // |SnapshotDelegate|
  sk_sp<SkImage> MakeRasterSnapshot(
  std::function<void(SkCanvas*)> draw_callback,
@@ -500,6 +484,24 @@ class Rasterizer final : public SnapshotDelegate {
 
  static bool NoDiscard(const flutter::LayerTree& layer_tree) { return false; }
 
+ Delegate& delegate_;
+ std::unique_ptr<Surface> surface_;
+ std::unique_ptr<SnapshotSurfaceProducer> snapshot_surface_producer_;
+ std::unique_ptr<flutter::CompositorContext> compositor_context_;
+ // This is the last successfully rasterized layer tree.
+ std::unique_ptr<flutter::LayerTree> last_layer_tree_;
+ // Set when we need attempt to rasterize the layer tree again. This layer_tree
+ // has not successfully rasterized. This can happen due to the change in the
+ // thread configuration. This will be inserted to the front of the pipeline.
+ std::unique_ptr<flutter::LayerTree> resubmitted_layer_tree_;
+ fml::closure next_frame_callback_;
+ bool user_override_resource_cache_bytes_;
+ std::optional<size_t> max_cache_bytes_;
+ fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_;
+ std::shared_ptr<ExternalViewEmbedder> external_view_embedder_;
+
+ // WeakPtrFactory must be the last member.
+ fml::TaskRunnerAffineWeakPtrFactory<Rasterizer> weak_factory_;
  FML_DISALLOW_COPY_AND_ASSIGN(Rasterizer);
 };
 

shell/gpu/gpu_surface_gl.h

@@ -54,6 +54,14 @@ class GPUSurfaceGL : public Surface {
  bool AllowsDrawingWhenGpuDisabled() const override;
 
  private:
+ bool CreateOrUpdateSurfaces(const SkISize& size);
+
+ sk_sp<SkSurface> AcquireRenderSurface(
+ const SkISize& untransformed_size,
+ const SkMatrix& root_surface_transformation);
+
+ bool PresentSurface(SkCanvas* canvas);
+
  GPUSurfaceGLDelegate* delegate_;
  sk_sp<GrDirectContext> context_;
  sk_sp<SkSurface> onscreen_surface_;
@@ -66,16 +74,9 @@ class GPUSurfaceGL : public Surface {
  // external view embedder is present.
  const bool render_to_surface_ = true;
  bool valid_ = false;
- fml::TaskRunnerAffineWeakPtrFactory<GPUSurfaceGL> weak_factory_;
-
- bool CreateOrUpdateSurfaces(const SkISize& size);
-
- sk_sp<SkSurface> AcquireRenderSurface(
- const SkISize& untransformed_size,
- const SkMatrix& root_surface_transformation);
-
- bool PresentSurface(SkCanvas* canvas);
 
+ // WeakPtrFactory must be the last member.
+ fml::TaskRunnerAffineWeakPtrFactory<GPUSurfaceGL> weak_factory_;
  FML_DISALLOW_COPY_AND_ASSIGN(GPUSurfaceGL);
 };
 

shell/gpu/gpu_surface_software.h

@@ -39,7 +39,6 @@ class GPUSurfaceSoftware : public Surface {
  // external view embedder is present.
  const bool render_to_surface_;
  fml::TaskRunnerAffineWeakPtrFactory<GPUSurfaceSoftware> weak_factory_;
-
  FML_DISALLOW_COPY_AND_ASSIGN(GPUSurfaceSoftware);
 };
 

shell/gpu/gpu_surface_vulkan.h

@@ -55,7 +55,6 @@ class GPUSurfaceVulkan : public Surface {
  const bool render_to_surface_;
 
  fml::WeakPtrFactory<GPUSurfaceVulkan> weak_factory_;
-
  FML_DISALLOW_COPY_AND_ASSIGN(GPUSurfaceVulkan);
 };
 

shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

@@ -199,6 +199,10 @@ - (void)dealloc {
  object:self
  userInfo:nil];
 
+ // It will be destroyed and invalidate its weak pointers
+ // before any other members are destroyed.
+ _weakFactory.reset();
+
  /// nil out weak references.
  [_registrars
  enumerateKeysAndObjectsUsingBlock:^(id key, FlutterEngineRegistrar* registrar, BOOL* stop) {

shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm

@@ -49,6 +49,10 @@ - (void)addSecondaryResponder:(nonnull id<FlutterKeySecondaryResponder>)responde
 }
 
 - (void)dealloc {
+ // It will be destroyed and invalidate its weak pointers
+ // before any other members are destroyed.
+ _weakFactory.reset();
+
  [_primaryResponders removeAllObjects];
  [_secondaryResponders removeAllObjects];
  [_primaryResponders release];