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

Issue 878373002: Fix UnlockEndpointIsDelayed again. (Closed)

Created:
5 years, 10 months ago by Adam Rice
Modified:
5 years, 10 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, glider+watch_chromium.org, timurrrr+watch_chromium.org, bruening+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix UnlockEndpointIsDelayed again. On DrMemory and Valgrind builds, it takes more than 1ms just to post the task inside UnlockEndpoint() and then enter the message loop, so the delayed task was being executed inside RunUntilIdle(). Remove the call to RunUntilIdle() and the check that the callback wasn't called inside it. The check at 314 is sufficient to verify that at least 1ms has passed, which in normal compiles means that a delay was applied. On DrMemory and Valgrind builds, the test may just verify that everything is running very slowly, but at least it will be deterministic. BUG=451999 TEST=net_unittests Committed: https://crrev.com/ba6836202b8d60478d96fd8509ad98cd89e1ee1c Cr-Commit-Position: refs/heads/master@{#314093}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -7 lines) Patch
M net/socket/websocket_endpoint_lock_manager_unittest.cc View 2 chunks +7 lines, -2 lines 0 comments Download
M tools/valgrind/gtest_exclude/net_unittests.gtest.txt View 1 chunk +0 lines, -3 lines 0 comments Download
M tools/valgrind/gtest_exclude/net_unittests.gtest-drmemory.txt View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
Adam Rice
5 years, 10 months ago (2015-01-28 07:09:39 UTC) #2
Adam Rice
+mmenke for net/ OWNERS.
5 years, 10 months ago (2015-01-28 09:01:22 UTC) #4
Adam Rice
On 2015/01/28 09:01:22, Adam Rice wrote: > +mmenke for net/ OWNERS. Oh, sorry, tyoshino hasn't ...
5 years, 10 months ago (2015-01-28 09:10:00 UTC) #5
tyoshino (SeeGerritForStatus)
lgtm
5 years, 10 months ago (2015-01-29 09:12:19 UTC) #6
mmenke
LGTM, rubberstamp (Sorry for not getting to this earlier, took a couple days off during ...
5 years, 10 months ago (2015-01-30 15:26:16 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/878373002/1
5 years, 10 months ago (2015-02-01 17:19:22 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 10 months ago (2015-02-01 18:17:23 UTC) #10
commit-bot: I haz the power
5 years, 10 months ago (2015-02-01 18:18:26 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/ba6836202b8d60478d96fd8509ad98cd89e1ee1c
Cr-Commit-Position: refs/heads/master@{#314093}

Powered by Google App Engine
This is Rietveld 408576698