Delete ContextFactoryObserver

The only remaining usage is in tests. Convert the test to
ContextLossObserver instead and delete ContextFactoryObserver.

Bug: 947279
Change-Id: I0ec87d11a011ea315dc1a4cea3b8bf66f1f30f91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1546269Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735086}

content/browser/compositor/image_transport_factory_browsertest.cc

@@ -6,6 +6,7 @@
 
 #include "base/run_loop.h"
 #include "build/build_config.h"
+#include "components/viz/common/gpu/context_lost_observer.h"
 #include "components/viz/common/gpu/context_provider.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/public/test/content_browser_test.h"
@@ -19,9 +20,9 @@ namespace {
 
 using ImageTransportFactoryBrowserTest = ContentBrowserTest;
 
-class MockContextFactoryObserver : public ui::ContextFactoryObserver {
+class MockContextLostObserver : public viz::ContextLostObserver {
  public:
-  MOCK_METHOD0(OnLostSharedContext, void());
+  MOCK_METHOD0(OnContextLost, void());
 };
 
 // Flaky on ChromeOS: crbug.com/394083
@@ -39,15 +40,16 @@ IN_PROC_BROWSER_TEST_F(ImageTransportFactoryBrowserTest,
   if (GpuDataManagerImpl::GetInstance()->IsGpuCompositingDisabled())
     return;
 
-  MockContextFactoryObserver observer;
-  factory->GetContextFactory()->AddObserver(&observer);
+  scoped_refptr<viz::ContextProvider> context_provider =
+      factory->GetContextFactory()->SharedMainThreadContextProvider();
+
+  MockContextLostObserver observer;
+  context_provider->AddObserver(&observer);
 
   base::RunLoop run_loop;
-  EXPECT_CALL(observer, OnLostSharedContext())
+  EXPECT_CALL(observer, OnContextLost())
       .WillOnce(testing::Invoke(&run_loop, &base::RunLoop::Quit));
 
-  scoped_refptr<viz::ContextProvider> context_provider =
-      factory->GetContextFactory()->SharedMainThreadContextProvider();
   gpu::gles2::GLES2Interface* gl = context_provider->ContextGL();
   gl->LoseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
                           GL_INNOCENT_CONTEXT_RESET_ARB);
@@ -58,7 +60,7 @@ IN_PROC_BROWSER_TEST_F(ImageTransportFactoryBrowserTest,
 
   run_loop.Run();
 
-  factory->GetContextFactory()->RemoveObserver(&observer);
+  context_provider->RemoveObserver(&observer);
 }
 
 }  // anonymous namespace

content/browser/compositor/test/test_image_transport_factory.h

@@ -44,8 +44,6 @@ class TestImageTransportFactory : public ui::ContextFactory,
   void RemoveCompositor(ui::Compositor* compositor) override {}
   gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() override;
   cc::TaskGraphRunner* GetTaskGraphRunner() override;
-  void AddObserver(ui::ContextFactoryObserver* observer) override {}
-  void RemoveObserver(ui::ContextFactoryObserver* observer) override {}
 
   // ui::ContextFactoryPrivate implementation.
   viz::FrameSinkId AllocateFrameSinkId() override;

content/browser/compositor/viz_process_transport_factory.cc

@@ -119,9 +119,6 @@ VizProcessTransportFactory::VizProcessTransportFactory(
 }
 
 VizProcessTransportFactory::~VizProcessTransportFactory() {
-  if (main_context_provider_)
-    main_context_provider_->RemoveObserver(this);
-
   task_graph_runner_->Shutdown();
 }
 
@@ -209,6 +206,9 @@ VizProcessTransportFactory::SharedMainThreadContextProvider() {
   if (IsGpuCompositingDisabled())
     return nullptr;
 
+  if (main_context_provider_ && IsContextLost(main_context_provider_.get()))
+    main_context_provider_.reset();
+
   if (!main_context_provider_) {
     auto context_result = gpu::ContextResult::kTransientFailure;
     while (context_result == gpu::ContextResult::kTransientFailure) {
@@ -245,16 +245,6 @@ cc::TaskGraphRunner* VizProcessTransportFactory::GetTaskGraphRunner() {
   return task_graph_runner_.get();
 }
 
-void VizProcessTransportFactory::AddObserver(
-    ui::ContextFactoryObserver* observer) {
-  observer_list_.AddObserver(observer);
-}
-
-void VizProcessTransportFactory::RemoveObserver(
-    ui::ContextFactoryObserver* observer) {
-  observer_list_.RemoveObserver(observer);
-}
-
 void VizProcessTransportFactory::DisableGpuCompositing() {
   if (!IsGpuCompositingDisabled())
     DisableGpuCompositing(nullptr);
@@ -269,15 +259,6 @@ VizProcessTransportFactory::GetContextFactoryPrivate() {
   return &context_factory_private_;
 }
 
-void VizProcessTransportFactory::OnContextLost() {
-  // PostTask to avoid destroying |main_context_provider_| while it's still
-  // informing observers about the context loss.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&VizProcessTransportFactory::OnLostMainThreadSharedContext,
-                     weak_ptr_factory_.GetWeakPtr()));
-}
-
 void VizProcessTransportFactory::DisableGpuCompositing(
     ui::Compositor* guilty_compositor) {
 #if defined(OS_CHROMEOS)
@@ -306,15 +287,7 @@ void VizProcessTransportFactory::DisableGpuCompositing(
 
   // Drop our reference on the gpu contexts for the compositors.
   worker_context_provider_.reset();
-  if (main_context_provider_) {
-    main_context_provider_->RemoveObserver(this);
-    main_context_provider_.reset();
-  }
-
-  // Consumers of the shared main thread context aren't CompositingModeWatchers,
-  // so inform them about the context loss due to switching to software
-  // compositing.
-  OnLostMainThreadSharedContext();
+  main_context_provider_.reset();
 
   // Reemove the FrameSink from every compositor that needs to fall back to
   // software compositing.
@@ -433,10 +406,8 @@ VizProcessTransportFactory::TryCreateContextsForGpuCompositing(
     }
   }
 
-  if (main_context_provider_ && IsContextLost(main_context_provider_.get())) {
-    main_context_provider_->RemoveObserver(this);
+  if (main_context_provider_ && IsContextLost(main_context_provider_.get()))
     main_context_provider_.reset();
-  }
 
   if (!main_context_provider_) {
     constexpr bool kCompositorContextSupportsLocking = false;
@@ -460,23 +431,9 @@ VizProcessTransportFactory::TryCreateContextsForGpuCompositing(
       main_context_provider_.reset();
       return context_result;
     }
-
-    main_context_provider_->AddObserver(this);
   }
 
   return gpu::ContextResult::kSuccess;
 }
 
-void VizProcessTransportFactory::OnLostMainThreadSharedContext() {
-  // It's possible that |main_context_provider_| was already reset in
-  // OnEstablishedGpuChannel(), so check if it's lost before resetting here.
-  if (main_context_provider_ && IsContextLost(main_context_provider_.get())) {
-    main_context_provider_->RemoveObserver(this);
-    main_context_provider_.reset();
-  }
-
-  for (auto& observer : observer_list_)
-    observer.OnLostSharedContext();
-}
-
 }  // namespace content

content/browser/compositor/viz_process_transport_factory.h

@@ -9,7 +9,6 @@
 
 #include "base/macros.h"
 #include "build/build_config.h"
-#include "components/viz/common/gpu/context_lost_observer.h"
 #include "components/viz/service/main/viz_compositor_thread_runner_impl.h"
 #include "content/browser/compositor/image_transport_factory.h"
 #include "gpu/command_buffer/common/context_result.h"
@@ -44,8 +43,7 @@ namespace content {
 // Interface implementations to interact with the display compositor in the viz
 // process.
 class VizProcessTransportFactory : public ui::ContextFactory,
-                                   public ImageTransportFactory,
-                                   public viz::ContextLostObserver {
+                                   public ImageTransportFactory {
  public:
   VizProcessTransportFactory(
       gpu::GpuChannelEstablishFactory* gpu_channel_establish_factory,
@@ -67,17 +65,12 @@ class VizProcessTransportFactory : public ui::ContextFactory,
   void RemoveCompositor(ui::Compositor* compositor) override;
   gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() override;
   cc::TaskGraphRunner* GetTaskGraphRunner() override;
-  void AddObserver(ui::ContextFactoryObserver* observer) override;
-  void RemoveObserver(ui::ContextFactoryObserver* observer) override;
 
   // ImageTransportFactory implementation.
   void DisableGpuCompositing() override;
   ui::ContextFactory* GetContextFactory() override;
   ui::ContextFactoryPrivate* GetContextFactoryPrivate() override;
 
-  // viz::ContextLostObserver implementation.
-  void OnContextLost() override;
-
  private:
   // Disables GPU compositing. This notifies UI and renderer compositors to drop
   // LayerTreeFrameSinks and request new ones. If fallback happens while
@@ -107,16 +100,12 @@ class VizProcessTransportFactory : public ui::ContextFactory,
   gpu::ContextResult TryCreateContextsForGpuCompositing(
       scoped_refptr<gpu::GpuChannelHost> gpu_channel_host);
 
-  void OnLostMainThreadSharedContext();
-
   gpu::GpuChannelEstablishFactory* const gpu_channel_establish_factory_;
 
   // Controls the compositing mode based on what mode the display compositors
   // are using.
   viz::CompositingModeReporterImpl* const compositing_mode_reporter_;
 
-  base::ObserverList<ui::ContextFactoryObserver>::Unchecked observer_list_;
-
   // ContextProvider used on worker threads for rasterization.
   scoped_refptr<viz::RasterContextProvider> worker_context_provider_;
 

ui/compositor/compositor.h

@@ -80,21 +80,6 @@ class ScrollInputHandler;
 
 constexpr int kCompositorLockTimeoutMs = 67;
 
-class COMPOSITOR_EXPORT ContextFactoryObserver {
- public:
-  virtual ~ContextFactoryObserver() {}
-
-  // Notifies that the viz::ContextProviders returned from
-  // ui::ContextFactory::SharedMainThreadContextProvider and/or
-  // ui::ContextFactory::SharedMainThreadRasterContextProvider were lost.
-  // When this is called, the old resources (e.g. shared context, GL helper)
-  // still exist, but are about to be destroyed. Getting a reference to those
-  // resources from the ContextFactory (e.g. through
-  // SharedMainThreadContextProvider()) will return newly recreated, valid
-  // resources.
-  virtual void OnLostSharedContext() = 0;
-};
-
 // This is privileged interface to the compositor. It is a global object.
 class COMPOSITOR_EXPORT ContextFactoryPrivate {
  public:
@@ -179,10 +164,6 @@ class COMPOSITOR_EXPORT ContextFactory {
 
   // Gets the task graph runner.
   virtual cc::TaskGraphRunner* GetTaskGraphRunner() = 0;
-
-  virtual void AddObserver(ContextFactoryObserver* observer) = 0;
-
-  virtual void RemoveObserver(ContextFactoryObserver* observer) = 0;
 };
 
 // Compositor object to take care of GPU painting.

ui/compositor/test/fake_context_factory.h

@@ -40,8 +40,6 @@ class FakeContextFactory : public ui::ContextFactory {
   void RemoveCompositor(ui::Compositor* compositor) override;
   gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() override;
   cc::TaskGraphRunner* GetTaskGraphRunner() override;
-  void AddObserver(ui::ContextFactoryObserver* observer) override {}
-  void RemoveObserver(ui::ContextFactoryObserver* observer) override {}
 
  protected:
   const viz::RendererSettings& renderer_settings() const {

ui/compositor/test/in_process_context_factory.cc

@@ -193,11 +193,6 @@ InProcessContextFactory::~InProcessContextFactory() {
   DCHECK(per_compositor_data_.empty());
 }
 
-void InProcessContextFactory::SendOnLostSharedContext() {
-  for (auto& observer : observer_list_)
-    observer.OnLostSharedContext();
-}
-
 void InProcessContextFactory::SetUseFastRefreshRateForTests() {
   refresh_rate_ = 200.0;
 }
@@ -411,14 +406,6 @@ void InProcessContextFactory::SetDisplayVSyncParameters(
   iter->second->vsync_interval = interval;
 }
 
-void InProcessContextFactory::AddObserver(ContextFactoryObserver* observer) {
-  observer_list_.AddObserver(observer);
-}
-
-void InProcessContextFactory::RemoveObserver(ContextFactoryObserver* observer) {
-  observer_list_.RemoveObserver(observer);
-}
-
 SkMatrix44 InProcessContextFactory::GetOutputColorMatrix(
     Compositor* compositor) const {
   auto iter = per_compositor_data_.find(compositor);

ui/compositor/test/in_process_context_factory.h

@@ -54,10 +54,6 @@ class InProcessContextFactory : public ContextFactory,
     use_test_surface_ = use_test_surface;
   }
 
-  // This is used to call SendOnLostSharedContext() on all clients, to ensure
-  // they stop using the SharedMainThreadContextProvider.
-  void SendOnLostSharedContext();
-
   // Set refresh rate will be set to 200 to spend less time waiting for
   // BeginFrame when used for tests.
   void SetUseFastRefreshRateForTests();
@@ -97,8 +93,6 @@ class InProcessContextFactory : public ContextFactory,
       ui::Compositor* compositor,
       mojo::PendingRemote<viz::mojom::VSyncParameterObserver> observer)
       override {}
-  void AddObserver(ContextFactoryObserver* observer) override;
-  void RemoveObserver(ContextFactoryObserver* observer) override;
 
   SkMatrix44 GetOutputColorMatrix(Compositor* compositor) const;
   gfx::DisplayColorSpaces GetDisplayColorSpaces(
@@ -125,7 +119,6 @@ class InProcessContextFactory : public ContextFactory,
   double refresh_rate_ = 60.0;
   viz::HostFrameSinkManager* const host_frame_sink_manager_;
   viz::FrameSinkManagerImpl* const frame_sink_manager_;
-  base::ObserverList<ContextFactoryObserver>::Unchecked observer_list_;
 
   viz::RendererSettings renderer_settings_;
   using PerCompositorDataMap =

ui/compositor/test/test_context_factories.cc

@@ -39,10 +39,7 @@ TestContextFactories::TestContextFactories(bool enable_pixel_output,
   host_frame_sink_manager_->SetLocalManager(frame_sink_manager_.get());
 }
 
-TestContextFactories::~TestContextFactories() {
-  if (implicit_factory_)
-    implicit_factory_->SendOnLostSharedContext();
-}
+TestContextFactories::~TestContextFactories() = default;
 
 ContextFactory* TestContextFactories::GetContextFactory() const {
   return implicit_factory_.get();