New issue
Advanced search Search tips

Issue 783790 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WebRTWebRtcAudioDebugRecordingsBrowserTest failing on WebRTC windows bots

Project Member Reported by guidou@chromium.org, Nov 10 2017

Issue description

Sample logs:

../../content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc(150): error: Value of: base::DeleteFile(file_path, false)
  Actual: false
Expected: true
../../content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc(174): error: Value of: base::IsDirectoryEmpty(temp_dir_path)
  Actual: false
Expected: true
../../content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc(175): error: Value of: base::DeleteFile(temp_dir_path, false)
  Actual: false
Expected: true


The the files are not deleted in the test could be that all the side effects of MediaStreamTrack.stop() are not complete yet at deletion time and this somehow prevents the files to be deleted.

Unfortunately MediaStreamTrack.stop() does not have a callback to get a notification of when all its side effects are complete, so maybe the fix is to just wait some time before trying to delete the files.
 

Comment 1 by guidou@chromium.org, Nov 10 2017

Description: Show this description

Comment 2 by guidou@chromium.org, Nov 10 2017

Cc: grunell@chromium.org
Owner: grunell@chromium.org
Status: Assigned (was: Untriaged)
Cc: guidou@chromium.org
Guido: which bot(s) was this? Is the test disabled now? And this is a regression, right? Which build did it start with?

Comment 5 by guidou@chromium.org, Nov 24 2017

All WebRTC windows bots.
The problem is that MediaStreamTrack.stop()'s implementation is asynchronous even though its API suggests it's not (it has no callbacks/promises).
This was causing some races with getUserMedia(), which were fixed by serializing stop() requests together with getUserMedia() to make sure that side effects from ech stop() call are complete before processing the next stop() or getUserMedia() request.
I believe the extra delays introduced by this serialization made this test fail on Windows WebRTC bots.
I made some fixes to minimize the delay in most cases, so it might be worth it to just re-enable the test on Windows to see if it starts passing.
Otherwise, just sleeping a little bit after all the stop calls should fix it too.
Thanks for the explanation! OK, I'll see what can be done.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 5 2017

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

commit 150d636613446a107edee5809a738f17ca80e9a4
Author: Henrik Grunell <grunell@chromium.org>
Date: Tue Dec 05 14:30:10 2017

Re-enable WebRtcAudioDebugRecordingsBrowserTest.CallWithAudioDebugRecordings on Windows.

Speculatively re-enable based on comment #5 in the bug.

TBR=guidou@chromium.org

Bug:  783790 
Change-Id: Ie10aa49212a4279a8696dfbc021a577bda0a2dfd
Reviewed-on: https://chromium-review.googlesource.com/808845
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521686}
[modify] https://crrev.com/150d636613446a107edee5809a738f17ca80e9a4/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc

Still flaky on Windows after re-enabling: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_browsertests&tests=WebRtcAudioDebugRecordingsBrowserTest.CallWithAudioDebugRecordings

Though, in all runs so far the retries have passed. Simplest solution would be a retry in the test, with a little delay. That's probably good enough. Alternatively, the test could listen to some internal state but afaik (haven't looked yet) that requires code changes for this purpose only which is to be avoided.
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 7 2017

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

commit 4b3b8130b5df559b562a43b28298089f8b9bf15d
Author: Henrik Grunell <grunell@chromium.org>
Date: Thu Dec 07 16:24:43 2017

Move AEC dump verification to last in WebRTWebRtcAudioDebugRecordingsBrowserTest tests.

An attempt to reduce flakiness, based on that the AEC dump requires longer time before being closed than the input and output recordings due to IPC and more thread jumps. See also the added comment in the file for more info.

Bug:  783790 
Change-Id: I1f2cd838a749eb1ec76e6e32e283b26e89ff293e
Reviewed-on: https://chromium-review.googlesource.com/813715
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522445}
[modify] https://crrev.com/4b3b8130b5df559b562a43b28298089f8b9bf15d/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc

Status: Started (was: Assigned)
Nah, #9 didn't help. So, I'll introduce a retry after a little delay if delete file fails. The flakiness affects the other test cases too, so the retry should be in all of them.
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 9 2018

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

commit 2cced9918b45eba08a8035cdbefba96e754e8b11
Author: Henrik Grunell <grunell@chromium.org>
Date: Tue Jan 09 10:40:19 2018

Retry deleting file in WebRtcAudioDebugRecordingsBrowserTest.

This is a simple way to mitigate flaky failure due to racy file close vs. delete.

Bug:  783790 
Change-Id: I01a71399b16fd392c5509f851cc1f4ec5b21f750
Reviewed-on: https://chromium-review.googlesource.com/832465
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527957}
[modify] https://crrev.com/2cced9918b45eba08a8035cdbefba96e754e8b11/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc

Will check flakiness dashboard in a few days. For reference, it still is flaky.
Status: Fixed (was: Started)
Looks good now.

Sign in to add a comment