Expand suspension of idle media players to all platforms.

media/blink/webmediaplayer_delegate.h

 // Copyright 2013 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.
 
 #ifndef MEDIA_BLINK_WEBMEDIAPLAYER_DELEGATE_H_
 #define MEDIA_BLINK_WEBMEDIAPLAYER_DELEGATE_H_
 
 namespace blink {
 class WebMediaPlayer;
 }
 namespace media {
 
 // An interface to allow a WebMediaPlayer to communicate changes of state to
 // objects that need to know.
 class WebMediaPlayerDelegate {
  public:
   class Observer {
    public:
-    // Called when the WebMediaPlayer is no longer in the foreground.  Audio may
-    // continue in the background unless |must_suspend| is true.
-    virtual void OnHidden(bool must_suspend) = 0;
+    // Called when the WebMediaPlayer enters the background or foreground
+    // respectively. Note: Some implementations will stop playback when hidden,
+    // and thus subsequently call WebMediaPlayerDelegate::PlayerGone().
+    virtual void OnHidden() = 0;
+    virtual void OnShown() = 0;
 
-    virtual void OnShown() = 0;
+    // Requests a WebMediaPlayer instance to release all idle resources. If
+    // |must_suspend| is false, the player may ignore the request. However, if
+    // |must_suspend| is true, the player must stop playback, release all idle
+    // resources, and finally call WebMediaPlayerDelegate::PlayerGone().
+    virtual void OnSuspendRequested(bool must_suspend) = 0;

[xhwang, 2016/03/10 21:51:06]

One question for API. Is it legit that a WMP implementation wants to force
releasing resources upon hidden? If so, it seems calling OnHidden() is not
enough. But calling OnSuspendRequested(true) only seems wrong as well because we
may miss something in OnHidden(). Calling both doesn't look nice either.

Can we say in this case we should always call OnHidden() and must not call
OnSuspendRequested()?

[DaleCurtis, 2016/03/10 23:11:36]

What case are you talking about? It seems your proposing a hypothetical and
assuming it happens in the code today? To be clear, there is no WMP
implementation that wants to release ALL resource types when hidden. I feel that
is a detail of the WMP implementation too. Further in that case it's actually
sufficient to call OnSuspendRequested(true) so I'm not sure exactly what you're
asking here :)

[xhwang, 2016/03/11 00:09:19]

Agreed that this is a hypothetical case.

As you said, in that case it should be sufficient to call
OnSuspendRequested(true). But this isn't clear in the API. People can easily
assume that whenever a player is hidden they have to call OnHidden().

[DaleCurtis, 2016/03/11 00:15:12]

I think that's a correct assumption though :) Hidden will always be delivered
when the render frame is backgrounded. The alternative is going back to a single
mega-method like OnSuspend(REASON_HIDDEN_BG_AUDIO_OPTIONAL |
REASON_FORCED_SUSPEND_MANDATORY | REASON_IDLE_OPTIONAL). Which I don't like as
much as the separate methods we have now. Not the least of which because my
names stink :)

+
     virtual void OnPlay() = 0;
     virtual void OnPause() = 0;
 
-    // Playout volume should be set to current_volume * multiplier.  The range
-    // is [0, 1] and is typically 1.
+    // Playout volume should be set to current_volume * multiplier. The range is
+    // [0, 1] and is typically 1.
     virtual void OnVolumeMultiplierUpdate(double multiplier) = 0;
   };
 
   WebMediaPlayerDelegate() {}
 
   // Subscribe or unsubscribe from observer callbacks respectively. A client
   // must use the delegate id returned by AddObserver() for all other calls.
   virtual int AddObserver(Observer* observer) = 0;
   virtual void RemoveObserver(int delegate_id) = 0;
 
   // The specified player started playing media.
   virtual void DidPlay(int delegate_id,
                        bool has_video,
                        bool has_audio,
                        bool is_remote,
                        base::TimeDelta duration) = 0;
 
   // The specified player stopped playing media.
   virtual void DidPause(int delegate_id, bool reached_end_of_stream) = 0;
 
-  // The specified player was destroyed or suspended.  This may be called
+  // The specified player was destroyed or suspended. This may be called
   // multiple times in row. Note: Clients must still call RemoveObserver() to
   // unsubscribe from callbacks.
   virtual void PlayerGone(int delegate_id) = 0;
 
   // Returns whether the render frame is currently hidden.
   virtual bool IsHidden() = 0;
 
  protected:
   virtual ~WebMediaPlayerDelegate() {}
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BLINK_WEBMEDIAPLAYER_DELEGATE_H_