Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(36)

Issue 640253002: Speeding up TwoGetUserMediaAndStop WebRTC test (Closed)

Created:
6 years, 2 months ago by phoglund_chromium
Modified:
6 years, 2 months ago
Reviewers:
perkj_chrome
CC:
chromium-reviews, darin-cc_chromium.org, jam, feature-media-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

As per the investigation in http://crbug.com/417756, it seems the Two* tests are susceptible to timeouts on GPU-less, slow bots. This patch will make the test no longer detect that video is up for both streams before proceeding - we assume other tests check that. Instead, we're happy with just getting a stream before proceeding. This removes the costly video checks from this test so it can test what it wants to test, which is to ensure we can stop streams in an orderly fashion. BUG=417756 R=perkj@chromium.org Committed: https://crrev.com/8fb48317ec49417cf3257af945c6f3ada5c47311 Cr-Commit-Position: refs/heads/master@{#298884}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Made test even faster. #

Patch Set 3 : Reverted gUM changes since that didn't help #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : Fixed formatting #

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -27 lines) Patch
M content/test/data/media/getusermedia.html View 1 2 3 4 5 6 7 chunks +20 lines, -24 lines 0 comments Download
M content/test/data/media/webrtc_test_utilities.js View 2 chunks +5 lines, -3 lines 0 comments Download

Messages

Total messages: 9 (1 generated)
phoglund_chromium
https://codereview.chromium.org/640253002/diff/1/content/test/data/media/getusermedia.html File content/test/data/media/getusermedia.html (right): https://codereview.chromium.org/640253002/diff/1/content/test/data/media/getusermedia.html#newcode169 content/test/data/media/getusermedia.html:169: var stream1 = null; Unless of course you see ...
6 years, 2 months ago (2014-10-09 12:20:07 UTC) #1
phoglund_chromium
Actually, never mind, this actually makes the tests slower on Windows(!). I really don't understand ...
6 years, 2 months ago (2014-10-09 13:07:46 UTC) #2
phoglund_chromium
On 2014/10/09 13:07:46, phoglund wrote: > Actually, never mind, this actually makes the tests slower ...
6 years, 2 months ago (2014-10-09 13:27:03 UTC) #3
phoglund_chromium
> The aspect ratio detection is way too slow - those tests take upwards of ...
6 years, 2 months ago (2014-10-09 13:59:27 UTC) #4
perkj_chrome
lgtm
6 years, 2 months ago (2014-10-09 14:23:03 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/640253002/120001
6 years, 2 months ago (2014-10-09 15:04:36 UTC) #7
commit-bot: I haz the power
Committed patchset #7 (id:120001) as 8b0f306f10f06763a9875819fe84a0a21a5d18d8
6 years, 2 months ago (2014-10-09 16:07:06 UTC) #8
commit-bot: I haz the power
6 years, 2 months ago (2014-10-09 16:08:22 UTC) #9
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/8fb48317ec49417cf3257af945c6f3ada5c47311
Cr-Commit-Position: refs/heads/master@{#298884}

Powered by Google App Engine
This is Rietveld 408576698