Revert "viz: RGBA_TEXTURE copy requests in SkiaRenderer"

This reverts commit 4a52a132.

Reason for revert: Crashing the GPU process between pixel_tests, about 40% of the runs. crbug.com/1111312

Original change's description:
> viz: RGBA_TEXTURE copy requests in SkiaRenderer
> 
> Currently, SkiaRenderer does not support RGBA_TEXTURE
> CopyOutputRequests which are used in Chrome OS for screen rotation
> animations [1] (among other things). As part of a larger effort to
> enable SkiaRenderer on Chrome OS, this CL implements RGBA_TEXTURE
> support in SkiaOutputSurfaceImplOnGpu::CopyOutput:
> 
> 1. Refactor SharedImageFactory ownership out of
>    DirectContextProviderDelegateImpl (used only for the
>    use_gl_renderer_copier bypass) and directly into
>    SkiaOutputSurfaceImplOnGpu.
> 2. Create a SharedImage and its Skia representation in CopyOutput and
>    blit the request rect from the source into the output SkSurface.
> 3. Send the SharedImage's mailbox back through the CopyOutputResult
>    with a release callback that destroys the SharedImage.
> 4. Enable SKIA_GL tests in cc_unittests.
> 
> [1] https://source.chromium.org/chromium/chromium/src/+/master:ash/rotator/screen_rotation_animator.cc;l=214;drc=b15cb5fd43aff2d181355401d99e47e2e44aa61f?originalUrl=https:%2F%2Fcs.chromium.org%2F
> 
> Bug: 1046788, 971257, 1098435
> Change-Id: I62d41390828ed0c79a1dc29c508efc748924ce06
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302733
> Reviewed-by: Khushal <khushalsagar@chromium.org>
> Reviewed-by: Jonathan Backer <backer@chromium.org>
> Commit-Queue: Brian Ho <hob@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#792971}

TBR=backer@chromium.org,dcastagna@chromium.org,khushalsagar@chromium.org,hob@chromium.org

Change-Id: I67bc898f661319f387453696cda55076f8b114b6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1046788
Bug: 971257
Bug: 1098435
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2329730Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793144}

cc/trees/layer_tree_host_pixeltest_readback.cc

