ash: Fix exported fast ink texture leak.

Exported textures needs to be delete to not leak memory. This
adds an ExportedTextureDeleter helper class that can be used
to delete exported textures while still making sure the
compositor had a chance to consume them.

Bug: 764007
Test: manual
Change-Id: I18fa0494282f2352e73b320c05add8d944615213
Reviewed-on: https://chromium-review.googlesource.com/669586
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502518}

ash/fast_ink/fast_ink_view.cc

@@ -29,19 +29,67 @@
 #include "ui/views/widget/widget.h"
 
 namespace ash {
+namespace {
+
+// Helper class that can be used to delete exported textures while still
+// making sure the compositor had a chance to consume them.
+class ExportedTextureDeleter : public ui::CompositorObserver {
+ public:
+  static void DeleteTexture(ui::Compositor* compositor,
+                            viz::ContextProvider* context_provider,
+                            uint32_t texture) {
+    // Deletes itself after texture has been deleted or when compositor is
+    // shutting down.
+    compositor->AddObserver(
+        new ExportedTextureDeleter(compositor, context_provider, texture));
+  }
+
+  // Overridden from ui::CompositorObserver:
+  void OnCompositingDidCommit(ui::Compositor* compositor) override {}
+  void OnCompositingStarted(ui::Compositor* compositor,
+                            base::TimeTicks start_time) override {
+    compositing_started_ = true;
+  }
+  void OnCompositingEnded(ui::Compositor* compositor) override {
+    if (compositing_started_)
+      delete this;
+  }
+  void OnCompositingLockStateChanged(ui::Compositor* compositor) override {}
+  void OnCompositingShuttingDown(ui::Compositor* compositor) override {
+    delete this;
+  }
+
+ private:
+  ExportedTextureDeleter(ui::Compositor* compositor,
+                         viz::ContextProvider* context_provider,
+                         uint32_t texture)
+      : compositor_(compositor),
+        context_provider_(context_provider),
+        texture_(texture) {}
+  ~ExportedTextureDeleter() override {
+    context_provider_->ContextGL()->DeleteTextures(1, &texture_);
+    compositor_->RemoveObserver(this);
+  }
+
+  ui::Compositor* const compositor_;
+  scoped_refptr<viz::ContextProvider> context_provider_;
+  const uint32_t texture_;
+  bool compositing_started_ = false;
+};
 
-// This struct contains the resources associated with a fast ink frame.
-struct FastInkResource {
-  FastInkResource() {}
-  ~FastInkResource() {
-    if (context_provider) {
-      gpu::gles2::GLES2Interface* gles2 = context_provider->ContextGL();
-      // Delete texture if not currently exported.
-      if (texture && !exported)
-        gles2->DeleteTextures(1, &texture);
-      if (image)
-        gles2->DestroyImageCHROMIUM(image);
+}  // namespace
+
+struct FastInkView::Resource {
+  Resource() {}
+  ~Resource() {
+    gpu::gles2::GLES2Interface* gles2 = context_provider->ContextGL();
+    if (texture) {
+      // We shouldn't delete exported textures.
+      DCHECK(!exported);
+      gles2->DeleteTextures(1, &texture);
     }
+    if (image)
+      gles2->DestroyImageCHROMIUM(image);
   }
   scoped_refptr<viz::ContextProvider> context_provider;
   uint32_t texture = 0;
@@ -50,7 +98,6 @@ struct FastInkResource {
   bool exported = false;
 };
 
-// FastInkView
 FastInkView::FastInkView(aura::Window* root_window) : weak_ptr_factory_(this) {
   widget_.reset(new views::Widget);
   views::Widget::InitParams params;
@@ -85,6 +132,18 @@ FastInkView::FastInkView(aura::Window* root_window) : weak_ptr_factory_(this) {
 
 FastInkView::~FastInkView() {
   frame_sink_->DetachFromClient();
+
+  // Use ExportedTextureDeleter to delete currently exported textures. This
+  // is needed to ensure that in-flight textures are consumed before they are
+  // deleted.
+  ui::Compositor* compositor =
+      widget_->GetNativeWindow()->layer()->GetCompositor();
+  for (auto& it : exported_resources_) {
+    Resource* resource = it.second.get();
+    ExportedTextureDeleter::DeleteTexture(
+        compositor, resource->context_provider.get(), resource->texture);
+    resource->texture = 0;
+  }
 }
 
 void FastInkView::DidReceiveCompositorFrameAck() {
@@ -98,7 +157,7 @@ void FastInkView::ReclaimResources(
   for (auto& entry : resources) {
     auto it = exported_resources_.find(entry.id);
     DCHECK(it != exported_resources_.end());
-    std::unique_ptr<FastInkResource> resource = std::move(it->second);
+    std::unique_ptr<Resource> resource = std::move(it->second);
     exported_resources_.erase(it);
 
     DCHECK(resource->exported);
@@ -232,7 +291,7 @@ void FastInkView::UpdateSurface() {
   DCHECK(needs_update_surface_);
   needs_update_surface_ = false;
 
-  std::unique_ptr<FastInkResource> resource;
+  std::unique_ptr<Resource> resource;
   // Reuse returned resource if available.
   if (!returned_resources_.empty()) {
     resource = std::move(returned_resources_.back());
@@ -241,7 +300,7 @@ void FastInkView::UpdateSurface() {
 
   // Create new resource if needed.
   if (!resource)
-    resource = base::MakeUnique<FastInkResource>();
+    resource = base::MakeUnique<Resource>();
 
   // Acquire context provider for resource if needed.
   // Note: We make no attempts to recover if the context provider is later

ash/fast_ink/fast_ink_view.h

@@ -28,7 +28,6 @@ class Widget;
 }
 
 namespace ash {
-struct FastInkResource;
 
 // FastInkView is a view supporting low-latency rendering.
 class FastInkView : public views::View, public cc::LayerTreeFrameSinkClient {
@@ -64,6 +63,8 @@ class FastInkView : public views::View, public cc::LayerTreeFrameSinkClient {
   virtual void OnRedraw(gfx::Canvas& canvas) = 0;
 
  private:
+  struct Resource;
+
   void UpdateBuffer();
   void UpdateSurface();
   void OnDidDrawSurface();
@@ -77,9 +78,9 @@ class FastInkView : public views::View, public cc::LayerTreeFrameSinkClient {
   bool needs_update_surface_ = false;
   bool pending_draw_surface_ = false;
   int next_resource_id_ = 1;
-  base::flat_map<viz::ResourceId, std::unique_ptr<FastInkResource>>
+  base::flat_map<viz::ResourceId, std::unique_ptr<Resource>>
       exported_resources_;
-  std::vector<std::unique_ptr<FastInkResource>> returned_resources_;
+  std::vector<std::unique_ptr<Resource>> returned_resources_;
   std::unique_ptr<cc::LayerTreeFrameSink> frame_sink_;
   base::WeakPtrFactory<FastInkView> weak_ptr_factory_;