Fix TextureLayerImpl::ReleaseResources().

This CL fixes a bug where LayerTreeHostImpl::OnPurgeMemory() causes
TextureLayerImpl to register duplicate SharedBitmapIds. OnPurgeMemory()
ends up calling TextureLayerImpl::ReleaseResources(), which assumed that
the LayerTreeFrameSink was lost if it was called. SharedBitmapIds should
only be reregistered if the LayerTreeFrameSink is actually lost.

Add new OnPurgeMemory() function on LayerImpl. This will by default keep
the old behaviour by calling ReleaseResources() but TextureLayerImpl can
override it to not do anything.

Bug: 862584
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I32cd4e45d629b2f8410adff80f712f1b517a1407
Reviewed-on: https://chromium-review.googlesource.com/1169430Reviewed-by: weiliangc <weiliangc@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582045}

cc/layers/layer_impl.cc

@@ -680,6 +680,10 @@ void LayerImpl::DidBeginTracing() {}
 
 void LayerImpl::ReleaseResources() {}
 
+void LayerImpl::OnPurgeMemory() {
+  ReleaseResources();
+}
+
 void LayerImpl::ReleaseTileResources() {}
 
 void LayerImpl::RecreateTileResources() {}

cc/layers/layer_impl.h

@@ -372,8 +372,13 @@ class CC_EXPORT LayerImpl {
   // that rendered this layer was lost.
   virtual void ReleaseResources();
 
+  // Releases resources in response to memory pressure. The default
+  // implementation just calls ReleaseResources() and subclasses will override
+  // if that's not appropriate.
+  virtual void OnPurgeMemory();
+
   // Release tile resources held by this layer. Called when a rendering mode
-  // switch has occured and tiles are no longer valid.
+  // switch has occurred and tiles are no longer valid.
   virtual void ReleaseTileResources();
 
   // Recreate tile resources held by this layer after they were released by a

cc/layers/texture_layer_impl.cc

@@ -170,6 +170,13 @@ SimpleEnclosedRegion TextureLayerImpl::VisibleOpaqueRegion() const {
   return SimpleEnclosedRegion();
 }
 
+void TextureLayerImpl::OnPurgeMemory() {
+  // Do nothing here intentionally as the LayerTreeFrameSink isn't lost.
+  // Unregistering SharedBitmapIds with the LayerTreeFrameSink wouldn't free
+  // the shared memory, as the TextureLayer and/or TextureLayerClient will still
+  // have a reference to it.
+}
+
 void TextureLayerImpl::ReleaseResources() {
   // Gpu resources are lost when the LayerTreeFrameSink is lost. But software
   // resources are still valid, and we can keep them here in that case.

cc/layers/texture_layer_impl.h

@@ -41,6 +41,7 @@ class CC_EXPORT TextureLayerImpl : public LayerImpl {
                    AppendQuadsData* append_quads_data) override;
   SimpleEnclosedRegion VisibleOpaqueRegion() const override;
   void ReleaseResources() override;
+  void OnPurgeMemory() override;
 
   // These setter methods don't cause any implicit damage, so the texture client
   // must explicitly invalidate if they intend to cause a visible change in the

cc/layers/texture_layer_unittest.cc

@@ -1611,6 +1611,86 @@ class SoftwareTextureLayerSwitchTreesTest : public SoftwareTextureLayerTest {
 
 SINGLE_AND_MULTI_THREAD_TEST_F(SoftwareTextureLayerSwitchTreesTest);
 
+// Verify that duplicate SharedBitmapIds aren't registered if resources are
+// purged due to memory pressure.
+class SoftwareTextureLayerPurgeMemoryTest : public SoftwareTextureLayerTest {
+ protected:
+  void BeginTest() override {
+    PostSetNeedsCommitToMainThread();
+
+    const gfx::Size size(1, 1);
+    const viz::ResourceFormat format = viz::RGBA_8888;
+
+    id_ = viz::SharedBitmap::GenerateId();
+    bitmap_ = base::MakeRefCounted<CrossThreadSharedBitmap>(
+        id_, viz::bitmap_allocation::AllocateMappedBitmap(size, format), size,
+        format);
+  }
+
+  void DidCommitAndDrawFrame() override {
+    step_ = layer_tree_host()->SourceFrameNumber();
+    switch (step_) {
+      case 1:
+        // The test starts by inserting the TextureLayer to the tree.
+        root_->AddChild(texture_layer_);
+        // And registers a SharedBitmapId, which should be given to the
+        // LayerTreeFrameSink.
+        registration_ = texture_layer_->RegisterSharedBitmapId(id_, bitmap_);
+        // Give the TextureLayer a resource so it contributes to the frame. It
+        // doesn't need to register the SharedBitmapId otherwise.
+        texture_layer_->SetTransferableResource(
+            viz::TransferableResource::MakeSoftware(id_, gfx::Size(1, 1),
+                                                    viz::RGBA_8888),
+            viz::SingleReleaseCallback::Create(
+                base::BindOnce([](const gpu::SyncToken&, bool) {})));
+        break;
+      case 2:
+        // Draw again after OnPurgeMemory() was called on the impl thread so we
+        // can verify that duplicate SharedBitmapIds aren't registered by
+        // TextureLayerImpl.
+        texture_layer_->SetNeedsDisplay();
+        break;
+      case 3:
+        // Release the TransferableResource before shutdown.
+        texture_layer_->ClearClient();
+        break;
+      case 4:
+        EndTest();
+    }
+  }
+
+  void DrawLayersOnThread(LayerTreeHostImpl* host_impl) override {
+    // TextureLayerImpl will have registered the SharedBitmapId at this point.
+    // Call OnPurgeMemory() to ensure that the same SharedBitmapId doesn't get
+    // registered again on the next draw.
+    if (step_ == 1)
+      static_cast<base::MemoryCoordinatorClient*>(host_impl)->OnPurgeMemory();
+  }
+
+  void DisplayReceivedCompositorFrameOnThread(
+      const viz::CompositorFrame& frame) override {
+    if (step_ == 0) {
+      // Before commit 1, the |texture_layer_| has no SharedBitmapId yet.
+      EXPECT_THAT(frame_sink_->owned_bitmaps(), testing::IsEmpty());
+      verified_frames_++;
+    } else {
+      // After commit 1, we added a SharedBitmapId to |texture_layer_|.
+      EXPECT_THAT(frame_sink_->owned_bitmaps(), testing::ElementsAre(id_));
+      verified_frames_++;
+    }
+  }
+
+  void AfterTest() override { EXPECT_EQ(4, verified_frames_); }
+
+  int step_ = 0;
+  int verified_frames_ = 0;
+  viz::SharedBitmapId id_;
+  SharedBitmapIdRegistration registration_;
+  scoped_refptr<CrossThreadSharedBitmap> bitmap_;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(SoftwareTextureLayerPurgeMemoryTest);
+
 class SoftwareTextureLayerMultipleRegisterTest
     : public SoftwareTextureLayerTest {
  protected:

cc/trees/layer_tree_host_impl.cc

@@ -2818,8 +2818,13 @@ void LayerTreeHostImpl::ActivateStateForImages() {
 
 void LayerTreeHostImpl::OnPurgeMemory() {
   ReleaseTileResources();
-  ReleaseTreeResources();
-  ClearUIResources();
+  active_tree_->OnPurgeMemory();
+  if (pending_tree_)
+    pending_tree_->OnPurgeMemory();
+  if (recycle_tree_)
+    recycle_tree_->OnPurgeMemory();
+
+  EvictAllUIResources();
   if (image_decode_cache_) {
     image_decode_cache_->SetShouldAggressivelyFreeResources(true);
     image_decode_cache_->SetShouldAggressivelyFreeResources(false);

cc/trees/layer_tree_impl.cc

@@ -134,6 +134,13 @@ void LayerTreeImpl::ReleaseResources() {
   }
 }
 
+void LayerTreeImpl::OnPurgeMemory() {
+  if (!LayerListIsEmpty()) {
+    LayerTreeHostCommon::CallFunctionForEveryLayer(
+        this, [](LayerImpl* layer) { layer->OnPurgeMemory(); });
+  }
+}
+
 void LayerTreeImpl::ReleaseTileResources() {
   if (!LayerListIsEmpty()) {
     LayerTreeHostCommon::CallFunctionForEveryLayer(

cc/trees/layer_tree_impl.h

@@ -101,6 +101,7 @@ class CC_EXPORT LayerTreeImpl {
 
   void Shutdown();
   void ReleaseResources();
+  void OnPurgeMemory();
   void ReleaseTileResources();
   void RecreateTileResources();