@@ -420,7 +420,12 @@ ReadbackTestConfig const kTestConfigs[] = {
     ReadbackTestConfig{TestRendererType::kSoftware, TestReadBackType::kBitmap},
     ReadbackTestConfig{TestRendererType::kGL, TestReadBackType::kTexture},
     ReadbackTestConfig{TestRendererType::kGL, TestReadBackType::kBitmap},
-    ReadbackTestConfig{TestRendererType::kSkiaGL, TestReadBackType::kTexture},
+    // TODO(crbug.com/1046788): The skia readback path doesn't support
+    // RGBA_TEXTURE readback requests yet. Don't run these tests on platforms
+    // that have UseSkiaForGLReadback enabled by default.
+    //
+    // ReadbackTestConfig{TestRendererType::kSkiaGL,
+    //                    TestReadBackType::kTexture},
     ReadbackTestConfig{TestRendererType::kSkiaGL, TestReadBackType::kBitmap},
 #if defined(ENABLE_CC_VULKAN_TESTS)
     ReadbackTestConfig{TestRendererType::kSkiaVk, TestReadBackType::kBitmap},
@@ -440,7 +445,12 @@ ReadbackTestConfig const kMaybeVulkanTestConfigs[] = {
     ReadbackTestConfig{TestRendererType::kSoftware, TestReadBackType::kBitmap},
     ReadbackTestConfig{TestRendererType::kGL, TestReadBackType::kTexture},
     ReadbackTestConfig{TestRendererType::kGL, TestReadBackType::kBitmap},
-    ReadbackTestConfig{TestRendererType::kSkiaGL, TestReadBackType::kTexture},
+    // TODO(crbug.com/1046788): The skia readback path doesn't support
+    // RGBA_TEXTURE readback requests yet. Don't run these tests on platforms
+    // that have UseSkiaForGLReadback enabled by default.
+    //
+    // ReadbackTestConfig{TestRendererType::kSkiaGL,
+    //                    TestReadBackType::kTexture},
     ReadbackTestConfig{TestRendererType::kSkiaGL, TestReadBackType::kBitmap},
 #if defined(ENABLE_CC_VULKAN_TESTS) && !defined(THREAD_SANITIZER) && \
     !defined(MEMORY_SANITIZER)

components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.h

@@ -26,7 +26,6 @@
 #include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/command_buffer/common/sync_token.h"
 #include "gpu/command_buffer/service/shared_context_state.h"
-#include "gpu/command_buffer/service/shared_image_representation.h"
 #include "gpu/command_buffer/service/sync_point_manager.h"
 #include "gpu/ipc/service/context_url.h"
 #include "gpu/ipc/service/display_context.h"
@@ -46,7 +45,6 @@ class GLSurface;
 
 namespace gpu {
 class SharedImageRepresentationFactory;
-class SharedImageFactory;
 class SyncPointClientState;
 }
 
@@ -206,7 +204,7 @@ class SkiaOutputSurfaceImplOnGpu
     num_readbacks_pending_--;
   }
 
-  gpu::MemoryTracker* GetMemoryTracker() { return memory_tracker_; }
+  gpu::MemoryTracker* GetMemoryTracker() { return memory_tracker_.get(); }
 
  private:
   class OffscreenSurface;
@@ -228,12 +226,6 @@ class SkiaOutputSurfaceImplOnGpu
   bool MakeCurrent(bool need_fbo0);
   void MarkContextLost(ContextLostReason reason);
 
-  void DestroySharedImageOnImplThread(
-      std::unique_ptr<gpu::SharedImageRepresentationSkia> representation,
-      scoped_refptr<gpu::SharedContextState> context_state,
-      const gpu::SyncToken& sync_token,
-      bool is_lost);
-
   void PullTextureUpdates(std::vector<gpu::SyncToken> sync_token);
 
   void ReleaseFenceSyncAndPushTextureUpdates(uint64_t sync_fence_release);
@@ -285,10 +277,9 @@ class SkiaOutputSurfaceImplOnGpu
   SkiaOutputSurfaceDependency* const dependency_;
   scoped_refptr<gpu::gles2::FeatureInfo> feature_info_;
   scoped_refptr<gpu::SyncPointClientState> sync_point_client_state_;
-  gpu::MemoryTracker* memory_tracker_;
+  std::unique_ptr<gpu::MemoryTracker> memory_tracker_;
   std::unique_ptr<gpu::SharedImageRepresentationFactory>
       shared_image_representation_factory_;
-  std::unique_ptr<gpu::SharedImageFactory> shared_image_factory_;
   VulkanContextProvider* const vulkan_context_provider_;
   DawnContextProvider* const dawn_context_provider_;
   const RendererSettings renderer_settings_;

gpu/command_buffer/service/shared_context_state.cc

@@ -78,15 +78,15 @@ void SharedContextState::compileError(const char* shader, const char* errors) {
   }
 }
 
