Surfaces: Allow parent to refer to child surfaces that don't exist yet.

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/macros.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/surfaces/compositor_frame_sink_support_client.h"
 #include "cc/surfaces/frame_sink_id.h"
 #include "cc/surfaces/surface_id.h"
 #include "cc/test/begin_frame_args_test.h"
 #include "cc/test/fake_external_begin_frame_source.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::ElementsAre;
 using testing::UnorderedElementsAre;
 using testing::IsEmpty;
 using testing::SizeIs;
 
 namespace cc {
 namespace test {
 namespace {
 
 constexpr FrameSinkId kParentFrameSink(2, 1);
 constexpr FrameSinkId kChildFrameSink1(65563, 1);
@@ skipping 45 matching lines @@
     return child_support2().current_surface_for_testing();
   }
 
   CompositorFrameSinkSupport& support(int index) { return *supports_[index]; }
   Surface* surface(int index) {
     return support(index).current_surface_for_testing();
   }
 
   SurfaceManager& surface_manager() { return surface_manager_; }
 
+  // Returns all the references where |surface_id| is the parent.
+  const SurfaceManager::SurfaceIdSet& GetReferencesFrom(

[vmpstr, 2017/02/10 19:36:00]

GetChildReferences?

[Fady Samuel, 2017/02/10 20:39:10]

Done.

+      const SurfaceId& surface_id) {
+    return surface_manager().parent_to_child_refs_[surface_id];
+  }
+
+  // Returns all the temporary references for the given frame sink id.
+  std::vector<LocalSurfaceId> GetTempReferencesFor(

[vmpstr, 2017/02/10 19:36:00]

GetTempReferences?

[Fady Samuel, 2017/02/10 20:39:10]

Done.

+      const FrameSinkId& frame_sink_id) {
+    return surface_manager().temp_references_[frame_sink_id];
+  }
+
   SurfaceDependencyTracker& dependency_tracker() {
     return *surface_manager_.dependency_tracker();
   }
 
   FakeExternalBeginFrameSource* begin_frame_source() {
     return begin_frame_source_.get();
   }
 
   // testing::Test:
   void SetUp() override {
@@ skipping 344 matching lines @@
   EXPECT_TRUE(parent_surface()->HasPendingFrame());
   EXPECT_THAT(parent_surface()->blocking_surfaces_for_testing(),
               UnorderedElementsAre(arbitrary_id));
   // Verify that the parent will add a reference to |arbitrary_id| and will not
   // remove a reference to |child_id|.
   EXPECT_THAT(parent_reference_tracker().references_to_add(),
               UnorderedElementsAre(parent_arbitrary_reference));
   EXPECT_THAT(parent_reference_tracker().references_to_remove(), IsEmpty());
 }
 
+// This test verifies that a SurfaceReference from parent to child can be added
+// prior to the child submitting a CompositorFrame. This test also verifies that
+// when the child later submits a CompositorFrame,
+TEST_F(CompositorFrameSinkSupportTest,
+       DisplayCompositorLockingReferenceAddedBeforeChildExists) {
+  const SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+  const SurfaceId child_id = MakeSurfaceId(kChildFrameSink1, 1);
+
+  // The parent submits a CompositorFrame that depends on |child_id| before the
+  // child submits a CompositorFrame.
+  parent_support().SubmitCompositorFrame(parent_id.local_surface_id(),
+                                         MakeCompositorFrame({child_id}));
+
+  // Verify that the CompositorFrame is blocked on |child_id|.
+  EXPECT_FALSE(parent_surface()->HasActiveFrame());
+  EXPECT_TRUE(parent_surface()->HasPendingFrame());
+  EXPECT_THAT(parent_surface()->blocking_surfaces_for_testing(),
+              UnorderedElementsAre(child_id));

[vmpstr, 2017/02/10 19:36:00]

Does this mean there's just one element and it's child_id? It's kind of weird to
use EXPECT_THAT for that :P

[kylechar, 2017/02/10 19:51:27]

I like EXPECT_THAT, so I'm kind of biased, but it seems less verbose than
writing something like:

EXPECT_EQ(std::unordered_set<SurfaceId>{child_id},
parent_surface()->blocking_surfaces_for_testing());

It's also tolerant to changing the container type.

[Fady Samuel, 2017/02/10 20:39:10]

I like EXPECT_THAT too. Learned it from Kyle's code and adopted it :-)

+
+  // Verify that a SurfaceReference(parent_id, child_id) exists in the
+  // SurfaceManager.
+  EXPECT_THAT(GetReferencesFrom(parent_id), ElementsAre(child_id));

[vmpstr, 2017/02/10 19:36:00]

same here

[Fady Samuel, 2017/02/10 20:39:10]

Acknowledged.

+
+  child_support1().SubmitCompositorFrame(
+      child_id.local_surface_id(), MakeCompositorFrame(empty_surface_ids()));
+
+  // Verify that the child CompositorFrame activates immediately.
+  EXPECT_TRUE(child_surface1()->HasActiveFrame());
+  EXPECT_FALSE(child_surface1()->HasPendingFrame());
+  EXPECT_THAT(child_surface1()->blocking_surfaces_for_testing(), IsEmpty());
+
+  // Verify that there is no temporary reference for the child and that
+  // the reference from the parent to the child still exists.
+  EXPECT_THAT(GetTempReferencesFor(child_id.frame_sink_id()), IsEmpty());
+  EXPECT_THAT(GetReferencesFrom(parent_id), ElementsAre(child_id));
+}
+
 }  // namespace test
 }  // namespace cc