SurfaceManager::CreateSurface must set the surface's factory

cc/surfaces/compositor_frame_sink_support_unittest.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/surfaces/compositor_frame_sink_support.h"
 
 #include "base/debug/stack_trace.h"
 #include "base/macros.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/surfaces/compositor_frame_sink_support_client.h"
@@ skipping 137 matching lines @@
   }
 
   SurfaceDependencyTracker& dependency_tracker() {
     return *surface_manager_.dependency_tracker();
   }
 
   FakeExternalBeginFrameSource* begin_frame_source() {
     return begin_frame_source_.get();
   }
 
+  std::unique_ptr<CompositorFrameSinkSupport> CreateCompositorFrameSinkSupport(
+      FrameSinkId frame_sink_id,
+      bool is_root = false) {
+    return base::MakeUnique<CompositorFrameSinkSupport>(
+        &support_client_, &surface_manager_, frame_sink_id, is_root,
+        true /* handles_frame_sink_id_invalidation */,
+        true /* needs_sync_points */);
+  }
+
   // testing::Test:
   void SetUp() override {
     testing::Test::SetUp();
     begin_frame_source_ =
         base::MakeUnique<FakeExternalBeginFrameSource>(0.f, false);
     std::unique_ptr<SurfaceDependencyTracker> dependency_tracker(
         new SurfaceDependencyTracker(&surface_manager_,
                                      begin_frame_source_.get()));
     surface_manager_.SetDependencyTracker(std::move(dependency_tracker));
-    supports_.push_back(base::MakeUnique<CompositorFrameSinkSupport>(
-        &support_client_, &surface_manager_, kDisplayFrameSink,
-        true /* is_root */, true /* handles_frame_sink_id_invalidation */,
-        true /* needs_sync_points */));
-    supports_.push_back(base::MakeUnique<CompositorFrameSinkSupport>(
-        &support_client_, &surface_manager_, kParentFrameSink,
-        false /* is_root */, true /* handles_frame_sink_id_invalidation */,
-        true /* needs_sync_points */));
-    supports_.push_back(base::MakeUnique<CompositorFrameSinkSupport>(
-        &support_client_, &surface_manager_, kChildFrameSink1,
-        false /* is_root */, true /* handles_frame_sink_id_invalidation */,
-        true /* needs_sync_points */));
-    supports_.push_back(base::MakeUnique<CompositorFrameSinkSupport>(
-        &support_client_, &surface_manager_, kChildFrameSink2,
-        false /* is_root */, true /* handles_frame_sink_id_invalidation */,
-        true /* needs_sync_points */));
-
+    supports_.push_back(CreateCompositorFrameSinkSupport(kDisplayFrameSink,
+                                                         true /* is_root */));
+    supports_.push_back(CreateCompositorFrameSinkSupport(kParentFrameSink));
+    supports_.push_back(CreateCompositorFrameSinkSupport(kChildFrameSink1));
+    supports_.push_back(CreateCompositorFrameSinkSupport(kChildFrameSink2));
     // Normally, the BeginFrameSource would be registered by the Display. We
     // register it here so that BeginFrames are received by the display support,
     // for use in the PassesOnBeginFrameAcks test. Other supports do not receive
     // BeginFrames, since the frame sink hierarchy is not set up in this test.
     surface_manager_.RegisterBeginFrameSource(begin_frame_source_.get(),
                                               kDisplayFrameSink);
   }
 
   void TearDown() override {
     surface_manager_.SetDependencyTracker(nullptr);
@@ skipping 851 matching lines @@
   child_support1().EvictFrame();
   EXPECT_EQ(nullptr, surface_manager().GetSurfaceForId(child_id));
 
   // Submit another frame with the same local surface id. This should work fine
   // and a new surface must be created.
   child_support1().SubmitCompositorFrame(child_id.local_surface_id(),
                                          CompositorFrame());
   EXPECT_NE(nullptr, surface_manager().GetSurfaceForId(child_id));
 }
 
+// Checks whether a LocalSurfaceId can be reused after the
+// CompositorFrameSinkSupport is destroyed and recreated with the same
+// FrameSinkId.
+TEST_F(CompositorFrameSinkSupportTest, ReuseSurfaceAfterSupportDestroyed) {
+  const SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+  const SurfaceId child_id = MakeSurfaceId(kArbitraryFrameSink, 1);
+  std::unique_ptr<CompositorFrameSinkSupport> child_support =
+      CreateCompositorFrameSinkSupport(kArbitraryFrameSink);
+
+  // Add a reference from parent to child to preserve the child surface.
+  parent_support().SubmitCompositorFrame(parent_id.local_surface_id(),
+                                         MakeCompositorFrame({child_id}));
+
+  // Submit the first frame. A surface must be created.
+  child_support->SubmitCompositorFrame(
+      child_id.local_surface_id(), MakeCompositorFrame(empty_surface_ids()));
+  Surface* surface = surface_manager().GetSurfaceForId(child_id);
+  ASSERT_TRUE(surface);
+  base::WeakPtr<SurfaceFactory> factory = surface->factory();
+  EXPECT_TRUE(factory);
+  EXPECT_FALSE(surface->destroyed());
+
+  // Recreate child_support. The surface must be marked as destroyed and its
+  // factory must be gone.
+  child_support.reset();
+  child_support = CreateCompositorFrameSinkSupport(kArbitraryFrameSink);
+  EXPECT_EQ(surface, surface_manager().GetSurfaceForId(child_id));
+  EXPECT_FALSE(factory);
+  EXPECT_TRUE(surface->destroyed());
+
+  // Submit another frame for the same local surface id. The same surface must
+  // be used and destroyed() must return false. The surface must have a factory.
+  child_support->SubmitCompositorFrame(child_id.local_surface_id(),
+                                       CompositorFrame());

[Fady Samuel, 2017/03/17 01:01:10]

Oops, here too: MakeCompositorFrame(empty_surface_ids())

[Saman Sami, 2017/03/20 16:26:09]

Done.

+  EXPECT_EQ(surface, surface_manager().GetSurfaceForId(child_id));
+  EXPECT_TRUE(surface->factory());
+  EXPECT_FALSE(surface->destroyed());
+}
+
 }  // namespace test
 }  // namespace cc