Fix recently audible tab alert state for portals.

1. Add having any kind of inner contents that is currently audible
   as a qualifying condition for a contents being audible. This
   encompasses portals.

2. Correspondingly, add attaching/detaching an inner contents as
   a possible reason for the audio state changing. Detachment can
   happen implicitly during contents destruction (which already
   deals with this, and would otherwise attempt to access
   destructed members), so this case is excluded.

3. Currently the timeout that the tab actually observes is the
   one issued directly by AudioStreamMonitor, but an inner contents
   AudioStreamMonitor may already be destroyed (along with its
   owning contents) at the time the 2-second timer expires. It
   notifies using the navigation state signal.

   The outer contents RecentlyAudibleHelper is aware of the correct
   state, but pending the resolution of crbug.com/846374 does not
   notify the tab strip of this. Until that is resolved, add a
   notification directly from the helper.

   This may duplicate the AudioStreamMonitor notification, but
   this appears to be harmless (we'll just recompute the alert state
   twice) and in any event will go away when crbug.com/846374 is
   fixed.

4. Browser tests that watch the tab UI state are included. These tests
   are high-level integration tests, because tests which merely
   examined the recently-audible state would not have caught some
   issues that arose from notification of that state change not being
   propagated to the chrome.

Fixed: 1085862
Change-Id: I9c6c7be64883914fdbfff5bf565718b108113b7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213118Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773374}

chrome/browser/portal/DEPS

+specific_include_rules = {
+  "portal_recently_audible_browsertest\.cc": [
+    # Used to verify that the browser UI was duly notified about audio state
+    # changes.
+    "+chrome/browser/ui/views/frame/browser_view.h",
+    "+chrome/browser/ui/views/tabs/tab_strip.h",
+  ],
+}

chrome/browser/ui/recently_audible_helper.cc

@@ -77,6 +77,14 @@ void RecentlyAudibleHelper::OnRecentlyAudibleTimerFired() {
          tick_clock_->NowTicks());
   // Notify of the transition to no longer being recently audible.
   callback_list_.Notify(false);
+
+  // This notification is redundant in most cases, because WebContents is
+  // notified by AudioStreamMonitor of changes due to audio in its own frames
+  // (but not in inner contents) directly.
+  //
+  // TODO(https://crbug.com/846374): Remove this once WebContents is notified
+  // via |callback_list_| in this class instead.
+  web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_AUDIO);
 }
 
 void RecentlyAudibleHelper::TransitionToNotCurrentlyAudible() {

chrome/test/BUILD.gn

@@ -1059,6 +1059,7 @@ if (!is_android) {
       "../browser/policy/site_isolation_policy_browsertest.cc",
       "../browser/policy/url_blacklist_policy_browsertest.cc",
       "../browser/portal/portal_browsertest.cc",
+      "../browser/portal/portal_recently_audible_browsertest.cc",
       "../browser/predictors/loading_predictor_browsertest.cc",
       "../browser/prefetch/prefetch_browsertest.cc",
       "../browser/prefs/pref_functional_browsertest.cc",

content/browser/web_contents/web_contents_impl.cc

@@ -306,6 +306,20 @@ void RecordMaxFrameCountUMA(size_t max_frame_count) {
                              max_frame_count);
 }
 
+// Returns whether the condition provided applies to any inner contents.
+// This check is not recursive (however, the predicate provided may itself
+// recurse each contents' own inner contents).
+//
+// For example, if this is used to aggregate state from inner contents to outer
+// contents, then that propagation will gather transitive descendants without
+// need for this helper to do so. In fact, in such cases recursing on inner
+// contents here would make that operation quadratic rather than linear.
+template <typename Functor>
+bool AnyInnerWebContents(WebContents* web_contents, const Functor& f) {
+  const auto& inner_contents = web_contents->GetInnerWebContents();
+  return std::any_of(inner_contents.begin(), inner_contents.end(), f);
+}
+
 }  // namespace
 
 CreatedWindow::CreatedWindow() = default;
@@ -494,6 +508,7 @@ void WebContentsImpl::WebContentsTreeNode::AttachInnerWebContents(
   inner_web_contents_.push_back(std::move(inner_web_contents));
 
   render_frame_host->frame_tree_node()->AddObserver(&inner_web_contents_node);
+  current_web_contents_->InnerWebContentsAttached(inner_web_contents_impl);
 }
 
 std::unique_ptr<WebContents>
@@ -505,6 +520,7 @@ WebContentsImpl::WebContentsTreeNode::DetachInnerWebContents(
       detached_contents = std::move(web_contents);
       std::swap(web_contents, inner_web_contents_.back());
       inner_web_contents_.pop_back();
+      current_web_contents_->InnerWebContentsDetached(inner_web_contents);
       return detached_contents;
     }
   }
@@ -1681,10 +1697,16 @@ void WebContentsImpl::OnAudioStateChanged() {
   // This notification can come from any embedded contents or from this
   // WebContents' stream monitor. Aggregate these signals to get the actual
   // state.
+  //
+  // Note that guests may not be attached as inner contents, and so may need to
+  // be checked separately.
   bool is_currently_audible =
       audio_stream_monitor_.IsCurrentlyAudible() ||
       (browser_plugin_embedder_ &&
-       browser_plugin_embedder_->AreAnyGuestsCurrentlyAudible());
+       browser_plugin_embedder_->AreAnyGuestsCurrentlyAudible()) ||
+      AnyInnerWebContents(this, [](WebContents* inner_contents) {
+        return inner_contents->IsCurrentlyAudible();
+      });
   if (is_currently_audible == is_currently_audible_)
     return;
 
@@ -5741,6 +5763,18 @@ void WebContentsImpl::InnerWebContentsCreated(WebContents* inner_web_contents) {
     observer.InnerWebContentsCreated(inner_web_contents);
 }
 
+void WebContentsImpl::InnerWebContentsAttached(
+    WebContents* inner_web_contents) {
+  if (inner_web_contents->IsCurrentlyAudible())
+    OnAudioStateChanged();
+}
+
+void WebContentsImpl::InnerWebContentsDetached(
+    WebContents* inner_web_contents) {
+  if (!is_being_destroyed_)
+    OnAudioStateChanged();
+}
+
 void WebContentsImpl::RenderViewCreated(RenderViewHost* render_view_host) {
   if (delegate_)
     view_->SetOverscrollControllerEnabled(CanOverscrollContent());

content/browser/web_contents/web_contents_impl.h

@@ -1493,6 +1493,12 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   // guarantee that |inner_web_contents| has been added to the WebContents tree.
   void InnerWebContentsCreated(WebContents* inner_web_contents);
 
+  // Called just after an inner web contents is attached.
+  void InnerWebContentsAttached(WebContents* inner_web_contents);
+
+  // Called just after an inner web contents is detached.
+  void InnerWebContentsDetached(WebContents* inner_web_contents);
+
   // Navigation helpers --------------------------------------------------------
   //
   // These functions are helpers for Navigate() and DidNavigate().