Do not use interface pointers in ScenicSurfaceFactory

OzonePlatformScenic used to own a mojom::ScenicGpuHostPtr, but it
was actually only used to initialize other objects. Instead, we moved
ownership of this object to ScenicSurfaceFactory.

Bug: 955171
Change-Id: Iad1bd211e543c663faab050b488578311a0b232a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1886616Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jacobo Aragunde Pérez <jaragunde@igalia.com>
Cr-Commit-Position: refs/heads/master@{#710706}

ui/ozone/platform/scenic/ozone_platform_scenic.cc

@@ -13,6 +13,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop_current.h"
 #include "base/message_loop/message_pump_type.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "ui/base/cursor/ozone/bitmap_cursor_factory_ozone.h"
 #include "ui/base/ime/fuchsia/input_method_fuchsia.h"
 #include "ui/display/fake/fake_display_delegate.h"
@@ -135,11 +136,11 @@ class OzonePlatformScenic
     base::MessageLoopCurrent::Get()->AddDestructionObserver(this);
 
     scenic_gpu_host_ = std::make_unique<ScenicGpuHost>(window_manager_.get());
-    scenic_gpu_host_remote_.Bind(
-        scenic_gpu_host_->CreateHostProcessSelfRemote());
 
-    surface_factory_ =
-        std::make_unique<ScenicSurfaceFactory>(scenic_gpu_host_remote_.get());
+    // SurfaceFactory is configured here to use a ui-process remote for software
+    // output.
+    surface_factory_ = std::make_unique<ScenicSurfaceFactory>(
+        scenic_gpu_host_->CreateHostProcessSelfRemote());
   }
 
   void InitializeGPU(const InitParams& params) override {
@@ -149,14 +150,19 @@ class OzonePlatformScenic
         // Some test such as gpu_unittests do this.
         // TODO(spang): This is not ideal; perhaps we should move the GL &
         // vulkan initializers out of SurfaceFactoryOzone.
-        surface_factory_ = std::make_unique<ScenicSurfaceFactory>(nullptr);
+        surface_factory_ =
+            std::make_unique<ScenicSurfaceFactory>(mojo::NullRemote());
       }
     } else {
       DCHECK(!surface_factory_);
+      mojo::PendingRemote<mojom::ScenicGpuHost> scenic_gpu_host_remote;
       scenic_gpu_service_ = std::make_unique<ScenicGpuService>(
-          scenic_gpu_host_remote_.BindNewPipeAndPassReceiver());
-      surface_factory_ =
-          std::make_unique<ScenicSurfaceFactory>(scenic_gpu_host_remote_.get());
+          scenic_gpu_host_remote.InitWithNewPipeAndPassReceiver());
+
+      // SurfaceFactory is configured here to use a gpu-process remote. The
+      // other end of the pipe will be attached through ScenicGpuService.
+      surface_factory_ = std::make_unique<ScenicSurfaceFactory>(
+          std::move(scenic_gpu_host_remote));
     }
   }
 
@@ -176,7 +182,6 @@ class OzonePlatformScenic
   void WillDestroyCurrentMessageLoop() override {
     // We must ensure to destroy any resources which rely on the MessageLoop's
     // async_dispatcher.
-    scenic_gpu_host_remote_.reset();
     surface_factory_ = nullptr;
     scenic_gpu_host_ = nullptr;
     overlay_manager_ = nullptr;
@@ -196,8 +201,6 @@ class OzonePlatformScenic
   std::unique_ptr<ScenicGpuService> scenic_gpu_service_;
   std::unique_ptr<ScenicSurfaceFactory> surface_factory_;
 
-  mojo::Remote<mojom::ScenicGpuHost> scenic_gpu_host_remote_;
-
   DISALLOW_COPY_AND_ASSIGN(OzonePlatformScenic);
 };
 

ui/ozone/platform/scenic/scenic_surface_factory.cc

@@ -74,8 +74,9 @@ fuchsia::sysmem::AllocatorSyncPtr ConnectSysmemAllocator() {
 
 }  // namespace
 
-ScenicSurfaceFactory::ScenicSurfaceFactory(mojom::ScenicGpuHost* gpu_host)
-    : gpu_host_(gpu_host),
+ScenicSurfaceFactory::ScenicSurfaceFactory(
+    mojo::PendingRemote<mojom::ScenicGpuHost> gpu_host)
+    : gpu_host_(std::move(gpu_host)),
       egl_implementation_(std::make_unique<GLOzoneEGLScenic>()),
       sysmem_buffer_manager_(ConnectSysmemAllocator()),
       weak_ptr_factory_(this) {

ui/ozone/platform/scenic/scenic_surface_factory.h

@@ -16,6 +16,8 @@
 #include "base/thread_annotations.h"
 #include "base/threading/thread_checker.h"
 #include "gpu/vulkan/buildflags.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/system/handle.h"
 #include "ui/ozone/platform/scenic/sysmem_buffer_manager.h"
 #include "ui/ozone/public/gl_ozone.h"
@@ -28,7 +30,8 @@ class ScenicSurface;
 
 class ScenicSurfaceFactory : public SurfaceFactoryOzone {
  public:
-  explicit ScenicSurfaceFactory(mojom::ScenicGpuHost* gpu_host);
+  explicit ScenicSurfaceFactory(
+      mojo::PendingRemote<mojom::ScenicGpuHost> gpu_host);
   ~ScenicSurfaceFactory() override;
 
   // SurfaceFactoryOzone implementation.
@@ -92,7 +95,7 @@ class ScenicSurfaceFactory : public SurfaceFactoryOzone {
       GUARDED_BY(surface_lock_);
   base::Lock surface_lock_;
 
-  mojom::ScenicGpuHost* const gpu_host_;
+  mojo::Remote<mojom::ScenicGpuHost> const gpu_host_;
   std::unique_ptr<GLOzone> egl_implementation_;
 
   fuchsia::ui::scenic::ScenicPtr scenic_;