Fixed the audio backgrounding bug

content/browser/renderer_host/render_process_host_impl.cc

Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index ff4f9869881ce4d4e3e62a73e012b588fa821d65..d9722724f76df15480940105f50991e9785e8590 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -495,7 +495,7 @@ RenderProcessHostImpl::RenderProcessHostImpl(
       pending_views_(0),
       mojo_application_host_(new MojoApplicationHost),
       visible_widgets_(0),
-      backgrounded_(true),
+      is_process_backgrounded_(false),
       is_initialized_(false),
       id_(ChildProcessHostImpl::GenerateChildProcessUniqueId()),
       browser_context_(browser_context),
@@ -533,11 +533,6 @@ RenderProcessHostImpl::RenderProcessHostImpl(
   subscribe_uniform_enabled_ =
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableSubscribeUniformExtension);
-
-  // Note: When we create the RenderProcessHostImpl, it's technically
-  //       backgrounded, because it has no visible listeners.  But the process
-  //       doesn't actually exist yet, so we'll Background it later, after
-  //       creation.
 }
 
 // static
@@ -1089,23 +1084,20 @@ void RenderProcessHostImpl::ShutdownForBadMessage() {
 }
 
 void RenderProcessHostImpl::WidgetRestored() {
-  // Verify we were properly backgrounded.
-  DCHECK_EQ(backgrounded_, (visible_widgets_ == 0));
   visible_widgets_++;
-  SetBackgrounded(false);
+  UpdateProcessPriority();
+  DCHECK(!is_process_backgrounded_);
 }
 
 void RenderProcessHostImpl::WidgetHidden() {
-  // On startup, the browser will call Hide
-  if (backgrounded_)
+  // On startup, the browser will call Hide. We ignore this call.
+  if (visible_widgets_ == 0)
     return;
 
-  DCHECK_EQ(backgrounded_, (visible_widgets_ == 0));
-  visible_widgets_--;
-  DCHECK_GE(visible_widgets_, 0);
+  --visible_widgets_;
   if (visible_widgets_ == 0) {
-    DCHECK(!backgrounded_);
-    SetBackgrounded(true);
+    DCHECK(!is_process_backgrounded_);
+    UpdateProcessPriority();
   }
 }
 
@@ -1113,6 +1105,10 @@ int RenderProcessHostImpl::VisibleWidgetCount() const {
   return visible_widgets_;
 }
 
+void RenderProcessHostImpl::AudioStateChanged() {
+  UpdateProcessPriority();
+}
+
 bool RenderProcessHostImpl::IsForGuestsOnly() const {
   return is_for_guests_only_;
 }
@@ -2135,6 +2131,8 @@ void RenderProcessHostImpl::ProcessDied(bool already_dead,
     delete queued_messages_.front();
     queued_messages_.pop();
   }
+  UpdateProcessPriority();
+  DCHECK(!is_process_backgrounded_);
 
   within_process_died_observer_ = true;
   NotificationService::current()->Notify(
@@ -2252,25 +2250,28 @@ void RenderProcessHostImpl::SuddenTerminationChanged(bool enabled) {
   SetSuddenTerminationAllowed(enabled);
 }
 
-void RenderProcessHostImpl::SetBackgrounded(bool backgrounded) {
-  TRACE_EVENT1("renderer_host", "RenderProcessHostImpl::SetBackgrounded",
-               "backgrounded", backgrounded);
-  // Note: we always set the backgrounded_ value.  If the process is NULL
-  // (and hence hasn't been created yet), we will set the process priority
-  // later when we create the process.
-  backgrounded_ = backgrounded;
-  if (!child_process_launcher_.get() || child_process_launcher_->IsStarting())
+void RenderProcessHostImpl::UpdateProcessPriority() {
+  if (!child_process_launcher_.get() || child_process_launcher_->IsStarting()) {
+    is_process_backgrounded_ = false;
     return;
+  }
 
-  // Don't background processes which have active audio streams.
-  if (backgrounded_ && audio_renderer_host_->HasActiveAudio())
-    return;
+  // We background a process as soon as it hosts no active audio streams and no
+  // visible widgets -- the callers must call this function whenever we
+  // transition in/out of thoses states. Processes are initially launched with

[Avi (use Gerrit), 2015/08/19 00:30:29]

"those states"

[sebsg, 2015/08/19 15:22:21]

Done.

+  // foreground priority, so when the process doesn't exist.

[Avi (use Gerrit), 2015/08/19 00:30:29]

"so when the process doesn't exist..." what? Complete that sentence.

[sebsg, 2015/08/19 15:22:21]

Thanks for noticing that, the comment was made when that logic used the process
existence state. Forgot to update the comment when I modified the code and left
it incomplete. 

+  const bool should_background =
+      visible_widgets_ == 0 && !audio_renderer_host_->HasActiveAudio() &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableRendererBackgrounding);
 
-  const base::CommandLine* command_line =
-      base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kDisableRendererBackgrounding))
+  if (is_process_backgrounded_ == should_background)
     return;
 
+  TRACE_EVENT1("renderer_host", "RenderProcessHostImpl::UpdateProcessPriority",
+               "should_background", should_background);
+  is_process_backgrounded_ = should_background;
+
 #if defined(OS_WIN)
   // The cbstext.dll loads as a global GetMessage hook in the browser process
   // and intercepts/unintercepts the kernel32 API SetPriorityClass in a
@@ -2290,7 +2291,7 @@ void RenderProcessHostImpl::SetBackgrounded(bool backgrounded) {
       base::FieldTrialList::Find("BackgroundRendererProcesses");
   if (!trial || !base::StartsWith(trial->group_name(), "Disallow",
                                   base::CompareCase::SENSITIVE)) {
-    child_process_launcher_->SetProcessBackgrounded(backgrounded);
+    child_process_launcher_->SetProcessBackgrounded(should_background);
   }
 #else
   // Control the background state from the browser process, otherwise the task
@@ -2298,11 +2299,11 @@ void RenderProcessHostImpl::SetBackgrounded(bool backgrounded) {
   // tasks executing at lowered priority ahead of it or simply by not being
   // swiftly scheduled by the OS per the low process priority
   // (http://crbug.com/398103).
-  child_process_launcher_->SetProcessBackgrounded(backgrounded);
+  child_process_launcher_->SetProcessBackgrounded(should_background);
 #endif  // OS_WIN
 
   // Notify the child process of background state.
-  Send(new ChildProcessMsg_SetProcessBackgrounded(backgrounded));
+  Send(new ChildProcessMsg_SetProcessBackgrounded(should_background));
 }
 
 void RenderProcessHostImpl::OnProcessLaunched() {
@@ -2325,7 +2326,8 @@ void RenderProcessHostImpl::OnProcessLaunched() {
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "465841 RenderProcessHostImpl::OnProcessLaunched::Backgrounded"));
     DCHECK(child_process_launcher_->GetProcess().IsValid());
-    SetBackgrounded(backgrounded_);
+    DCHECK(!is_process_backgrounded_);
+    UpdateProcessPriority();
   }
 
   // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841