-SharedContextState::MemoryTrackerObserver::MemoryTrackerObserver(
+SharedContextState::MemoryTracker::MemoryTracker(
     base::WeakPtr<gpu::MemoryTracker::Observer> peak_memory_monitor)
     : peak_memory_monitor_(peak_memory_monitor) {}
 
-SharedContextState::MemoryTrackerObserver::~MemoryTrackerObserver() {
+SharedContextState::MemoryTracker::~MemoryTracker() {
   DCHECK(!size_);
 }
 
-void SharedContextState::MemoryTrackerObserver::OnMemoryAllocatedChange(
+void SharedContextState::MemoryTracker::OnMemoryAllocatedChange(
     CommandBufferId id,
     uint64_t old_size,
     uint64_t new_size,
@@ -100,45 +100,6 @@ void SharedContextState::MemoryTrackerObserver::OnMemoryAllocatedChange(
   }
 }
 
-base::AtomicSequenceNumber g_next_command_buffer_id;
-
-SharedContextState::MemoryTracker::MemoryTracker(Observer* observer)
-    : command_buffer_id_(gpu::CommandBufferId::FromUnsafeValue(
-          g_next_command_buffer_id.GetNext() + 1)),
-      client_tracing_id_(base::trace_event::MemoryDumpManager::GetInstance()
-                             ->GetTracingProcessId()),
-      observer_(observer) {}
-
-SharedContextState::MemoryTracker::~MemoryTracker() {
-  DCHECK(!size_);
-}
-
-void SharedContextState::MemoryTracker::TrackMemoryAllocatedChange(
-    int64_t delta) {
-  DCHECK(delta >= 0 || size_ >= static_cast<uint64_t>(-delta));
-  uint64_t old_size = size_;
-  size_ += delta;
-  DCHECK(observer_);
-  observer_->OnMemoryAllocatedChange(command_buffer_id_, old_size, size_,
-                                     gpu::GpuPeakMemoryAllocationSource::SKIA);
-}
-
-uint64_t SharedContextState::MemoryTracker::GetSize() const {
-  return size_;
-}
-
-uint64_t SharedContextState::MemoryTracker::ClientTracingId() const {
-  return client_tracing_id_;
-}
-
-int SharedContextState::MemoryTracker::ClientId() const {
-  return gpu::ChannelIdFromCommandBufferId(command_buffer_id_);
-}
-
-uint64_t SharedContextState::MemoryTracker::ContextGroupTracingId() const {
-  return command_buffer_id_.GetUnsafeValue();
-}
-
 SharedContextState::SharedContextState(
     scoped_refptr<gl::GLShareGroup> share_group,
     scoped_refptr<gl::GLSurface> surface,
@@ -153,9 +114,7 @@ SharedContextState::SharedContextState(
     : use_virtualized_gl_contexts_(use_virtualized_gl_contexts),
       context_lost_callback_(std::move(context_lost_callback)),
       gr_context_type_(gr_context_type),
-      memory_tracker_observer_(peak_memory_monitor),
-      memory_tracker_(&memory_tracker_observer_),
-      memory_type_tracker_(&memory_tracker_),
+      memory_tracker_(peak_memory_monitor),
       vk_context_provider_(vulkan_context_provider),
       metal_context_provider_(metal_context_provider),
       dawn_context_provider_(dawn_context_provider),
@@ -213,6 +172,7 @@ SharedContextState::SharedContextState(
   // Initialize the scratch buffer to some small initial size.
   scratch_deserialization_buffer_.resize(
       kInitialScratchDeserializationBufferSize);
+
 }
 
 SharedContextState::~SharedContextState() {
@@ -235,8 +195,8 @@ SharedContextState::~SharedContextState() {
   DCHECK(!owned_gr_context_ || owned_gr_context_->unique());
 
   // GPU memory allocations except skia_gr_cache_size_ tracked by this
-  // memory_tracker_observer_ should have been released.
-  DCHECK_EQ(skia_gr_cache_size_, memory_tracker_observer_.GetMemoryUsage());
+  // memory_tracker_ should have been released.
+  DCHECK_EQ(skia_gr_cache_size_, memory_tracker_.GetMemoryUsage());
   // gr_context_ and all resources owned by it will be released soon, so set it
   // to null, and UpdateSkiaOwnedMemorySize() will update skia memory usage to
   // 0, to ensure that PeakGpuMemoryMonitor sees 0 allocated memory.
@@ -551,9 +511,6 @@ bool SharedContextState::IsCurrent(gl::GLSurface* surface) {
   return context_->IsCurrent(surface);
 }
 
-// TODO(https://crbug.com/1110357): Account for memory tracked by
-// memory_tracker_ and memory_type_tracker_ (e.g. SharedImages allocated in
-// SkiaOutputSurfaceImplOnGpu::CopyOutput).
 bool SharedContextState::OnMemoryDump(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
@@ -617,13 +574,13 @@ void SharedContextState::PurgeMemory(
 
 uint64_t SharedContextState::GetMemoryUsage() {
   UpdateSkiaOwnedMemorySize();
-  return memory_tracker_observer_.GetMemoryUsage();
+  return memory_tracker_.GetMemoryUsage();
 }
 
 void SharedContextState::UpdateSkiaOwnedMemorySize() {
   if (!gr_context_) {
-    memory_tracker_observer_.OnMemoryAllocatedChange(CommandBufferId(),
-                                                     skia_gr_cache_size_, 0u);
+    memory_tracker_.OnMemoryAllocatedChange(CommandBufferId(),
+                                            skia_gr_cache_size_, 0u);
     skia_gr_cache_size_ = 0u;
     return;
   }
@@ -632,7 +589,7 @@ void SharedContextState::UpdateSkiaOwnedMemorySize() {
   // Skia does not have a CommandBufferId. PeakMemoryMonitor currently does not
   // use CommandBufferId to identify source, so use zero here to separate
   // prevent confusion.
-  memory_tracker_observer_.OnMemoryAllocatedChange(
+  memory_tracker_.OnMemoryAllocatedChange(
       CommandBufferId(), skia_gr_cache_size_, static_cast<uint64_t>(new_size));
   skia_gr_cache_size_ = static_cast<uint64_t>(new_size);
 }

gpu/command_buffer/service/shared_context_state.h

@@ -24,7 +24,6 @@
 #include "gpu/command_buffer/service/memory_tracking.h"
 #include "gpu/config/gpu_preferences.h"
 #include "gpu/gpu_gles2_export.h"
-#include "gpu/ipc/common/command_buffer_id.h"
 #include "gpu/ipc/common/gpu_peak_memory.h"
 #include "gpu/vulkan/buildflags.h"
 #include "third_party/skia/include/core/SkSurface.h"
@@ -154,13 +153,7 @@ class GPU_GLES2_EXPORT SharedContextState
   bool support_vulkan_external_object() const {
     return support_vulkan_external_object_;
   }
-  gpu::MemoryTracker::Observer* memory_tracker_observer() {
-    return &memory_tracker_observer_;
-  }
-  gpu::MemoryTracker* memory_tracker() { return &memory_tracker_; }
-  gpu::MemoryTypeTracker* memory_type_tracker() {
-    return &memory_type_tracker_;
-  }
+  gpu::MemoryTracker::Observer* memory_tracker() { return &memory_tracker_; }
   ExternalSemaphorePool* external_semaphore_pool() {
 #if BUILDFLAG(ENABLE_VULKAN)
     return external_semaphore_pool_.get();
@@ -220,14 +213,13 @@ class GPU_GLES2_EXPORT SharedContextState
 
   // Observer which is notified when SkiaOutputSurfaceImpl takes ownership of a
   // shared image, and forward information to both histograms and task manager.
-  class GPU_GLES2_EXPORT MemoryTrackerObserver
-      : public gpu::MemoryTracker::Observer {
+  class GPU_GLES2_EXPORT MemoryTracker : public gpu::MemoryTracker::Observer {
    public:
-    explicit MemoryTrackerObserver(
+    explicit MemoryTracker(
         base::WeakPtr<gpu::MemoryTracker::Observer> peak_memory_monitor);
-    MemoryTrackerObserver(MemoryTrackerObserver&) = delete;
-    MemoryTrackerObserver& operator=(MemoryTrackerObserver&) = delete;
-    ~MemoryTrackerObserver() override;
+    MemoryTracker(MemoryTracker&) = delete;
+    MemoryTracker& operator=(MemoryTracker&) = delete;
+    ~MemoryTracker() override;
 
     // gpu::MemoryTracker::Observer implementation:
     void OnMemoryAllocatedChange(
@@ -245,29 +237,6 @@ class GPU_GLES2_EXPORT SharedContextState
     base::WeakPtr<gpu::MemoryTracker::Observer> const peak_memory_monitor_;
   };
 
-  // MemoryTracker implementation used to track SharedImages owned by
-  // SkiaOutputSurfaceImpl.
-  class MemoryTracker : public gpu::MemoryTracker {
-   public:
-    explicit MemoryTracker(gpu::MemoryTracker::Observer* observer);
-    MemoryTracker(const MemoryTracker&) = delete;
-    MemoryTracker& operator=(const MemoryTracker&) = delete;
-    ~MemoryTracker() override;
-
-    // MemoryTracker implementation:
-    void TrackMemoryAllocatedChange(int64_t delta) override;
-    uint64_t GetSize() const override;
-    uint64_t ClientTracingId() const override;
-    int ClientId() const override;
-    uint64_t ContextGroupTracingId() const override;
-
-   private:
-    gpu::CommandBufferId command_buffer_id_;
-    const uint64_t client_tracing_id_;
-    gpu::MemoryTracker::Observer* const observer_;
-    uint64_t size_ = 0;
-  };
-
   ~SharedContextState() override;
 
   // gpu::GLContextVirtualDelegate implementation.
@@ -294,9 +263,7 @@ class GPU_GLES2_EXPORT SharedContextState
   bool support_vulkan_external_object_ = false;
   ContextLostCallback context_lost_callback_;
   GrContextType gr_context_type_ = GrContextType::kGL;
-  MemoryTrackerObserver memory_tracker_observer_;
   MemoryTracker memory_tracker_;
-  gpu::MemoryTypeTracker memory_type_tracker_;
   viz::VulkanContextProvider* const vk_context_provider_;
   viz::MetalContextProvider* const metal_context_provider_;
   viz::DawnContextProvider* const dawn_context_provider_;