Refactor WebMediaPlayerDelegate interface.

media/blink/webmediaplayer_delegate.h

Index: media/blink/webmediaplayer_delegate.h
diff --git a/media/blink/webmediaplayer_delegate.h b/media/blink/webmediaplayer_delegate.h
index 62c93bc22dd4339fcd74ae9dcd0dd532cbdba494..b9886f77e36abca096dfe4760f523805ff483ded 100644
--- a/media/blink/webmediaplayer_delegate.h
+++ b/media/blink/webmediaplayer_delegate.h
@@ -12,73 +12,107 @@ namespace media {
 
 enum class MediaContentType;
 
-// An interface to allow a WebMediaPlayer to communicate changes of state to
-// objects that need to know.
+// An interface to collect WebMediaPlayer state changes and to fan out commands
+// from the browser.
 class WebMediaPlayerDelegate {
  public:
+  // Note: WebMediaPlayerDelegate implementations should not call an Observer
+  // method on a stack that includes a call from the player.
+  // Note: It is likely that players will call WebMediaPlayerDelegate methods
+  // from within Observer callbacks.
   class Observer {
    public:
-    // 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;
-
-    // Requests a WebMediaPlayer instance to release all idle resources. If
-    // |must_suspend| is true, the player must stop playback, release all idle
-    // resources, and finally call WebMediaPlayerDelegate::PlayerGone(). If
-    // |must_suspend| is false, the player may ignore the request. Optionally,
-    // it may do some or all of the same actions as when |must_suspend| is true.
-    // To be clear, the player is not required to call PlayerGone() when
-    // |must_suspend| is false.
-    // Return false to reject the request and indicate that further calls to
-    // OnSuspendRequested() are required. Otherwise the Observer is removed
-    // from the idle list.
-    virtual bool OnSuspendRequested(bool must_suspend) = 0;
+    // Called when the host frame is hidden (usually by tab switching).
+    virtual void OnFrameHidden() = 0;
 
+    // Called when the host frame is closed. Note that it is sometimes possible
+    // for a closed frame to be shown again. (Android only; other platforms tear
+    // down players when the host frame is closed.)
+    virtual void OnFrameClosed() = 0;
+
+    // Called when the host frame is shown (usually by tab switching).
+    virtual void OnFrameShown() = 0;
+
+    // Called when an idle player has become stale, usually interpreted to mean
+    // that it is unlikely to be interacted with in the near future.
+    //
+    // Players should typically respond by releasing resources, for example by
+    // discarding their decoders.
+    virtual void OnIdleTimeout() = 0;
+
+    // Called when external controls are activated.
     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.
+    // Called to control audio ducking. Output volume should be set to
+    // |player_volume| * |multiplier|. The range of |multiplier| is [0, 1],
+    // where 1 indicates normal (non-ducked) playback.
     virtual void OnVolumeMultiplierUpdate(double multiplier) = 0;
   };
 
-  WebMediaPlayerDelegate() {}
+  // These return the visibility state of the host frame.
+  // TODO(sandersd): Experiment to determine exactly what gets called when
+  // restoring a closed tab.
+  virtual bool IsFrameHidden() = 0;

[whywhat, 2017/01/06 17:18:51]

nit: say that IsFrameHidden also considers closed frame to be hidden? it's now a
bit inconsistent with OnFrameHidden() which will not get called when
IsFrameHidden becomes true (OnFrameClosed will be called though). Perhaps, my
earlier suggestion of making IsFrameClosed part of IsFrameHidden was bad and the
original version was cleaner.

[sandersd (OOO until July 31), 2017/01/06 23:08:35]

Done.

+  virtual bool IsFrameClosed() = 0;
 
-  // Subscribe or unsubscribe from observer callbacks respectively. A client
-  // must use the delegate id returned by AddObserver() for all other calls.
+  // Returns |true| if background video playback has been enabled, for example
+  // by a media session 'play' command.
+  virtual bool IsBackgroundVideoPlaybackAllowed() = 0;
+
+  // Subscribe to observer callbacks. A player must use the returned |player_id|
+  // for the rest of the calls below.
   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,
+  // Unsubscribe from observer callbacks.
+  virtual void RemoveObserver(int player_id) = 0;
+
+  // Notify playback started. This will request appropriate wake locks and, if
+  // applicable, show a pause button in external controls.
+  //
+  // DidPlay() should not be called for remote playback.
+  virtual void DidPlay(int player_id,
                        bool has_audio,
-                       bool is_remote,
+                       bool has_video,
                        media::MediaContentType media_content_type) = 0;
 
-  // The specified player stopped playing media. This may be called at any time
-  // with or without a DidPlay() having previously occurred. Calling this will
-  // cause the delegate to be registered for idle suspension. I.e., after some
-  // time elapses without a DidPlay(), OnSuspendRequested() will be issued.
-  virtual void DidPause(int delegate_id, bool reached_end_of_stream) = 0;
-
-  // The specified player was destroyed or suspended and will no longer accept
-  // Observer::OnPlay() or Observer::OnPause() calls. 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;
-
-  // Returns whether there's a video playing in background within the render
-  // frame.
-  virtual bool IsPlayingBackgroundVideo() = 0;
+  // Notify that playback is paused. This will drop wake locks and, if
+  // applicable, show a play button in external controls.
+  // TODO(sandersd): It may be helpful to get |has_audio| and |has_video| here,
+  // so that we can do the right thing with media that starts paused.
+  virtual void DidPause(int player_id) = 0;
+
+  // Notify that playback is stopped. This will drop wake locks and remove any
+  // external controls.
+  //
+  // Clients must still call RemoveObserver() to unsubscribe from observer
+  // callbacks.
+  virtual void PlayerGone(int player_id) = 0;
+
+  // Set the player's idle state. While idle, a player may recieve an
+  // OnIdleTimeout() callback.
+  // TODO(sandersd): Merge this into DidPlay()/DidPause()/PlayerGone().
+  virtual void SetIdle(int player_id, bool is_idle) = 0;
+
+  // Get the player's idle state. A stale player is considered idle.
+  // TODO(sandersd): Remove this. It is only used in tests and in one special
+  // case in WMPI.
+  virtual bool IsIdle(int player_id) = 0;
+
+  // Returns a stale player to an idle state, and resumes OnIdleTimeout() calls
+  // without an additional idle timeout.
+  // TODO(sandersd): This exists only to support WMPI's didLoadingProgress()
+  // workaround. A better option may be to take a 'minimum idle' duration in
+  // SetIdle().
+  virtual void ClearStaleFlag(int player_id) = 0;
+
+  // Returns |true| if the player is stale; that is that OnIdleTimeout() was
+  // called and returned |true|.
+  virtual bool IsStale(int player_id) = 0;
 
  protected:
-  virtual ~WebMediaPlayerDelegate() {}
+  WebMediaPlayerDelegate() = default;
+  virtual ~WebMediaPlayerDelegate() = default;
 };
 
 }  // namespace media