Is this something you can look at Tommy or should we reassign? Either the test is broken or there's a bug.

Reassigning to Khushal

I'll take a look at this.

Ran it with a 200 repeat count locally, the test didn't fail. Is there a bot where this is flaky? May be I need to use some specific build flags?

It happened to me when running cc_unittests (the whole suite) locally. I think it was a release build with dchecks on fwiw.

Usually running a test individually doesn't show flake well unless you get the whole system under load. I'd try the whole suite.

Ugh, so I ran the test suite a 100 times locally with these build flags, didn't see it break.

From the failure there it looks like the commit count is 1 when DidInitializeOutputSurface is called (https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/remote_channel_unittest.cc&q=remote_channel_unittest.cc&l=202). And the output surface was initialized thrice. The test assumes that the output surface will be initialized only twice, once when the LayerTreeHost is created and second after it releases the output surface and asks the client for a new one. Is this a valid assumption to make in the LayerTreeTests?

 Issue 606964  has been merged into this issue.

Just happened again, while running the whole test suite (with an asan build fwiw).

It now has a bunch of NOTIMPLEMENTED spam also.

[ RUN      ] RemoteChannelTestReleaseOutputSurfaceDuringCommit.RunRemote_DirectRenderer
[3066:3066:0602/143304:8550665921239:ERROR:remote_channel_main.cc(108)] Not implemented reached in virtual void cc::RemoteChannelMain::SetVisibleOnImpl(bool)Visibility is not controlled by the server
[3066:3066:0602/143304:8550665925650:ERROR:remote_channel_main.cc(108)] Not implemented reached in virtual void cc::RemoteChannelMain::SetVisibleOnImpl(bool)Visibility is not controlled by the server
[3066:3066:0602/143304:8550665927342:ERROR:remote_channel_main.cc(108)] Not implemented reached in virtual void cc::RemoteChannelMain::SetVisibleOnImpl(bool)Visibility is not controlled by the server
[3066:3066:0602/143304:8550665935923:ERROR:remote_channel_main.cc(108)] Not implemented reached in virtual void cc::RemoteChannelMain::SetVisibleOnImpl(bool)Visibility is not controlled by the server
[3066:3066:0602/143304:8550665937247:ERROR:remote_channel_main.cc(108)] Not implemented reached in virtual void cc::RemoteChannelMain::SetVisibleOnImpl(bool)Visibility is not controlled by the server
../../cc/trees/remote_channel_unittest.cc:214: Failure
Value of: commit_count_
  Actual: 1
Expected: 0
../../cc/trees/remote_channel_unittest.cc:234: Failure
Value of: output_surface_initialized_count_
  Actual: 3
Expected: 2
[  FAILED  ] RemoteChannelTestReleaseOutputSurfaceDuringCommit.RunRemote_DirectRenderer (21 ms)

The Not implemented stuff is expected, not the cause of the bug. I've got a fix up here: https://codereview.chromium.org/2042433002/

>  Is this a valid assumption to make in the LayerTreeTests?

Oh I didn't reply, ya that should be. But I guess ReleaseOutputSurfaceOnLayerTreeHost() can happen more than once. Maybe the test usually ends before the 2nd one takes place or something. I've seen flake of that sort in other ways before.

I think I figured it out. I was making an assumption that we would receive only 1 BeginMainFrame and that's not necessarily true. So each time we would receive a BeginMainFrame on the server, we would release the output surface on the client before responding, which is why the multiple output surface initializations. The patch above should fix that.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2acaecbc0509795e5e2c55dcb39bd22f40da0cc

commit d2acaecbc0509795e5e2c55dcb39bd22f40da0cc
Author: khushalsagar <khushalsagar@chromium.org>
Date: Thu Jun 09 21:30:13 2016

cc: Fix flaky RemoteChannelTest.ReleaseOutputSurfaceDuringCommit.

The test flakes because it expects only 1 BeginMainFrame to be sent from
the Scheduler and sets the expectations for commit count and output
surface initializations accordingly.

Update the test to perform the following:

1) When the first BeginMainFrame request is sent to the server, release
the output surface on the client. It would earlier do it for each
BeginMainFrame request, do it only for the first one.

2) Make sure that the first commit doesn't go through on the client
after the output surface is released till a new output surface is
initialized.

BUG= 612562 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/d2acaecbc0509795e5e2c55dcb39bd22f40da0cc/cc/trees/remote_channel_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2acaecbc0509795e5e2c55dcb39bd22f40da0cc

commit d2acaecbc0509795e5e2c55dcb39bd22f40da0cc
Author: khushalsagar <khushalsagar@chromium.org>
Date: Thu Jun 09 21:30:13 2016

cc: Fix flaky RemoteChannelTest.ReleaseOutputSurfaceDuringCommit.

The test flakes because it expects only 1 BeginMainFrame to be sent from
the Scheduler and sets the expectations for commit count and output
surface initializations accordingly.

Update the test to perform the following:

1) When the first BeginMainFrame request is sent to the server, release
the output surface on the client. It would earlier do it for each
BeginMainFrame request, do it only for the first one.

2) Make sure that the first commit doesn't go through on the client
after the output surface is released till a new output surface is
initialized.

BUG= 612562 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/d2acaecbc0509795e5e2c55dcb39bd22f40da0cc/cc/trees/remote_channel_unittest.cc