Propagate audible state from player to the containing tab

content/browser/android/audio_monitor_android.cc

Index: content/browser/android/audio_monitor_android.cc
diff --git a/content/browser/android/audio_monitor_android.cc b/content/browser/android/audio_monitor_android.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b59723e79f4b960b9eec748bd2b62f0115fa99b7
--- /dev/null
+++ b/content/browser/android/audio_monitor_android.cc
@@ -0,0 +1,53 @@
+// Copyright 2015 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/android/audio_monitor_android.h"
+
+#include <climits>
+#include "content/public/browser/web_contents.h"
+
+namespace content {
+
+AudioMonitorAndroid::AudioMonitorAndroid(WebContents* web_contents)
+    : web_contents_(web_contents),
+      is_audible_(false) {
+}
+
+AudioMonitorAndroid::~AudioMonitorAndroid() {}
+
+void AudioMonitorAndroid::OnAudibleStateChanged(RenderFrameHost* rfh,
+                                                int player_id,
+                                                bool is_audible) {
+  audio_status_map_[Key(rfh, player_id)] = is_audible;
+  UpdateStatusAndNotify();
+}
+
+void AudioMonitorAndroid::RemovePlayer(RenderFrameHost* rfh, int player_id) {
+  audio_status_map_.erase(Key(rfh, player_id));
+  UpdateStatusAndNotify();
+}
+
+void AudioMonitorAndroid::RenderFrameDeleted(RenderFrameHost* rfh) {
+  StatusMap::iterator begin = audio_status_map_.lower_bound(Key(rfh, 0));
+  StatusMap::iterator end = audio_status_map_.upper_bound(Key(rfh, INT_MAX));
+  audio_status_map_.erase(begin, end);
+  UpdateStatusAndNotify();
+}
+
+void AudioMonitorAndroid::UpdateStatusAndNotify() {
+  bool is_audible = false;
+  for (const auto& player_status : audio_status_map_) {

[no sievers, 2015/02/10 01:33:03]

Does this really need a separate class for the implementation?
It seems like we are really only tracking one bit of state here (per player),
but are adding a lot of plumbing and complexity that could cause bugs (think
objects pointing to each other and their lifetime) and makes the code harder to
follow.

If you kept the interface, can MediaWebContentsObserver just implement this?
Although I think only 'IsAudible()' might remain, in which case it could be a
callback.

[Tima Vaisburd, 2015/02/11 03:34:05]

It does not require a separate class, strictly speaking, but I think in this
situation
it would make sense to have an AndroidAudioMonitor that is in a sense parallel
to AudioStreamMonitor
of the desktop.

[no sievers, 2015/02/12 19:01:11]

If you are arguing that way: Maybe we should make AudioStreamMonitor the base
interface?

+    if (player_status.second) {
+      is_audible = true;
+      break; // at least one player is making noise
+    }
+  }
+
+  if (is_audible_ != is_audible) {
+    is_audible_ = is_audible;
+    web_contents_->NotifyNavigationStateChanged(INVALIDATE_TYPE_TAB);
+  }
+}
+
+} // namespace content