Fix "brief unmute" issue.

This change defers unmuting with one "post task", same as output stream
destruction, so that the ordering of operations is respected. Likely
fixes crbug/957728.

Bug: 957728
Change-Id: I0c8b6dcebe5c6f2c24f673ecf80cd7c2e48be157
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1591521
Commit-Queue: Max Morin <maxmorin@chromium.org>
Auto-Submit: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655980}

services/audio/stream_factory.cc

@@ -222,13 +222,30 @@ void StreamFactory::DestroyMuter(LocalMuter* muter) {
   CHECK_EQ(magic_bytes_, 0x600DC0DEu);
   DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_);
   DCHECK(muter);
-  SetStateForCrashing("destroying muter");
 
-  const auto it = std::find_if(muters_.begin(), muters_.end(),
-                               base::MatchesUniquePtr(muter));
-  DCHECK(it != muters_.end());
-  muters_.erase(it);
-  SetStateForCrashing("destroyed muter");
+  // Output streams have a task posting before destruction (see the OnError
+  // function in output_stream.cc). To ensure that stream destruction and
+  // unmuting is done in the intended order (the order in which the messeges are
+  // received by the service), we post a task for destroying the muter as well.
+  // Otherwise, a "destroy all streams, then destroy the muter" sequence may
+  // result in a brief blip of audio.
+  auto do_destroy = [](base::WeakPtr<StreamFactory> weak_this,
+                       LocalMuter* muter) {
+    if (weak_this) {
+      weak_this->SetStateForCrashing("destroying muter");
+
+      const auto it =
+          std::find_if(weak_this->muters_.begin(), weak_this->muters_.end(),
+                       base::MatchesUniquePtr(muter));
+      DCHECK(it != weak_this->muters_.end());
+      weak_this->muters_.erase(it);
+      weak_this->SetStateForCrashing("destroyed muter");
+    }
+  };
+
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(do_destroy, weak_ptr_factory_.GetWeakPtr(), muter));
 }
 
 void StreamFactory::DestroyLoopbackStream(LoopbackStream* stream) {

services/audio/stream_factory.h

@@ -13,6 +13,7 @@
 #include "base/containers/flat_set.h"
 #include "base/containers/unique_ptr_adapters.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
 #include "media/mojo/interfaces/audio_logging.mojom.h"
 #include "media/mojo/interfaces/audio_output_stream.mojom.h"
@@ -120,6 +121,8 @@ class StreamFactory final : public mojom::StreamFactory {
   // TODO(crbug.com/888478): Remove this after diagnosis.
   volatile uint32_t magic_bytes_;
 
+  base::WeakPtrFactory<StreamFactory> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(StreamFactory);
 };