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

Issue 64493003: Ensure the wedge timer isn't started until after StartStream(). (Closed)

Created:
7 years, 1 month ago by DaleCurtis
Modified:
7 years, 1 month ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, miu+watch_chromium.org
Visibility:
Public.

Description

Ensure the wedge timer isn't started until after StartStream(). Otherwise, in rare conditions, a very long StartStream() may incorrectly cause a wedge to be flagged. If StartStream() hangs the message loop is blocked and the timer will never run anyways. Also, increase wedge timer to 5 seconds. BUG=160920 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233843

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix order. Increase timeout. #

Patch Set 3 : Remove crash. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -5 lines) Patch
M media/audio/audio_output_controller.cc View 1 2 2 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
DaleCurtis
As discussed.
7 years, 1 month ago (2013-11-07 23:04:38 UTC) #1
miu
https://codereview.chromium.org/64493003/diff/1/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/64493003/diff/1/media/audio/audio_output_controller.cc#newcode219 media/audio/audio_output_controller.cc:219: on_more_io_data_called_ = 0; Should this statement (setting on_more_io_data_called_ to ...
7 years, 1 month ago (2013-11-07 23:18:40 UTC) #2
DaleCurtis
I also updated the wedge timer to 5 seconds, since we're going to be using ...
7 years, 1 month ago (2013-11-07 23:22:54 UTC) #3
miu
lgtm
7 years, 1 month ago (2013-11-07 23:25:16 UTC) #4
scherkus (not reviewing)
lgtm
7 years, 1 month ago (2013-11-08 00:15:05 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/64493003/100002
7 years, 1 month ago (2013-11-08 00:26:11 UTC) #6
commit-bot: I haz the power
7 years, 1 month ago (2013-11-08 09:28:54 UTC) #7
Message was sent while issue was closed.
Change committed as 233843

Powered by Google App Engine
This is Rietveld 408576698