Fixed the audio backgrounding bug

content/browser/renderer_host/media/audio_renderer_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/audio_renderer_host.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/memory/shared_memory.h"
@@ skipping 633 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (entry->playing() == is_playing)
     return;
 
   bool should_alert_resource_scheduler;
   if (is_playing) {
     should_alert_resource_scheduler =
         !RenderFrameHasActiveAudio(entry->render_frame_id());
     entry->set_playing(true);
     base::AtomicRefCountInc(&num_playing_streams_);
+
+    // If it's the first audio stream to start for this renderer.

[DaleCurtis, 2015/06/29 22:44:23]

Comment doesn't add much value, explain why instead of just transliterating the
conditional.

[sebsg, 2015/07/02 12:58:08]

Done.

+    if (base::AtomicRefCountIsOne(&num_playing_streams_)) {
+      RenderProcessHost* render_process_host =
+          RenderProcessHost::FromID(render_process_id_);
+
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(&RenderProcessHost::AudioStarted,
+                     base::Unretained(render_process_host)));

[gab, 2015/06/30 13:41:40]

base::Unretained() here is incorrect because there are no guarantees that
|render_process_host| will still be around by the time this task is run (which
could result in a use-after-free).

For educational purposes, here are a couple ways to go about passing the |this|
pointer:

1) Use Unretained if you have lifetime guarantees (you don't here).

2) Use refcounting (default if not using any bind helper): only an option if the
object implements RefCounted (RenderProcessHost doesn't) AND it makes sense for
your task to prevent the object from being deleted (in this case it doesn't
either).

3) Use a WeakPtr: only an option if the object vends WeakPtrs (e.g. implements
SupportsWeakPtr or has a WeakPtrFactory) -- not the case here either, but would
have been a nice option (WeakPtrs will check that they are still valid before
running the callback).

None of the above actually work great here so here's what I suggest, instead of
binding to the RenderProcessHost object right away, add a method in this task's
anonymous namespace that runs on the UI thread and takes the
|render_process_id_| as an argument, from it obtain the |render_process_host|
pointer and call the RenderProcessHost's AudioFoo() methods directly.

In fact, as I'm done writing this I had a thought that you probably shouldn't
call RenderProcessHost::FromID() outside the UI thread. And indeed you
shouldn't:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

So this is really the only option (and you should enable dcheck_always_on=1 in
your build ASAP!).

[DaleCurtis, 2015/06/30 16:34:35]

Oops, thanks for catching this. The last method is in fact what should be done
here. I had a brain fart and thought this was attaching to the browser process,
so if it went poof everything went with it.

[sebsg, 2015/07/02 12:58:08]

Done.

[sebsg, 2015/07/02 12:58:08]

Done.

+    }
   } else {
     entry->set_playing(false);
     should_alert_resource_scheduler =
         !RenderFrameHasActiveAudio(entry->render_frame_id());
-    base::AtomicRefCountDec(&num_playing_streams_);
+
+    // If it was the last audio stream playing for this renderer.

[DaleCurtis, 2015/06/29 22:44:23]

Ditto.

[sebsg, 2015/07/02 12:58:08]

Done.

+    if (!base::AtomicRefCountDec(&num_playing_streams_)) {
+      RenderProcessHost* render_process_host =
+          RenderProcessHost::FromID(render_process_id_);
+
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(&RenderProcessHost::AudioStopped,
+                     base::Unretained(render_process_host)));
+    }
   }
 
   if (should_alert_resource_scheduler && ResourceDispatcherHostImpl::Get()) {
     BrowserThread::PostTaskAndReplyWithResult(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&RenderFrameIdToRenderViewId, render_process_id_,
                    entry->render_frame_id()),
         base::Bind(&NotifyResourceDispatcherOfAudioStateChange,
                    render_process_id_, is_playing));
   }
 }
 
 bool AudioRendererHost::HasActiveAudio() {
   return !base::AtomicRefCountIsZero(&num_playing_streams_);
 }
 
 bool AudioRendererHost::RenderFrameHasActiveAudio(int render_frame_id) const {
   for (AudioEntryMap::const_iterator it = audio_entries_.begin();
        it != audio_entries_.end();
        ++it) {
     AudioEntry* entry = it->second;
     if (entry->render_frame_id() == render_frame_id && entry->playing())
       return true;
   }
   return false;
 }
 
 }  // namespace content