Fix bug removing temporary surface references.

If there are multiple surfaces from the same FrameSinkId that have
temporary references we might not remove one of the temporary
references.  We track the temporary references in DisplayConfigurator
and remove them when a real reference is added. An iterator is
incremented twice in some cases and we stop tracking one extra temporary
reference we never removed. DisplayCompositor no longer knows about the
temporary reference, so it never gets removed.

BUG=none

Review-Url: https://codereview.chromium.org/2612873003
Cr-Commit-Position: refs/heads/master@{#441761}

services/ui/surfaces/display_compositor.cc

@@ -169,13 +169,13 @@ void DisplayCompositor::AddSurfaceReference(const cc::SurfaceReference& ref) {
   }
 
   // Remove markers for temporary references up to |child_id|, as the temporary
-  // references they correspond to were removed above. If |ref_iter| is the last
-  // element in |refs| then we are removing all temporary references for the
-  // FrameSinkId and can remove the map entry entirely.
+  // references they correspond to were removed above. If |temp_ref_iter| points
+  // at the last element in |refs| then we are removing all temporary references
+  // for the FrameSinkId and can remove the map entry entirely.
   if (++temp_ref_iter == refs.end())
     temp_references_.erase(child_id.frame_sink_id());
   else
-    refs.erase(refs.begin(), ++temp_ref_iter);
+    refs.erase(refs.begin(), temp_ref_iter);
 }
 
 void DisplayCompositor::RemoveSurfaceReference(

services/ui/surfaces/display_compositor_unittest.cc

@@ -285,5 +285,36 @@ TEST_F(DisplayCompositorTest, AddSurfacesSkipReference) {
   EXPECT_EQ(0u, CountTempReferences());
 }
 
+TEST_F(DisplayCompositorTest, RemoveFirstTempRefOnly) {
+  const cc::SurfaceId parent_id = MakeSurfaceId(1, 1, 1);
+  const cc::SurfaceId surface_id1 = MakeSurfaceId(2, 1, 1);
+  const cc::SurfaceId surface_id2 = MakeSurfaceId(2, 1, 2);
+
+  // Add two surfaces that have the same FrameSinkId. This would happen when a
+  // client submits two CFs before parent submits a new CF.
+  surface_observer()->OnSurfaceCreated(
+      cc::SurfaceInfo(surface_id1, 1.0f, gfx::Size(1, 1)));
+  surface_observer()->OnSurfaceCreated(
+      cc::SurfaceInfo(surface_id2, 1.0f, gfx::Size(1, 1)));
+  RunUntilIdle();
+
+  // Client should get OnSurfaceCreated call and temporary reference added for
+  // both surfaces.
+  EXPECT_EQ("OnSurfaceCreated(2:1:1);OnSurfaceCreated(2:1:2)",
+            client_.events());
+  EXPECT_EQ("Add(0:0:0-2:1:1);Add(0:0:0-2:1:2)", reference_manager_.events());
+  EXPECT_EQ(2u, CountTempReferences());
+
+  // Add a reference to the surface with the earlier LocalFrameId.
+  AddSurfaceReference(parent_id, surface_id1);
+  RunUntilIdle();
+
+  // The real reference should be added for 2:1:1 and temporary reference
+  // should be removed. The temporary reference for 2:1:2 should remain.
+  EXPECT_EQ("Add(1:1:1-2:1:1);Remove(0:0:0-2:1:1)",
+            reference_manager_.events());
+  EXPECT_EQ(1u, CountTempReferences());
+}
+
 }  // namespace test
 }  // namespace ui