Layout Test MediaStreamTrack-gc-no-crash.html flaky when using video capture service |
||
Issue descriptionSee reverted CL which activates the video capture service by default: https://chromium-review.googlesource.com/c/chromium/src/+/719416 After the CL landed, the test failed 3 times in a row starting at build https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/12301 The 4th time it passed, which was in build https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/12304 With the next build the CL was reverted, and it looks like the test always passed since. There might be a (possibly existing) race condition that is typically never hit, but gets hit more often with some timing changes when the new service is activated.
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1702c9095ed676cd89f6bac4c1e522dd9a32e3dc commit 1702c9095ed676cd89f6bac4c1e522dd9a32e3dc Author: Christian Fremerey <chfremer@chromium.org> Date: Tue Dec 05 16:59:04 2017 Make test MediaStreamTrack-gc-no-crash deterministic The Layout Test MediaStreamTrack-gc-no-crash.html was non-determinstic in that it depend on a race between the JavaScript side declaring the asynchronous test "finished" and the callback to GetUserMedia arriving. It appears there were at least 3 possible outcomes. The first case was that the finishJSTest() would be invoked before the call to GetUserMedia would do anything much, and the test would essentially verify nothing. The second case was that when finishJSTest() was invoked, GetUserMedia would have reach a point where it would have temporarily taken shared ownership of the blink document but the callbacks would not yet have been invoked. The third case would be that finishJSTest() would be invoked after the callback to GetUserMedia had finished running. It looks like in practice, at least on my local machine, the first case was the only one reached so far. When trying to activate the new video capture service with [1], the timing changed so that the second case would typically be reached. This case would lead to the leak detector running at the end of the test reporting leaked blink objects and failing the test. I verified that the same would happen without activating the video capture service, simply by making the call to FakeVideoCaptureDeviceFactory::GetDescriptors() taking a bit longer, via PlatformThread::Sleep(). After this CL, the test should deterministically always reach the third case. [1] https://chromium-review.googlesource.com/c/chromium/src/+/719416 Bug: 789669 Test: third_party/WebKit/Tools/Scripts/run-webkit-tests --target gn_release --enable-leak-detection fast/mediastream/MediaStreamTrack-gc-no-crash.html Change-Id: I17c39e66cfd8c4af89f8f9b078d6512feaedbe6d Reviewed-on: https://chromium-review.googlesource.com/806442 Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org> Commit-Queue: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#521723} [modify] https://crrev.com/1702c9095ed676cd89f6bac4c1e522dd9a32e3dc/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-gc-no-crash-expected.txt [modify] https://crrev.com/1702c9095ed676cd89f6bac4c1e522dd9a32e3dc/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-gc-no-crash.html
,
Dec 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0c5b69e33aaf67089ff8dc584bac670b0168c62 commit f0c5b69e33aaf67089ff8dc584bac670b0168c62 Author: Christian Fremerey <chfremer@chromium.org> Date: Wed Dec 06 21:48:43 2017 Make blink layout-test argumen-types.html deterministic This is a second instance of the same issue and fix as reviewed in https://chromium-review.googlesource.com/c/chromium/src/+/806442. TBR=emircan@chromium.org,hajimehoshi@chromium.org Bug: 789669 Test: third_party/WebKit/Tools/Scripts/run-webkit-tests --target gn_release --enable-leak-detection fast/mediastream/argument-types.html Change-Id: If16c19395209a19a4548491f8b3e04893915adc2 Reviewed-on: https://chromium-review.googlesource.com/811865 Reviewed-by: Christian Fremerey <chfremer@chromium.org> Reviewed-by: Emircan Uysaler <emircan@chromium.org> Commit-Queue: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#522206} [modify] https://crrev.com/f0c5b69e33aaf67089ff8dc584bac670b0168c62/third_party/WebKit/LayoutTests/fast/mediastream/argument-types-expected.txt [modify] https://crrev.com/f0c5b69e33aaf67089ff8dc584bac670b0168c62/third_party/WebKit/LayoutTests/fast/mediastream/argument-types.html
,
Dec 7 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by chfremer@chromium.org
, Nov 30 2017