Avoid registering both the platform view and the rasterizer in the shell. (#4293)

Instead, the rasterizer can be accessed via the platform view.

shell/common/platform_view.cc

@@ -17,10 +17,12 @@
 namespace shell {
 
 PlatformView::PlatformView(std::unique_ptr<Rasterizer> rasterizer)
-    : rasterizer_(std::move(rasterizer)), size_(SkISize::Make(0, 0)) {}
+    : rasterizer_(std::move(rasterizer)), size_(SkISize::Make(0, 0)) {
+  Shell::Shared().AddPlatformView(this);
+}
 
 PlatformView::~PlatformView() {
-  blink::Threads::UI()->PostTask([] { Shell::Shared().PurgePlatformViews(); });
+  Shell::Shared().RemovePlatformView(this);
 
   Rasterizer* rasterizer = rasterizer_.release();
   blink::Threads::Gpu()->PostTask([rasterizer]() { delete rasterizer; });
@@ -40,13 +42,6 @@ void PlatformView::CreateEngine() {
   engine_.reset(new Engine(this));
 }
 
-// Add this to the shell's list of PlatformVIews.
-// Subclasses should call this after the object is fully constructed.
-void PlatformView::PostAddToShellTask() {
-  blink::Threads::UI()->PostTask(
-      [self = shared_from_this()] { Shell::Shared().AddPlatformView(self); });
-}
-
 void PlatformView::DispatchPlatformMessage(
     fxl::RefPtr<blink::PlatformMessage> message) {
   blink::Threads::UI()->PostTask(

shell/common/platform_view.h

@@ -74,7 +74,6 @@ class PlatformView : public std::enable_shared_from_this<PlatformView> {
   explicit PlatformView(std::unique_ptr<Rasterizer> rasterizer);
 
   void CreateEngine();
-  void PostAddToShellTask();
 
   void SetupResourceContextOnIOThreadPerform(
       fxl::AutoResetWaitableEvent* event);

shell/common/platform_view_service_protocol.cc

@@ -210,29 +210,21 @@ bool PlatformViewServiceProtocol::ListViews(const char* method,
                                             intptr_t num_params,
                                             void* user_data,
                                             const char** json_object) {
-  // Ask the Shell for the list of platform views.
-  Shell& shell = Shell::Shared();
-  std::vector<Shell::PlatformViewInfo> platform_views;
-  shell.GetPlatformViewIds(&platform_views);
-
   std::stringstream response;
-
   response << "{\"type\":\"FlutterViewList\",\"views\":[";
   bool prefix_comma = false;
-  for (auto it = platform_views.begin(); it != platform_views.end(); it++) {
-    uintptr_t view_id = it->view_id;
-    int64_t isolate_id = it->isolate_id;
-    const std::string& isolate_name = it->isolate_name;
-    if (!view_id) {
-      continue;
-    }
-    if (prefix_comma) {
-      response << ',';
-    } else {
-      prefix_comma = true;
-    }
-    AppendFlutterView(&response, view_id, isolate_id, isolate_name);
-  }
+  Shell::Shared().IteratePlatformViews(
+      [&response, &prefix_comma](PlatformView* view) -> bool {
+        if (prefix_comma) {
+          response << ',';
+        } else {
+          prefix_comma = true;
+        }
+        AppendFlutterView(&response, reinterpret_cast<uintptr_t>(view),
+                          view->engine().GetUIIsolateMainPort(),
+                          view->engine().GetUIIsolateName());
+        return true;
+      });
   response << "]}";
   // Copy the response.
   *json_object = strdup(response.str().c_str());
@@ -258,6 +250,18 @@ static sk_sp<SkData> EncodeBitmapAsPNG(const SkBitmap& bitmap) {
   return data;
 }
 
+static fxl::WeakPtr<Rasterizer> GetRandomRasterizer() {
+  fxl::WeakPtr<Rasterizer> rasterizer;
+  Shell::Shared().IteratePlatformViews(
+      [&rasterizer](PlatformView* view) -> bool {
+        rasterizer = view->rasterizer().GetWeakRasterizerPtr();
+        // We just grab the first rasterizer so there is no need to iterate
+        // further.
+        return false;
+      });
+  return rasterizer;
+}
+
 bool PlatformViewServiceProtocol::Screenshot(const char* method,
                                              const char** param_keys,
                                              const char** param_values,
@@ -291,13 +295,9 @@ bool PlatformViewServiceProtocol::Screenshot(const char* method,
 }
 
 void PlatformViewServiceProtocol::ScreenshotGpuTask(SkBitmap* bitmap) {
-  std::vector<fxl::WeakPtr<Rasterizer>> rasterizers;
-  Shell::Shared().GetRasterizers(&rasterizers);
-  if (rasterizers.size() != 1)
-    return;
+  auto rasterizer = GetRandomRasterizer();
 
-  Rasterizer* rasterizer = rasterizers[0].get();
-  if (rasterizer == nullptr)
+  if (!rasterizer)
     return;
 
   flow::LayerTree* layer_tree = rasterizer->GetLastLayerTree();
@@ -357,13 +357,9 @@ bool PlatformViewServiceProtocol::ScreenshotSkp(const char* method,
 }
 
 sk_sp<SkPicture> PlatformViewServiceProtocol::ScreenshotSkpGpuTask() {
-  std::vector<fxl::WeakPtr<Rasterizer>> rasterizers;
-  Shell::Shared().GetRasterizers(&rasterizers);
-  if (rasterizers.size() != 1)
-    return nullptr;
+  auto rasterizer = GetRandomRasterizer();
 
-  Rasterizer* rasterizer = rasterizers[0].get();
-  if (rasterizer == nullptr)
+  if (!rasterizer)
     return nullptr;
 
   flow::LayerTree* layer_tree = rasterizer->GetLastLayerTree();

shell/common/shell.cc

@@ -28,14 +28,6 @@ namespace {
 
 static Shell* g_shell = nullptr;
 
-bool IsInvalid(const fxl::WeakPtr<Rasterizer>& rasterizer) {
-  return !rasterizer;
-}
-
-bool IsViewInvalid(const std::weak_ptr<PlatformView>& platform_view) {
-  return platform_view.expired();
-}
-
 template <typename T>
 bool GetSwitchValue(const fxl::CommandLine& command_line,
                     Switch sw,
@@ -195,10 +187,6 @@ const fxl::CommandLine& Shell::GetCommandLine() const {
   return command_line_;
 }
 
-TracingController& Shell::tracing_controller() {
-  return tracing_controller_;
-}
-
 void Shell::InitGpuThread() {
   gpu_thread_checker_.reset(new fxl::ThreadChecker());
 }
@@ -207,61 +195,32 @@ void Shell::InitUIThread() {
   ui_thread_checker_.reset(new fxl::ThreadChecker());
 }
 
-void Shell::AddRasterizer(const fxl::WeakPtr<Rasterizer>& rasterizer) {
-  FXL_DCHECK(gpu_thread_checker_ &&
-             gpu_thread_checker_->IsCreationThreadCurrent());
-  rasterizers_.push_back(rasterizer);
-}
-
-void Shell::PurgeRasterizers() {
-  FXL_DCHECK(gpu_thread_checker_ &&
-             gpu_thread_checker_->IsCreationThreadCurrent());
-  rasterizers_.erase(
-      std::remove_if(rasterizers_.begin(), rasterizers_.end(), IsInvalid),
-      rasterizers_.end());
-}
-
-void Shell::GetRasterizers(std::vector<fxl::WeakPtr<Rasterizer>>* rasterizers) {
-  FXL_DCHECK(gpu_thread_checker_ &&
-             gpu_thread_checker_->IsCreationThreadCurrent());
-  *rasterizers = rasterizers_;
-}
-
-void Shell::AddPlatformView(
-    const std::shared_ptr<PlatformView>& platform_view) {
-  std::lock_guard<std::mutex> lk(platform_views_mutex_);
-  if (platform_view) {
-    platform_views_.push_back(platform_view);
+void Shell::AddPlatformView(PlatformView* platform_view) {
+  if (platform_view == nullptr) {
+    return;
   }
+  fxl::MutexLocker lock(&platform_views_mutex_);
+  platform_views_.insert(platform_view);
 }
 
-void Shell::PurgePlatformViews() {
-  std::lock_guard<std::mutex> lk(platform_views_mutex_);
-  platform_views_.erase(std::remove_if(platform_views_.begin(),
-                                       platform_views_.end(), IsViewInvalid),
-                        platform_views_.end());
+void Shell::RemovePlatformView(PlatformView* platform_view) {
+  if (platform_view == nullptr) {
+    return;
+  }
+  fxl::MutexLocker lock(&platform_views_mutex_);
+  platform_views_.erase(platform_view);
 }
 
-void Shell::GetPlatformViews(
-    std::vector<std::weak_ptr<PlatformView>>* platform_views) {
-  std::lock_guard<std::mutex> lk(platform_views_mutex_);
-  *platform_views = platform_views_;
-}
-
-void Shell::GetPlatformViewIds(
-    std::vector<PlatformViewInfo>* platform_view_ids) {
-  std::lock_guard<std::mutex> lk(platform_views_mutex_);
-  for (auto it = platform_views_.begin(); it != platform_views_.end(); it++) {
-    std::shared_ptr<PlatformView> view = it->lock();
-    if (!view) {
-      // Skip dead views.
-      continue;
+void Shell::IteratePlatformViews(
+    std::function<bool(PlatformView*)> iterator) const {
+  if (iterator == nullptr) {
+    return;
+  }
+  fxl::MutexLocker lock(&platform_views_mutex_);
+  for (PlatformView* view : platform_views_) {
+    if (!iterator(view)) {
+      return;
     }
-    PlatformViewInfo info;
-    info.view_id = reinterpret_cast<uintptr_t>(view.get());
-    info.isolate_id = view->engine().GetUIIsolateMainPort();
-    info.isolate_name = view->engine().GetUIIsolateName();
-    platform_view_ids->push_back(info);
   }
 }
 
@@ -302,18 +261,28 @@ void Shell::RunInPlatformViewUIThread(uintptr_t view_id,
 
   *view_existed = false;
 
-  for (auto it = platform_views_.begin(); it != platform_views_.end(); it++) {
-    std::shared_ptr<PlatformView> view = it->lock();
-    if (!view)
-      continue;
-    if (reinterpret_cast<uintptr_t>(view.get()) == view_id) {
-      *view_existed = true;
-      view->RunFromSource(assets_directory, main, packages);
-      *dart_isolate_id = view->engine().GetUIIsolateMainPort();
-      *isolate_name = view->engine().GetUIIsolateName();
-      break;
-    }
-  }
+  IteratePlatformViews(
+      [
+        view_id,                                         // argument
+        assets_directory = std::move(assets_directory),  // argument
+        main = std::move(main),                          // argument
+        packages = std::move(packages),                  // argument
+        &view_existed,                                   // out
+        &dart_isolate_id,                                // out
+        &isolate_name                                    // out
+      ](PlatformView * view)
+          ->bool {
+            if (reinterpret_cast<uintptr_t>(view) != view_id) {
+              // Keep looking.
+              return true;
+            }
+            *view_existed = true;
+            view->RunFromSource(assets_directory, main, packages);
+            *dart_isolate_id = view->engine().GetUIIsolateMainPort();
+            *isolate_name = view->engine().GetUIIsolateName();
+            // We found the requested view. Stop iterating over platform views.
+            return false;
+          });
 
   latch->Signal();
 }

shell/common/shell.h

@@ -5,22 +5,24 @@
 #ifndef SHELL_COMMON_SHELL_H_
 #define SHELL_COMMON_SHELL_H_
 
+#include <unordered_set>
+
 #include "flutter/fml/thread.h"
 #include "flutter/shell/common/tracing_controller.h"
 #include "lib/fxl/command_line.h"
+#include "lib/fxl/functional/closure.h"
 #include "lib/fxl/macros.h"
 #include "lib/fxl/memory/ref_ptr.h"
 #include "lib/fxl/memory/weak_ptr.h"
+#include "lib/fxl/synchronization/mutex.h"
+#include "lib/fxl/synchronization/thread_annotations.h"
 #include "lib/fxl/synchronization/thread_checker.h"
 #include "lib/fxl/synchronization/waitable_event.h"
 #include "lib/fxl/tasks/task_runner.h"
 
-#include <mutex>
-
 namespace shell {
 
 class PlatformView;
-class Rasterizer;
 
 class Shell {
  public:
@@ -35,31 +37,12 @@ class Shell {
 
   const fxl::CommandLine& GetCommandLine() const;
 
-  TracingController& tracing_controller();
+  void AddPlatformView(PlatformView* platform_view);
 
-  // Maintain a list of rasterizers.
-  // These APIs must only be accessed on the GPU thread.
-  void AddRasterizer(const fxl::WeakPtr<Rasterizer>& rasterizer);
-  void PurgeRasterizers();
-  void GetRasterizers(std::vector<fxl::WeakPtr<Rasterizer>>* rasterizer);
+  void RemovePlatformView(PlatformView* platform_view);
 
-  // List of PlatformViews.
-
-  // These APIs can be called from any thread.
-  void AddPlatformView(const std::shared_ptr<PlatformView>& platform_view);
-  void PurgePlatformViews();
-  void GetPlatformViews(
-      std::vector<std::weak_ptr<PlatformView>>* platform_views);
-
-  struct PlatformViewInfo {
-    uintptr_t view_id;
-    int64_t isolate_id;
-    std::string isolate_name;
-  };
-
-  // These APIs can be called from any thread.
-  // Return the list of platform view ids at the time of this call.
-  void GetPlatformViewIds(std::vector<PlatformViewInfo>* platform_view_ids);
+  void IteratePlatformViews(
+      std::function<bool /* continue */ (PlatformView*)> iterator) const;
 
   // Attempt to run a script inside a flutter view indicated by |view_id|.
   // Will set |view_existed| to true if the view was found and false otherwise.
@@ -72,12 +55,24 @@ class Shell {
                          std::string* isolate_name);
 
  private:
+  fxl::CommandLine command_line_;
+  std::unique_ptr<fml::Thread> gpu_thread_;
+  std::unique_ptr<fml::Thread> ui_thread_;
+  std::unique_ptr<fml::Thread> io_thread_;
+  std::unique_ptr<fxl::ThreadChecker> gpu_thread_checker_;
+  std::unique_ptr<fxl::ThreadChecker> ui_thread_checker_;
+  TracingController tracing_controller_;
+  mutable fxl::Mutex platform_views_mutex_;
+  std::unordered_set<PlatformView*> platform_views_
+      FXL_GUARDED_BY(platform_views_mutex_);
+
   static void Init(fxl::CommandLine command_line,
                    const std::string& bundle_path);
 
   Shell(fxl::CommandLine command_line);
 
   void InitGpuThread();
+
   void InitUIThread();
 
   void RunInPlatformViewUIThread(uintptr_t view_id,
@@ -89,22 +84,6 @@ class Shell {
                                  std::string* isolate_name,
                                  fxl::AutoResetWaitableEvent* latch);
 
-  fxl::CommandLine command_line_;
-
-  std::unique_ptr<fml::Thread> gpu_thread_;
-  std::unique_ptr<fml::Thread> ui_thread_;
-  std::unique_ptr<fml::Thread> io_thread_;
-
-  std::unique_ptr<fxl::ThreadChecker> gpu_thread_checker_;
-  std::unique_ptr<fxl::ThreadChecker> ui_thread_checker_;
-
-  TracingController tracing_controller_;
-
-  std::vector<fxl::WeakPtr<Rasterizer>> rasterizers_;
-  std::vector<std::weak_ptr<PlatformView>> platform_views_;
-
-  std::mutex platform_views_mutex_;
-
   FXL_DISALLOW_COPY_AND_ASSIGN(Shell);
 };
 

shell/gpu/gpu_rasterizer.cc

@@ -17,16 +17,9 @@
 namespace shell {
 
 GPURasterizer::GPURasterizer(std::unique_ptr<flow::ProcessInfo> info)
-    : compositor_context_(std::move(info)), weak_factory_(this) {
-  auto weak_ptr = weak_factory_.GetWeakPtr();
-  blink::Threads::Gpu()->PostTask(
-      [weak_ptr]() { Shell::Shared().AddRasterizer(weak_ptr); });
-}
+    : compositor_context_(std::move(info)), weak_factory_(this) {}
 
-GPURasterizer::~GPURasterizer() {
-  weak_factory_.InvalidateWeakPtrs();
-  Shell::Shared().PurgeRasterizers();
-}
+GPURasterizer::~GPURasterizer() = default;
 
 fxl::WeakPtr<Rasterizer> GPURasterizer::GetWeakRasterizerPtr() {
   return weak_factory_.GetWeakPtr();

shell/platform/android/platform_view_android.cc

@@ -139,8 +139,6 @@ void PlatformViewAndroid::Attach() {
   SetupResourceContextOnIOThread();
 
   UpdateThreadPriorities();
-
-  PostAddToShellTask();
 }
 
 void PlatformViewAndroid::Detach() {

shell/platform/darwin/desktop/platform_view_mac.mm

@@ -29,7 +29,6 @@ PlatformViewMac::~PlatformViewMac() = default;
 
 void PlatformViewMac::Attach() {
   CreateEngine();
-  PostAddToShellTask();
 }
 
 void PlatformViewMac::SetupAndLoadDart() {

shell/platform/embedder/platform_view_embedder.cc

@@ -37,7 +37,6 @@ bool PlatformViewEmbedder::SurfaceSupportsSRGB() const {
 
 void PlatformViewEmbedder::Attach() {
   CreateEngine();
-  PostAddToShellTask();
   NotifyCreated(std::make_unique<shell::GPUSurfaceGL>(this));
 }
 

shell/testing/platform_view_test.cc

@@ -14,7 +14,6 @@ PlatformViewTest::PlatformViewTest()
 
 void PlatformViewTest::Attach() {
   CreateEngine();
-  PostAddToShellTask();
 }
 
 PlatformViewTest::~PlatformViewTest() = default;