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

Issue 10795055: Fix 2 threading-related bugs in pyauto's remote inspector module. (Closed)

Created:
8 years, 5 months ago by dennis_jeffrey
Modified:
8 years, 5 months ago
Reviewers:
Nirnimesh
CC:
chromium-reviews, dennis_jeffrey, anantha, dyu1, Nirnimesh, fdeng1, marja
Visibility:
Public.

Description

Fix 2 threading-related bugs in pyauto's remote inspector module. The first problem was a small typo where the wrong variable was being used (a boolean value was being used where a threading.Condition object should have been used). The second problem was in the API functions StartTimelineEventMonitoring and StopTimelineEventMonitoring. Previously, these two functions would send a request to tell the remote inspector to start/stop timeline mode, but they did not wait until the requests were serviced before returning. This sometimes led to subtle multithreading problems where the remote inspector module would hang. The fix is to ensure these two API functions wait until their requests are serviced before returning. BUG=None TEST=Ran a Chrome Endure test for 5 seconds with --repeat=100. Previously, we'd see a hang within the first 10 runs or so. Now, all 100 runs pass. NOTRY=true Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=147859

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -6 lines) Patch
M chrome/test/pyautolib/remote_inspector_client.py View 4 chunks +32 lines, -6 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
dennis_jeffrey
8 years, 5 months ago (2012-07-20 22:35:25 UTC) #1
Nirnimesh
LGTM
8 years, 5 months ago (2012-07-23 07:10:01 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dennisjeffrey@chromium.org/10795055/1
8 years, 5 months ago (2012-07-23 16:20:26 UTC) #3
commit-bot: I haz the power
8 years, 5 months ago (2012-07-23 16:20:55 UTC) #4
Change committed as 147859

Powered by Google App Engine
This is Rietveld 408576698