Simplify the AudioScheduler implementation.

- Merge OnClientDisconnected() into Stop(), since each AudioScheduler now supports only a single ClientSession.
- Remove unnecessary thread-hopping in the implementation of Stop().
- Guarantee that no more packets will reach AudioStub after Stop() returns.


Review URL: https://chromiumcodereview.appspot.com/11232027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163465 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 42 insertions, 61 deletions

diff --git a/remoting/host/audio_scheduler.cc b/remoting/host/audio_scheduler.cc
index c5e3ef3..02cc989 100644
--- a/remoting/host/audio_scheduler.cc
+++ b/remoting/host/audio_scheduler.cc
@@ -29,62 +29,43 @@ AudioScheduler::AudioScheduler(
       audio_stub_(audio_stub),
       network_stopped_(false),
       enabled_(true) {
-  DCHECK(audio_task_runner_);
-  DCHECK(network_task_runner_);
+  DCHECK(network_task_runner_->BelongsToCurrentThread());
   DCHECK(audio_capturer_);
   DCHECK(audio_stub_);
+
   audio_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&AudioScheduler::DoStart, this));
+      FROM_HERE, base::Bind(&AudioScheduler::StartOnAudioThread, this));
 }
 
 void AudioScheduler::Stop(const base::Closure& done_task) {
-  DCHECK(!done_task.is_null());
-
-  if (!audio_task_runner_->BelongsToCurrentThread()) {
-    audio_task_runner_->PostTask(FROM_HERE, base::Bind(
-        &AudioScheduler::Stop, this, done_task));
-    return;
-  }
-
-  audio_capturer_->Stop();
-
-  network_task_runner_->PostTask(FROM_HERE, base::Bind(
-      &AudioScheduler::DoStopOnNetworkThread, this, done_task));
-}
-
-void AudioScheduler::OnClientDisconnected() {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(!done_task.is_null());
   DCHECK(audio_stub_);
+
+  // Clear |audio_stub_| to prevent audio packets being delivered to the client.
   audio_stub_ = NULL;
+
+  audio_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&AudioScheduler::StopOnAudioThread, this, done_task));
 }
 
 AudioScheduler::~AudioScheduler() {
 }
 
-void AudioScheduler::NotifyAudioPacketCaptured(
-    scoped_ptr<AudioPacket> packet) {
-  DCHECK(packet.get());
-
-  if (!enabled_)
-    return;
-
-  scoped_ptr<AudioPacket> encoded_packet =
-      audio_encoder_->Encode(packet.Pass());
+void AudioScheduler::StartOnAudioThread() {
+  DCHECK(audio_task_runner_->BelongsToCurrentThread());
 
-  // The audio encoder returns a NULL audio packet if there's no audio to send.
-  if (encoded_packet.get()) {
-    network_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&AudioScheduler::DoSendAudioPacket,
-                              this, base::Passed(encoded_packet.Pass())));
-  }
+  // TODO(kxing): Do something with the return value.
+  audio_capturer_->Start(
+      base::Bind(&AudioScheduler::EncodeAudioPacket, this));
 }
 
-void AudioScheduler::DoStart() {
+void AudioScheduler::StopOnAudioThread(const base::Closure& done_task) {
   DCHECK(audio_task_runner_->BelongsToCurrentThread());
+  audio_capturer_->Stop();
 
-  // TODO(kxing): Do something with the return value.
-  audio_capturer_->Start(
-      base::Bind(&AudioScheduler::NotifyAudioPacketCaptured, this));
+  network_task_runner_->PostTask(FROM_HERE, done_task);
 }
 
 void AudioScheduler::SetEnabled(bool enabled) {
@@ -97,27 +78,32 @@ void AudioScheduler::SetEnabled(bool enabled) {
   enabled_ = enabled;
 }
 
-void AudioScheduler::DoSendAudioPacket(scoped_ptr<AudioPacket> packet) {
-  DCHECK(network_task_runner_->BelongsToCurrentThread());
+void AudioScheduler::EncodeAudioPacket(scoped_ptr<AudioPacket> packet) {
+  DCHECK(audio_task_runner_->BelongsToCurrentThread());
   DCHECK(packet.get());
 
-  if (network_stopped_ || !audio_stub_)
+  if (!enabled_)
     return;
 
-  audio_stub_->ProcessAudioPacket(
-      packet.Pass(),
-      base::Bind(&AudioScheduler::OnCaptureCallbackNotified, this));
-}
+  scoped_ptr<AudioPacket> encoded_packet =
+      audio_encoder_->Encode(packet.Pass());
 
-void AudioScheduler::OnCaptureCallbackNotified() {
+  // The audio encoder returns a NULL audio packet if there's no audio to send.
+  if (encoded_packet.get()) {
+    network_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&AudioScheduler::SendAudioPacket,
+                              this, base::Passed(encoded_packet.Pass())));
+  }
 }
 
-void AudioScheduler::DoStopOnNetworkThread(const base::Closure& done_task) {
+void AudioScheduler::SendAudioPacket(scoped_ptr<AudioPacket> packet) {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
+  DCHECK(packet.get());
 
-  network_stopped_ = true;
+  if (!audio_stub_)
+    return;
 
-  done_task.Run();
+  audio_stub_->ProcessAudioPacket(packet.Pass(), base::Closure());
 }
 
 }  // namespace remoting
diff --git a/remoting/host/audio_scheduler.h b/remoting/host/audio_scheduler.h
index 130edeb..9ccc991 100644
--- a/remoting/host/audio_scheduler.h
+++ b/remoting/host/audio_scheduler.h
@@ -43,9 +43,6 @@ class AudioScheduler : public base::RefCountedThreadSafe<AudioScheduler> {
   // Stop the recording session.
   void Stop(const base::Closure& done_task);
 
-  // Called when a client disconnects.
-  void OnClientDisconnected();
-
   // Enable or disable audio on a running session.
   // This leaves the audio capturer running, and only affects whether or not the
   // captured audio is encoded and sent on the wire.
@@ -55,18 +52,17 @@ class AudioScheduler : public base::RefCountedThreadSafe<AudioScheduler> {
   friend class base::RefCountedThreadSafe<AudioScheduler>;
   virtual ~AudioScheduler();
 
-  void NotifyAudioPacketCaptured(scoped_ptr<AudioPacket> packet);
-
-  void DoStart();
+  // Called on the audio thread to start capturing.
+  void StartOnAudioThread();
 
-  // Sends an audio packet to the client.
-  void DoSendAudioPacket(scoped_ptr<AudioPacket> packet);
+  // Called on the audio thread to stop capturing.
+  void StopOnAudioThread(const base::Closure& done_task);
 
-  // Signal network thread to cease activities.
-  void DoStopOnNetworkThread(const base::Closure& done_task);
+  // Called on the audio thread when a new audio packet is available.
+  void EncodeAudioPacket(scoped_ptr<AudioPacket> packet);
 
-  // Called when an AudioPacket has been delivered to the client.
-  void OnCaptureCallbackNotified();
+  // Called on the network thread to send a captured packet to the audio stub.
+  void SendAudioPacket(scoped_ptr<AudioPacket> packet);
 
   scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index 7400287..d311d46 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -213,7 +213,6 @@ void ClientSession::Stop(const base::Closure& done_task) {
 
   done_task_ = done_task;
   if (audio_scheduler_.get()) {
-    audio_scheduler_->OnClientDisconnected();
     audio_scheduler_->Stop(base::Bind(&ClientSession::OnRecorderStopped, this));
     audio_scheduler_ = NULL;
   }