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

Side by Side Diff: content/browser/media/audio_stream_monitor.cc

Issue 2698813007: Fix teardown of stale AudioStreamMonitor poll callbacks. (Closed)
Patch Set: Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/browser/media/audio_stream_monitor.h" 5 #include "content/browser/media/audio_stream_monitor.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/bind_helpers.h" 8 #include "base/bind_helpers.h"
9 #include "content/browser/web_contents/web_contents_impl.h" 9 #include "content/browser/web_contents/web_contents_impl.h"
10 #include "content/public/browser/browser_thread.h" 10 #include "content/public/browser/browser_thread.h"
11 #include "content/public/browser/invalidate_type.h" 11 #include "content/public/browser/invalidate_type.h"
12 #include "content/public/browser/render_frame_host.h" 12 #include "content/public/browser/render_frame_host.h"
13 13
14 namespace content { 14 namespace content {
15 15
16 namespace { 16 namespace {
17 17
18 enum class ActionType { STARTING, STOPPING }; 18 enum class ActionType { STARTING, STOPPING };
19 AudioStreamMonitor* StartStopMonitoringHelper(ActionType action_type, 19 AudioStreamMonitor* StartStopMonitoringHelper(ActionType action_type,
20 int render_process_id, 20 int render_process_id,
21 int render_frame_id) { 21 int render_frame_id) {
22 DCHECK_CURRENTLY_ON(BrowserThread::UI); 22 DCHECK_CURRENTLY_ON(BrowserThread::UI);
23 23
24 // It's important that this code uses only the process id for lookup as there 24 // It's important that this code uses only the process id for lookup as there
25 // may not be a RenderFrameHost or WebContents attached to the RenderProcess 25 // may not be a RenderFrameHost or WebContents attached to the RenderProcess
26 // at time of call; e.g., in the event of a crash. 26 // at time of call; e.g., in the event of a crash.
Charlie Reis 2017/02/17 21:27:55 Looks like this was already partly-noted here? :)
DaleCurtis 2017/02/17 22:03:59 Yes, but when written I had assumed only the Rende
27 RenderProcessHost* const render_process_host = 27 RenderProcessHost* const render_process_host =
28 RenderProcessHost::FromID(render_process_id); 28 RenderProcessHost::FromID(render_process_id);
29 if (!render_process_host) 29 if (!render_process_host)
30 return nullptr; 30 return nullptr;
31 31
32 // TODO(dalecurtis, maxmorin): We should really only be sending these when the 32 // TODO(dalecurtis, maxmorin): We should really only be sending these when the
33 // streams are audible or we don't have power level monitoring. 33 // streams are audible or we don't have power level monitoring.
34 if (action_type == ActionType::STARTING) 34 if (action_type == ActionType::STARTING)
35 render_process_host->OnAudioStreamAdded(); 35 render_process_host->OnAudioStreamAdded();
36 else 36 else
(...skipping 21 matching lines...) Expand all
58 bool AudioStreamMonitor::WasRecentlyAudible() const { 58 bool AudioStreamMonitor::WasRecentlyAudible() const {
59 DCHECK(thread_checker_.CalledOnValidThread()); 59 DCHECK(thread_checker_.CalledOnValidThread());
60 return was_recently_audible_; 60 return was_recently_audible_;
61 } 61 }
62 62
63 bool AudioStreamMonitor::IsCurrentlyAudible() const { 63 bool AudioStreamMonitor::IsCurrentlyAudible() const {
64 DCHECK(thread_checker_.CalledOnValidThread()); 64 DCHECK(thread_checker_.CalledOnValidThread());
65 return is_audible_; 65 return is_audible_;
66 } 66 }
67 67
68 void AudioStreamMonitor::RenderProcessGone(int render_process_id) {
69 DCHECK(thread_checker_.CalledOnValidThread());
70
71 // Note: It's possible for the RenderProcessHost and WebContents (and thus
72 // this class) to survive the death of the render process and subsequently be
73 // reused. During this period StartStopMonitoringHelper() will be unable to
74 // lookup the WebContents using the now-dead |render_frame_id|. We must thus
75 // have this secondary mechanism for clearing stale callbacks.
76
77 for (auto it = poll_callbacks_.begin(); it != poll_callbacks_.end();) {
78 if (it->first.first == render_process_id) {
79 it = poll_callbacks_.erase(it);
80 if (!power_level_monitoring_available())
81 OnStreamRemoved();
82 } else {
83 ++it;
84 }
85 }
86
87 if (poll_callbacks_.empty())
88 poll_timer_.Stop();
89 }
90
68 // static 91 // static
69 void AudioStreamMonitor::StartMonitoringStream( 92 void AudioStreamMonitor::StartMonitoringStream(
70 int render_process_id, 93 int render_process_id,
71 int render_frame_id, 94 int render_frame_id,
72 int stream_id, 95 int stream_id,
73 const ReadPowerAndClipCallback& read_power_callback) { 96 const ReadPowerAndClipCallback& read_power_callback) {
74 BrowserThread::PostTask(BrowserThread::UI, 97 BrowserThread::PostTask(BrowserThread::UI,
75 FROM_HERE, 98 FROM_HERE,
76 base::Bind(&StartMonitoringHelper, 99 base::Bind(&StartMonitoringHelper,
77 render_process_id, 100 render_process_id,
(...skipping 62 matching lines...) Expand 10 before | Expand all | Expand 10 after
140 FROM_HERE, base::TimeDelta::FromSeconds(1) / 163 FROM_HERE, base::TimeDelta::FromSeconds(1) /
141 static_cast<int>(kPowerMeasurementsPerSecond), 164 static_cast<int>(kPowerMeasurementsPerSecond),
142 base::Bind(&AudioStreamMonitor::Poll, base::Unretained(this))); 165 base::Bind(&AudioStreamMonitor::Poll, base::Unretained(this)));
143 } 166 }
144 } 167 }
145 168
146 void AudioStreamMonitor::StopMonitoringStreamOnUIThread(int render_process_id, 169 void AudioStreamMonitor::StopMonitoringStreamOnUIThread(int render_process_id,
147 int stream_id) { 170 int stream_id) {
148 DCHECK(thread_checker_.CalledOnValidThread()); 171 DCHECK(thread_checker_.CalledOnValidThread());
149 172
150 const StreamID qualified_id(render_process_id, stream_id); 173 // In the event of render process death, these may have already been cleared.
151 DCHECK(poll_callbacks_.find(qualified_id) != poll_callbacks_.end()); 174 auto it = poll_callbacks_.find(StreamID(render_process_id, stream_id));
152 poll_callbacks_.erase(qualified_id); 175 if (it == poll_callbacks_.end())
176 return;
153 177
178 poll_callbacks_.erase(it);
154 if (poll_callbacks_.empty()) 179 if (poll_callbacks_.empty())
155 poll_timer_.Stop(); 180 poll_timer_.Stop();
156 } 181 }
157 182
158 void AudioStreamMonitor::Poll() { 183 void AudioStreamMonitor::Poll() {
159 bool was_audible = is_audible_; 184 bool was_audible = is_audible_;
160 is_audible_ = false; 185 is_audible_ = false;
161 186
162 for (StreamPollCallbackMap::const_iterator it = poll_callbacks_.begin(); 187 for (StreamPollCallbackMap::const_iterator it = poll_callbacks_.begin();
163 it != poll_callbacks_.end(); 188 it != poll_callbacks_.end();
(...skipping 53 matching lines...) Expand 10 before | Expand all | Expand 10 after
217 DCHECK(!power_level_monitoring_available()); 242 DCHECK(!power_level_monitoring_available());
218 DCHECK_GT(active_streams_, 0u); 243 DCHECK_GT(active_streams_, 0u);
219 if (--active_streams_ == 0u) { 244 if (--active_streams_ == 0u) {
220 is_audible_ = false; 245 is_audible_ = false;
221 web_contents_->OnAudioStateChanged(false); 246 web_contents_->OnAudioStateChanged(false);
222 MaybeToggle(); 247 MaybeToggle();
223 } 248 }
224 } 249 }
225 250
226 } // namespace content 251 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698