Refactoring media notification

content/browser/media/android/media_session.cc

Index: content/browser/media/android/media_session.cc
diff --git a/content/browser/media/android/media_session.cc b/content/browser/media/android/media_session.cc
index 60360f420e243da124a46a11dda071643d82519e..62ecfc5bdd6825c72d01f06a0b6869b32a312347 100644
--- a/content/browser/media/android/media_session.cc
+++ b/content/browser/media/android/media_session.cc
@@ -87,6 +87,7 @@ bool MediaSession::AddPlayer(MediaSessionObserver* observer,
     players_.clear();
 
   players_.insert(PlayerIdentifier(observer, player_id));
+

[whywhat, 2015/12/09 16:59:27]

nit: accidental change?

   UpdateWebContents();
 
   return true;
@@ -121,14 +122,9 @@ void MediaSession::OnSuspend(JNIEnv* env,
   if (audio_focus_state_ != State::ACTIVE)
     return;
 
-  OnSuspendInternal(SuspendType::SYSTEM);
-  if (!temporary)
-    SetAudioFocusState(State::INACTIVE);
+  OnSuspendInternal(SuspendType::SYSTEM,
+                    temporary ? State::SUSPENDED : State::INACTIVE);
 
-  uma_helper_.RecordSessionSuspended(
-      temporary ? MediaSessionSuspendedSource::SystemTransient
-                : MediaSessionSuspendedSource::SystemPermanent);
-  UpdateWebContents();
 }
 
 void MediaSession::OnResume(JNIEnv* env, const JavaParamRef<jobject>& obj) {
@@ -136,7 +132,6 @@ void MediaSession::OnResume(JNIEnv* env, const JavaParamRef<jobject>& obj) {
     return;
 
   OnResumeInternal(SuspendType::SYSTEM);
-  UpdateWebContents();
 }
 
 void MediaSession::Resume() {
@@ -158,14 +153,14 @@ void MediaSession::Resume() {
 void MediaSession::Suspend() {
   DCHECK(!IsSuspended());
 
-  OnSuspendInternal(SuspendType::UI);
+  OnSuspendInternal(SuspendType::UI, State::SUSPENDED);
 }
 
 void MediaSession::Stop() {
   DCHECK(audio_focus_state_ != State::INACTIVE);
 
   if (audio_focus_state_ != State::SUSPENDED)
-    OnSuspendInternal(SuspendType::UI);
+    OnSuspendInternal(SuspendType::UI, State::SUSPENDED);
 
   DCHECK(audio_focus_state_ == State::SUSPENDED);
   players_.clear();
@@ -204,17 +199,35 @@ void MediaSession::RemoveAllPlayersForTest() {
   AbandonSystemAudioFocusIfNeeded();
 }
 
-void MediaSession::OnSuspendInternal(SuspendType type) {
-  // SuspendType::System will handle the UMA recording at the calling point
-  // because there are more than one type.
+void MediaSession::OnSuspendInternal(SuspendType type, State new_state) {
+  assert(new_state == State::SUSPENDED || new_state == State::INACTIVE);

[whywhat, 2015/12/09 16:59:27]

Don't we use DCHECKs in C++ code outside Blink?

+  // UI suspend cannot use State::INACTIVE.
+  assert(type == SuspendType::SYSTEM || new_state == State::SUSPENDED);
+
   if (type == SuspendType::UI)
     uma_helper_.RecordSessionSuspended(MediaSessionSuspendedSource::UI);
+  if (type == SuspendType::SYSTEM) {
+    switch (new_state) {
+      case State::SUSPENDED:
+        uma_helper_.RecordSessionSuspended(
+            MediaSessionSuspendedSource::SystemTransient);
+        break;
+      case State::INACTIVE:
+        uma_helper_.RecordSessionSuspended(
+            MediaSessionSuspendedSource::SystemPermanent);
+        break;
+      default:
+        break;
+    }
+  }
 
-  SetAudioFocusState(State::SUSPENDED);
+  SetAudioFocusState(new_state);
   suspend_type_ = type;
 
   for (const auto& it : players_)
     it.observer->OnSuspend(it.player_id);
+
+  UpdateWebContents();
 }
 
 void MediaSession::OnResumeInternal(SuspendType type) {
@@ -225,6 +238,8 @@ void MediaSession::OnResumeInternal(SuspendType type) {
 
   for (const auto& it : players_)
     it.observer->OnResume(it.player_id);
+
+  UpdateWebContents();
 }
 
 MediaSession::MediaSession(WebContents* web_contents)