Commit a5f87e7a authored by maxmorin's avatar maxmorin Committed by Commit bot

Revert of Remove dangling PlayingState pointers in WebRtcAudioRenderer....

Revert of Remove dangling PlayingState pointers in WebRtcAudioRenderer. (patchset #4 id:60001 of https://codereview.chromium.org/2758453004/ )

Reason for revert:
This causes a memory use increase across the board on Android, even when WebRTC isn't used.

Original issue's description:
> Remove dangling PlayingState pointers in WebRtcAudioRenderer.
>
> This solution is a bit of a hack, but it works. Did manual testing and ran
> unit tests. Bug 697256 does not reproduce with this patch.
>
> R=tommi@chromium.org
>
> BUG=697256
>
> Review-Url: https://codereview.chromium.org/2758453004
> Cr-Commit-Position: refs/heads/master@{#458025}
> Committed: https://chromium.googlesource.com/chromium/src/+/ba21ffd704aebcc8a5e5e1ab1f072e9b79b51fcb

TBR=tommi@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=697256,703743

Review-Url: https://codereview.chromium.org/2856943003
Cr-Commit-Position: refs/heads/master@{#468984}
parent da7c2682
...@@ -54,24 +54,17 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer { ...@@ -54,24 +54,17 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
public: public:
// Callback definition for a callback that is called when when Play(), Pause() // Callback definition for a callback that is called when when Play(), Pause()
// or SetVolume are called (whenever the internal |playing_state_| changes). // or SetVolume are called (whenever the internal |playing_state_| changes).
using OnPlayStateChanged = typedef base::Callback<void(const blink::WebMediaStream&,
base::Callback<void(const blink::WebMediaStream&, WebRtcAudioRenderer::PlayingState*)>
WebRtcAudioRenderer::PlayingState*)>; OnPlayStateChanged;
// Signals that the PlayingState* is about to become invalid, see comment in
// OnPlayStateRemoved.
using OnPlayStateRemoved =
base::OnceCallback<void(WebRtcAudioRenderer::PlayingState*)>;
SharedAudioRenderer(const scoped_refptr<MediaStreamAudioRenderer>& delegate, SharedAudioRenderer(const scoped_refptr<MediaStreamAudioRenderer>& delegate,
const blink::WebMediaStream& media_stream, const blink::WebMediaStream& media_stream,
const OnPlayStateChanged& on_play_state_changed, const OnPlayStateChanged& on_play_state_changed)
OnPlayStateRemoved on_play_state_removed)
: delegate_(delegate), : delegate_(delegate),
media_stream_(media_stream), media_stream_(media_stream),
started_(false), started_(false),
on_play_state_changed_(on_play_state_changed), on_play_state_changed_(on_play_state_changed) {
on_play_state_removed_(std::move(on_play_state_removed)) {
DCHECK(!on_play_state_changed_.is_null()); DCHECK(!on_play_state_changed_.is_null());
DCHECK(!media_stream_.IsNull()); DCHECK(!media_stream_.IsNull());
} }
...@@ -81,7 +74,6 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer { ...@@ -81,7 +74,6 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
DVLOG(1) << __func__; DVLOG(1) << __func__;
Stop(); Stop();
std::move(on_play_state_removed_).Run(&playing_state_);
} }
void Start() override { void Start() override {
...@@ -156,7 +148,6 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer { ...@@ -156,7 +148,6 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
bool started_; bool started_;
WebRtcAudioRenderer::PlayingState playing_state_; WebRtcAudioRenderer::PlayingState playing_state_;
OnPlayStateChanged on_play_state_changed_; OnPlayStateChanged on_play_state_changed_;
OnPlayStateRemoved on_play_state_removed_;
}; };
} // namespace } // namespace
...@@ -229,12 +220,9 @@ bool WebRtcAudioRenderer::Initialize(WebRtcAudioRendererSource* source) { ...@@ -229,12 +220,9 @@ bool WebRtcAudioRenderer::Initialize(WebRtcAudioRendererSource* source) {
scoped_refptr<MediaStreamAudioRenderer> scoped_refptr<MediaStreamAudioRenderer>
WebRtcAudioRenderer::CreateSharedAudioRendererProxy( WebRtcAudioRenderer::CreateSharedAudioRendererProxy(
const blink::WebMediaStream& media_stream) { const blink::WebMediaStream& media_stream) {
SharedAudioRenderer::OnPlayStateChanged on_play_state_changed = content::SharedAudioRenderer::OnPlayStateChanged on_play_state_changed =
base::Bind(&WebRtcAudioRenderer::OnPlayStateChanged, this); base::Bind(&WebRtcAudioRenderer::OnPlayStateChanged, this);
SharedAudioRenderer::OnPlayStateRemoved on_play_state_removed = return new SharedAudioRenderer(this, media_stream, on_play_state_changed);
base::BindOnce(&WebRtcAudioRenderer::OnPlayStateRemoved, this);
return new SharedAudioRenderer(this, media_stream, on_play_state_changed,
std::move(on_play_state_removed));
} }
bool WebRtcAudioRenderer::IsStarted() const { bool WebRtcAudioRenderer::IsStarted() const {
...@@ -605,27 +593,6 @@ void WebRtcAudioRenderer::OnPlayStateChanged( ...@@ -605,27 +593,6 @@ void WebRtcAudioRenderer::OnPlayStateChanged(
} }
} }
void WebRtcAudioRenderer::OnPlayStateRemoved(PlayingState* state) {
// It is possible we associated |state| to a source for a track that is no
// longer easily reachable. We iterate over |source_playing_states_| to
// ensure there are no dangling pointers to |state| there. See
// crbug.com/697256.
// TODO(maxmorin): Clean up cleanup code in this and related classes so that
// this hack isn't necessary.
DCHECK(thread_checker_.CalledOnValidThread());
for (auto it = source_playing_states_.begin();
it != source_playing_states_.end();) {
PlayingStates& states = it->second;
// We cannot use RemovePlayingState as it might invalidate |it|.
states.erase(std::remove(states.begin(), states.end(), state),
states.end());
if (states.empty())
it = source_playing_states_.erase(it);
else
++it;
}
}
void WebRtcAudioRenderer::PrepareSink() { void WebRtcAudioRenderer::PrepareSink() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
media::AudioParameters new_sink_params; media::AudioParameters new_sink_params;
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <stdint.h> #include <stdint.h>
#include <map> #include <map>
#include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
...@@ -194,9 +193,6 @@ class CONTENT_EXPORT WebRtcAudioRenderer ...@@ -194,9 +193,6 @@ class CONTENT_EXPORT WebRtcAudioRenderer
void OnPlayStateChanged(const blink::WebMediaStream& media_stream, void OnPlayStateChanged(const blink::WebMediaStream& media_stream,
PlayingState* state); PlayingState* state);
// Called when |state| is about to be destructed.
void OnPlayStateRemoved(PlayingState* state);
// Updates |sink_params_| and |audio_fifo_| based on |sink_|, and initializes // Updates |sink_params_| and |audio_fifo_| based on |sink_|, and initializes
// |sink_|. // |sink_|.
void PrepareSink(); void PrepareSink();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment