Simplify weak pointer factories and allow vending weak pointers on multiple threads. (#5029)

fml/memory/weak_ptr.h

@@ -159,35 +159,20 @@ class WeakPtr {
 template <typename T>
 class WeakPtrFactory {
  public:
-  explicit WeakPtrFactory(T* ptr) : ptr_(ptr) { FXL_DCHECK(ptr_); }
-
-  ~WeakPtrFactory() { InvalidateWeakPtrs(); }
-
-  // Gets a new weak pointer, which will be valid until either
-  // |InvalidateWeakPtrs()| is called or this object is destroyed.
-  WeakPtr<T> GetWeakPtr() {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
-    if (!flag_)
-      flag_ = fxl::MakeRefCounted<fml::internal::WeakPtrFlag>();
-    return WeakPtr<T>(ptr_, flag_.Clone(), checker_);
+  explicit WeakPtrFactory(T* ptr)
+      : ptr_(ptr), flag_(fxl::MakeRefCounted<fml::internal::WeakPtrFlag>()) {
+    FXL_DCHECK(ptr_);
   }
 
-  // Call this method to invalidate all existing weak pointers. (Note that
-  // additional weak pointers can be produced even after this is called.)
-  void InvalidateWeakPtrs() {
+  ~WeakPtrFactory() {
     FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
-    if (!flag_)
-      return;
     flag_->Invalidate();
-    flag_ = nullptr;
   }
 
-  // Call this method to determine if any weak pointers exist. (Note that a
-  // "false" result is definitive, but a "true" result may not be if weak
-  // pointers are held/reset/destroyed/reassigned on other threads.)
-  bool HasWeakPtrs() const {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
-    return flag_ && !flag_->HasOneRef();
+  // Gets a new weak pointer, which will be valid until either
+  // |InvalidateWeakPtrs()| is called or this object is destroyed.
+  WeakPtr<T> GetWeakPtr() const {
+    return WeakPtr<T>(ptr_, flag_.Clone(), checker_);
   }
 
  private:

fml/memory/weak_ptr_unittest.cc

@@ -162,36 +162,5 @@ TEST(WeakPtrTest, UpcastMoveAssignment) {
   EXPECT_EQ(&data, ptr2.get());
 }
 
-TEST(WeakPtrTest, InvalidateWeakPtrs) {
-  int data = 0;
-  WeakPtrFactory<int> factory(&data);
-  WeakPtr<int> ptr = factory.GetWeakPtr();
-  EXPECT_EQ(&data, ptr.get());
-  EXPECT_TRUE(factory.HasWeakPtrs());
-  factory.InvalidateWeakPtrs();
-  EXPECT_EQ(nullptr, ptr.get());
-  EXPECT_FALSE(factory.HasWeakPtrs());
-
-  // Test that the factory can create new weak pointers after a
-  // |InvalidateWeakPtrs()| call, and that they remain valid until the next
-  // |InvalidateWeakPtrs()| call.
-  WeakPtr<int> ptr2 = factory.GetWeakPtr();
-  EXPECT_EQ(&data, ptr2.get());
-  EXPECT_TRUE(factory.HasWeakPtrs());
-  factory.InvalidateWeakPtrs();
-  EXPECT_EQ(nullptr, ptr2.get());
-  EXPECT_FALSE(factory.HasWeakPtrs());
-}
-
-TEST(WeakPtrTest, HasWeakPtrs) {
-  int data = 0;
-  WeakPtrFactory<int> factory(&data);
-  {
-    WeakPtr<int> ptr = factory.GetWeakPtr();
-    EXPECT_TRUE(factory.HasWeakPtrs());
-  }
-  EXPECT_FALSE(factory.HasWeakPtrs());
-}
-
 }  // namespace
 }  // namespace fml

runtime/dart_isolate.cc

@@ -107,7 +107,6 @@ DartIsolate::DartIsolate(const DartVM* vm,
       isolate_snapshot_(std::move(isolate_snapshot)),
       weak_factory_(this) {
   FXL_DCHECK(isolate_snapshot_) << "Must contain a valid isolate snapshot.";
-  weak_prototype_ = weak_factory_.GetWeakPtr();
 
   if (vm_ == nullptr) {
     return;
@@ -679,7 +678,7 @@ fxl::RefPtr<DartSnapshot> DartIsolate::GetIsolateSnapshot() const {
 }
 
 fml::WeakPtr<DartIsolate> DartIsolate::GetWeakIsolatePtr() const {
-  return weak_prototype_;
+  return weak_factory_.GetWeakPtr();
 }
 
 void DartIsolate::AddIsolateShutdownCallback(fxl::Closure closure) {

runtime/dart_isolate.h

@@ -103,7 +103,6 @@ class DartIsolate : public UIDartState {
   Phase phase_ = Phase::Unknown;
   const fxl::RefPtr<DartSnapshot> isolate_snapshot_;
   std::vector<std::unique_ptr<AutoFireClosure>> shutdown_callbacks_;
-  fml::WeakPtr<DartIsolate> weak_prototype_;
   fml::WeakPtrFactory<DartIsolate> weak_factory_;
 
   FXL_WARN_UNUSED_RESULT

shell/common/engine.cc

@@ -56,8 +56,6 @@ Engine::Engine(Delegate& delegate,
       activity_running_(false),
       have_surface_(false),
       weak_factory_(this) {
-  weak_prototype_ = weak_factory_.GetWeakPtr();
-
   if (legacy_sky_platform_) {
     // TODO: Remove this legacy call along with the platform. This is what makes
     // the engine unable to run from multiple threads in the legacy
@@ -84,7 +82,7 @@ Engine::~Engine() {
 }
 
 fml::WeakPtr<Engine> Engine::GetWeakPtr() const {
-  return weak_prototype_;
+  return weak_factory_.GetWeakPtr();
 }
 
 bool Engine::UpdateAssetManager(

shell/common/engine.h

@@ -109,7 +109,6 @@ class Engine final : public blink::RuntimeDelegate {
   fxl::RefPtr<blink::AssetManager> asset_manager_;
   bool activity_running_;
   bool have_surface_;
-  fml::WeakPtr<Engine> weak_prototype_;
   fml::WeakPtrFactory<Engine> weak_factory_;
 
   // |blink::RuntimeDelegate|

shell/common/io_manager.cc

@@ -49,11 +49,6 @@ IOManager::IOManager(sk_sp<GrContext> resource_context,
                          "context. Async texture uploads will be disabled. "
                          "Expect performance degradation.";
   }
-
-  if (resource_context_weak_factory_) {
-    resource_context_weak_prototype_ =
-        resource_context_weak_factory_->GetWeakPtr();
-  }
 }
 
 IOManager::~IOManager() {
@@ -63,7 +58,9 @@ IOManager::~IOManager() {
 }
 
 fml::WeakPtr<GrContext> IOManager::GetResourceContext() const {
-  return resource_context_weak_prototype_;
+  return resource_context_weak_factory_
+             ? resource_context_weak_factory_->GetWeakPtr()
+             : fml::WeakPtr<GrContext>();
 }
 
 fxl::RefPtr<flow::SkiaUnrefQueue> IOManager::GetSkiaUnrefQueue() const {

shell/common/io_manager.h

@@ -35,7 +35,6 @@ class IOManager {
  private:
   // Resource context management.
   sk_sp<GrContext> resource_context_;
-  fml::WeakPtr<GrContext> resource_context_weak_prototype_;
   std::unique_ptr<fml::WeakPtrFactory<GrContext>>
       resource_context_weak_factory_;
 

shell/common/platform_view.cc

@@ -20,9 +20,7 @@ PlatformView::PlatformView(Delegate& delegate, blink::TaskRunners task_runners)
     : delegate_(delegate),
       task_runners_(std::move(task_runners)),
       size_(SkISize::Make(0, 0)),
-      weak_factory_(this) {
-  weak_prototype_ = weak_factory_.GetWeakPtr();
-}
+      weak_factory_(this) {}
 
 PlatformView::~PlatformView() = default;
 
@@ -73,7 +71,7 @@ sk_sp<GrContext> PlatformView::CreateResourceContext() const {
 }
 
 fml::WeakPtr<PlatformView> PlatformView::GetWeakPtr() const {
-  return weak_prototype_;
+  return weak_factory_.GetWeakPtr();
 }
 
 void PlatformView::UpdateSemantics(blink::SemanticsNodeUpdates update) {}

shell/common/platform_view.h

@@ -120,7 +120,6 @@ class PlatformView {
   std::unique_ptr<VsyncWaiter> vsync_waiter_;
 
   SkISize size_;
-  fml::WeakPtr<PlatformView> weak_prototype_;
   fml::WeakPtrFactory<PlatformView> weak_factory_;
 
   virtual std::unique_ptr<Surface> CreateRenderingSurface();

shell/common/rasterizer.cc

@@ -16,14 +16,12 @@
 namespace shell {
 
 Rasterizer::Rasterizer(blink::TaskRunners task_runners)
-    : task_runners_(std::move(task_runners)), weak_factory_(this) {
-  weak_prototype_ = weak_factory_.GetWeakPtr();
-}
+    : task_runners_(std::move(task_runners)), weak_factory_(this) {}
 
 Rasterizer::~Rasterizer() = default;
 
 fml::WeakPtr<Rasterizer> Rasterizer::GetWeakPtr() const {
-  return weak_prototype_;
+  return weak_factory_.GetWeakPtr();
 }
 
 void Rasterizer::Setup(std::unique_ptr<Surface> surface) {

shell/common/rasterizer.h

@@ -65,7 +65,6 @@ class Rasterizer final {
   flow::CompositorContext compositor_context_;
   std::unique_ptr<flow::LayerTree> last_layer_tree_;
   fxl::Closure next_frame_callback_;
-  fml::WeakPtr<Rasterizer> weak_prototype_;
   fml::WeakPtrFactory<Rasterizer> weak_factory_;
 
   void DoDraw(std::unique_ptr<flow::LayerTree> layer_tree);