Do not remove entry from |frame_sink_source_map_| when client is unregistered.

Currently, FrameSinkManager::UnregisterFrameSinkManagerClient removes
the entry associated with |frame_sink_id| from |frame_sink_source_map_|
if the |frame_sink_id| does not have any children. This would result in
a client not connected with a BeginFrameSource when it is unregistered
and re-registered with the same FrameSinkId.

This CL removes the lines that removes the entry from |frame_sink_source_map_|
if the |frame_sink_id| has no child in
FrameSinkManager::UnregisterFrameSinkManagerClient. It also adds a unit
test to verify that the client gets reconnected to the BeginFrameSource
after unregistering and re-registering.

Bug: 735805
Change-Id: I93ae860b1192e23aa116749d0bbcdb553ddc2246
Reviewed-on: https://chromium-review.googlesource.com/563705
Commit-Queue: Xingyu Zhang <staraz@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485172}

cc/surfaces/frame_sink_manager.cc

@@ -67,8 +67,6 @@ void FrameSinkManager::UnregisterFrameSinkManagerClient(
   if (source_iter != frame_sink_source_map_.end()) {
     if (source_iter->second.source)
       client_iter->second->SetBeginFrameSource(nullptr);
-    if (!source_iter->second.has_children())
-      frame_sink_source_map_.erase(source_iter);
   }
   clients_.erase(client_iter);
 }

cc/surfaces/frame_sink_manager_unittest.cc

@@ -13,6 +13,11 @@
 
 namespace cc {
 
+namespace {
+
+constexpr FrameSinkId kArbitraryFrameSinkId(1, 1);
+}
+
 class FakeFrameSinkManagerClient : public FrameSinkManagerClient {
  public:
   explicit FakeFrameSinkManagerClient(const FrameSinkId& frame_sink_id)
@@ -115,6 +120,28 @@ TEST_F(FrameSinkManagerTest, SingleClients) {
   EXPECT_EQ(nullptr, client.source());
 }
 
+// This test verifies that a client is still connected to the BeginFrameSource
+// after restart.
+TEST_F(FrameSinkManagerTest, ClientRestart) {
+  FakeFrameSinkManagerClient client(kArbitraryFrameSinkId);
+  StubBeginFrameSource source;
+  client.Register(&manager_);
+
+  manager_.RegisterBeginFrameSource(&source, kArbitraryFrameSinkId);
+  EXPECT_EQ(&source, client.source());
+
+  // |client| is disconnect from |source| after Unregister.
+  client.Unregister();
+  EXPECT_EQ(nullptr, client.source());
+
+  // |client| is reconnected with |source| after re-Register.
+  client.Register(&manager_);
+  EXPECT_EQ(&source, client.source());
+
+  manager_.UnregisterBeginFrameSource(&source);
+  EXPECT_EQ(nullptr, client.source());
+}
+
 // This test verifies that a PrimaryBeginFrameSource will receive BeginFrames
 // from the first BeginFrameSource registered. If that BeginFrameSource goes
 // away then it will receive BeginFrames from the second BeginFrameSource.