Split MediaContentType and AudioFocusType

content/browser/media/session/media_session.cc

 // 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/media/session/media_session.h"
 
 #include "content/browser/media/session/media_session_delegate.h"
 #include "content/browser/media/session/media_session_observer.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
+#include "media/base/media_content_type.h"
 
 namespace content {
 
 namespace {
 
 const double kDefaultVolumeMultiplier = 1.0;
 
 }  // anonymous namespace
 
 using MediaSessionSuspendedSource =
@@ skipping 39 matching lines @@
 void MediaSession::SetMetadata(const MediaMetadata& metadata) {
   metadata_ = metadata;
   // TODO(zqzhang): On Android, the metadata is sent though JNI everytime the
   // media session play/pause state changes. Need to find a way to seprate the
   // state change and Metadata update. See https://crbug.com/621855.
   static_cast<WebContentsImpl*>(web_contents())->OnMediaSessionStateChanged();
 }
 
 bool MediaSession::AddPlayer(MediaSessionObserver* observer,
                              int player_id,
-                             Type type) {
+                             media::MediaContentType type) {
+  if (type == media::MediaContentType::Uncontrollable) {
+    return true;
+  }

[whywhat, 2016/08/18 20:33:41]

nit: no need for {} here

[Zhiqiang Zhang (Slow), 2016/08/19 15:05:55]

Done.

   observer->OnSetVolumeMultiplier(player_id, volume_multiplier_);
 
   // If the audio focus is already granted and is of type Content, there is

[whywhat, 2016/08/18 20:33:41]

nit: this comment needs to be updated

[Zhiqiang Zhang (Slow), 2016/08/19 15:05:55]

Moved these to the next block (early return).

   // nothing to do. If it is granted of type Transient the requested type is
   // also transient, there is also nothing to do. Otherwise, the session needs
   // to request audio focus again.
+  // TODO(zqzhang): handle duckable and uncontrollable.

[whywhat, 2016/08/18 20:33:41]

nit: do you need to file a crbug to track the TODO?

[Zhiqiang Zhang (Slow), 2016/08/19 15:05:55]

Done.

+  AudioFocusManager::AudioFocusType required_audio_focus_type =

[whywhat, 2016/08/18 20:33:42]

could this be a media::AudioFocutTypeFromMediaContentType or something?
nit: also, prefer if's to ?: unless it fits on one line
nit: s/required/requested/

[Zhiqiang Zhang (Slow), 2016/08/19 15:05:55]

Using if for now. After adding Pepper&WebRTC, we'll decide whether to move the
logic into a helper function.
I think "required" is better, which means the required audio focus for playing
the added player.

+      (type == media::MediaContentType::Gain)

[whywhat, 2016/08/18 20:33:41]

there's no MediaContentType::Gain, right?

[Zhiqiang Zhang (Slow), 2016/08/19 15:05:55]

Done.

+          ? AudioFocusManager::AudioFocusType::Gain
+          : AudioFocusManager::AudioFocusType::GainTransientMayDuck;
+
   if (audio_focus_state_ == State::ACTIVE &&
-      (audio_focus_type_ == Type::Content || audio_focus_type_ == type)) {
+      (audio_focus_type_ == AudioFocusManager::AudioFocusType::Gain ||
+       audio_focus_type_ == required_audio_focus_type)) {
     players_.insert(PlayerIdentifier(observer, player_id));
     return true;
   }
 
   State old_audio_focus_state = audio_focus_state_;
-  State audio_focus_state = RequestSystemAudioFocus(type) ? State::ACTIVE
-                                                          : State::INACTIVE;
+  State audio_focus_state = RequestSystemAudioFocus(required_audio_focus_type)
+                                ? State::ACTIVE
+                                : State::INACTIVE;
   SetAudioFocusState(audio_focus_state);
-  audio_focus_type_ = type;
+  audio_focus_type_ = required_audio_focus_type;
 
   if (audio_focus_state_ != State::ACTIVE)
     return false;
 
   // The session should be reset if a player is starting while all players are
   // suspended.
   if (old_audio_focus_state != State::ACTIVE)
     players_.clear();
 
   players_.insert(PlayerIdentifier(observer, player_id));
@@ skipping 106 matching lines @@
 bool MediaSession::IsReallySuspended() const {
   return audio_focus_state_ == State::SUSPENDED;
 }
 
 bool MediaSession::IsSuspended() const {
   // TODO(mlamouri): should be == State::SUSPENDED.
   return audio_focus_state_ != State::ACTIVE;
 }
 
 bool MediaSession::IsControllable() const {
   // Only content type media session can be controllable unless it is inactive.

[whywhat, 2016/08/18 20:33:42]

nit: update comment?

[Zhiqiang Zhang (Slow), 2016/08/19 15:05:55]

Done.

   return audio_focus_state_ != State::INACTIVE &&
-         audio_focus_type_ == Type::Content;
+         audio_focus_type_ == AudioFocusManager::AudioFocusType::Gain;
 }
 
 std::unique_ptr<base::CallbackList<void(MediaSession::State)>::Subscription>
 MediaSession::RegisterMediaSessionStateChangedCallbackForTest(
     const StateChangedCallback& cb) {
   return media_session_state_listeners_.Add(cb);
 }
 
 void MediaSession::SetDelegateForTests(
     std::unique_ptr<MediaSessionDelegate> delegate) {
   delegate_ = std::move(delegate);
 }
 
 bool MediaSession::IsActiveForTest() const {
   return audio_focus_state_ == State::ACTIVE;
 }
 
-MediaSession::Type MediaSession::audio_focus_type_for_test() const {
+AudioFocusManager::AudioFocusType MediaSession::audio_focus_type_for_test()
+    const {
   return audio_focus_type_;
 }
 
 MediaSessionUmaHelper* MediaSession::uma_helper_for_test() {
   return &uma_helper_;
 }
 
 void MediaSession::RemoveAllPlayersForTest() {
   players_.clear();
   AbandonSystemAudioFocusIfNeeded();
@@ skipping 53 matching lines @@
 
   for (const auto& it : players_)
     it.observer->OnResume(it.player_id);
 
   UpdateWebContents();
 }
 
 MediaSession::MediaSession(WebContents* web_contents)
     : WebContentsObserver(web_contents),
       audio_focus_state_(State::INACTIVE),
-      audio_focus_type_(Type::Transient),
-      volume_multiplier_(kDefaultVolumeMultiplier) {
-}
+      audio_focus_type_(
+          AudioFocusManager::AudioFocusType::GainTransientMayDuck),
+      volume_multiplier_(kDefaultVolumeMultiplier) {}
 
 void MediaSession::Initialize() {
   delegate_ = MediaSessionDelegate::Create(this);
 }
 
-bool MediaSession::RequestSystemAudioFocus(Type type) {
+bool MediaSession::RequestSystemAudioFocus(
+    AudioFocusManager::AudioFocusType type) {
   bool result = delegate_->RequestAudioFocus(type);
   uma_helper_.RecordRequestAudioFocusResult(result);
   return result;
 }
 
 void MediaSession::AbandonSystemAudioFocusIfNeeded() {
   if (audio_focus_state_ == State::INACTIVE || !players_.empty())
     return;
 
   delegate_->AbandonAudioFocus();
@@ skipping 19 matching lines @@
     case State::SUSPENDED:
       uma_helper_.OnSessionSuspended();
       break;
     case State::INACTIVE:
       uma_helper_.OnSessionInactive();
       break;
   }
 }
 
 }  // namespace content