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

Issue 8385001: Stop audio streams in all codepaths that need it. (Closed)

Created:
9 years, 2 months ago by Ami GONE FROM CHROMIUM
Modified:
9 years, 2 months ago
Reviewers:
enal1, enal
CC:
chromium-reviews, hclam+watch_chromium.org, ddorwin+watch_chromium.org, fischman+watch_chromium.org, acolwell+watch_chromium.org, annacc+watch_chromium.org, ajwong+watch_chromium.org, vrk (LEFT CHROMIUM), scherkus (not reviewing), ihf+watch_chromium.org
Visibility:
Public.

Description

Stop audio streams in all codepaths that need it. Before we were leaving dangling streams un-Stop()'d, causing delayed messages to eventually crash the browser when their source_callback_ was followed but already invalid. BUG=101228 TEST=trybots, manual testing. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107000

Patch Set 1 #

Patch Set 2 : enal CR response. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -11 lines) Patch
M media/audio/audio_output_controller.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M media/audio/audio_output_controller.cc View 1 6 chunks +17 lines, -11 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Ami GONE FROM CHROMIUM
9 years, 2 months ago (2011-10-24 19:44:39 UTC) #1
enal1
LGTM Can you move if (stream) { Stop(), Close(), stream = NULL } into separate ...
9 years, 2 months ago (2011-10-24 19:54:13 UTC) #2
Ami GONE FROM CHROMIUM
> Can you move if (stream) { Stop(), Close(), stream = NULL } into separate ...
9 years, 2 months ago (2011-10-24 22:31:29 UTC) #3
enal1
9 years, 2 months ago (2011-10-24 22:49:39 UTC) #4
LGTM once again.

Powered by Google App Engine
This is Rietveld 408576698