Strip out stream counting from AudioRendererHost.

content/browser/renderer_host/render_process_host_impl.cc

 // Copyright 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.
 
 // Represents the browser side of the browser <--> renderer communication
 // channel. There will be one RenderProcessHost per renderer process.
 
 #include "content/browser/renderer_host/render_process_host_impl.h"
 
 #include <algorithm>
@@ skipping 301 matching lines @@
     return IPC::InvalidPlatformFileForTransit();
   }
   return IPC::TakePlatformFileForTransit(std::move(dump_file));
 }
 
 // Allow us to only run the trial in the first renderer.
 bool has_done_stun_trials = false;
 
 #endif
 
+// Tracks the maximum number of simultaneous output streams browser-wide.
+// Accessed on UI thread.
+base::LazyInstance<media::AudioStreamsTracker> g_audio_streams_tracker =
+    LAZY_INSTANCE_INITIALIZER;
+
 // the global list of all renderer processes
 base::LazyInstance<IDMap<RenderProcessHost*>>::Leaky g_all_hosts =
     LAZY_INSTANCE_INITIALIZER;
 
 // Map of site to process, to ensure we only have one RenderProcessHost per
 // site in process-per-site mode.  Each map is specific to a BrowserContext.
 class SiteProcessMap : public base::SupportsUserData::Data {
  public:
   typedef base::hash_map<std::string, RenderProcessHost*> SiteToProcessMap;
   SiteProcessMap() {}
@@ skipping 443 matching lines @@
 
   is_dead_ = true;
 
   UnregisterHost(GetID());
 
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableGpuShaderDiskCache)) {
     BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                             base::Bind(&RemoveShaderInfo, GetID()));
   }
+
+  // If we had any streams, report UMA stats for the maximum number of
+  // simultaneous streams for this render process and for the whole browser
+  // process since last reported.
+  if (audio_streams_tracker_.max_stream_count() > 0) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Media.AudioRendererIpcStreams",
+                                audio_streams_tracker_.max_stream_count(), 1,
+                                50, 51);
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "Media.AudioRendererIpcStreamsTotal",
+        g_audio_streams_tracker.Get().max_stream_count(), 1, 100, 101);

[ncarter (slow), 2017/01/31 18:33:12]

This logging used to happen each time a MessageFilter was destroyed; now it
happens each time a RenderProcessHostImpl is destroyed. This is not the same:
each type of MessageFilter can be created and destroyed multiple times for a
particular RenderProcessHostImpl -- in particular, this happens in the crash /
sad tab case -- the RenderProcessHostImpl stays around, but
CreateMessageFilters() occurs again, creating a second generation of message
filters for the RPH instance, if the tab is reloaded.

So this isn't the same as the old behavior. Are you sure it's what you intend?
In particular I worry that if you force-kill a process that has active audio
streams, will the count actually fall to zero, or will you wind up in an
incorrect state?

It would be good to have browsertests for that case. histogram_tester.h in base
can be useful for that sort of test.

[DaleCurtis, 2017/01/31 22:08:31]

Ah, I didn't know multiple sets of MessageFilter's can be created like this. The
spirit of the logging was render process shutdown, though the implementation was
not. I'd actually like to just delete these metrics; I don't think they're all
that useful. +grunell for commentary.

[Henrik Grunell, 2017/02/03 08:52:20]

Sorry Dale, I missed that I was supposed to reply to this. I think we can remove
these metrics. It was added for the mixing experiment which is deprecated in its
current form. I don't see any reason to keep it around. If we'll work on
changing the mixing later on, we can re-add these or other metrics.

Olga and Max: are they useful in the separate process work? (I doubt that.)

[DaleCurtis, 2017/02/06 22:19:02]

Deleted, thanks!

+    g_audio_streams_tracker.Get().ResetMaxStreamCount();
+  }
 }
 
 bool RenderProcessHostImpl::Init() {
   // calling Init() more than once does nothing, this makes it more convenient
   // for the view host which may not be sure in some cases
   if (HasConnection())
     return true;
 
   is_dead_ = false;
 
@@ skipping 753 matching lines @@
   if (visible_widgets_ == 0) {
     DCHECK(!is_process_backgrounded_);
     UpdateProcessPriority();
   }
 }
 
 int RenderProcessHostImpl::VisibleWidgetCount() const {
   return visible_widgets_;
 }
 
-void RenderProcessHostImpl::AudioStateChanged() {
+void RenderProcessHostImpl::OnAudioStreamAdded() {
+  audio_streams_tracker_.IncreaseStreamCount();
+  g_audio_streams_tracker.Get().IncreaseStreamCount();
   UpdateProcessPriority();

[Max Morin, 2017/01/30 11:47:07]

It probably makes sense to only UpdateProcessPriority if the value of
audio_streams_tracker_.has_streams() changed.

[DaleCurtis, 2017/02/06 22:19:02]

UpdateProcessPriority() takes care of this.

 }
 
+void RenderProcessHostImpl::OnAudioStreamRemoved() {
+  audio_streams_tracker_.DecreaseStreamCount();
+  g_audio_streams_tracker.Get().DecreaseStreamCount();
+  UpdateProcessPriority();
+}
+
 bool RenderProcessHostImpl::IsForGuestsOnly() const {
   return is_for_guests_only_;
 }
 
 StoragePartition* RenderProcessHostImpl::GetStoragePartition() const {
   return storage_partition_impl_;
 }
 
 static void AppendCompositorCommandLineFlags(base::CommandLine* command_line) {
   command_line->AppendSwitchASCII(
@@ skipping 1236 matching lines @@
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableRendererPriorityManagement)) {
     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 those states.
   const bool should_background =
-      visible_widgets_ == 0 && !audio_renderer_host_->HasActiveAudio() &&
+      visible_widgets_ == 0 && !audio_streams_tracker_.has_streams() &&
       !base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableRendererBackgrounding);
 
 // TODO(sebsg): Remove this ifdef when https://crbug.com/537671 is fixed.
 #if !defined(OS_ANDROID)
   if (is_process_backgrounded_ == should_background)
     return;
 #endif
 
   TRACE_EVENT1("renderer_host", "RenderProcessHostImpl::UpdateProcessPriority",
@@ skipping 228 matching lines @@
   LOG(ERROR) << "Terminating render process for bad Mojo message: " << error;
 
   // The ReceivedBadMessage call below will trigger a DumpWithoutCrashing.
   // Capture the error message in a crash key value.
   base::debug::ScopedCrashKey error_key_value("mojo-message-error", error);
   bad_message::ReceivedBadMessage(render_process_id,
                                   bad_message::RPH_MOJO_PROCESS_ERROR);
 }
 
 }  // namespace content