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..8c7c6ce10824a161d87c035ac354f9dc5d1abf5f 100644
--- a/media/blink/webmediaplayer_delegate.h
+++ b/media/blink/webmediaplayer_delegate.h
@@ -12,73 +12,108 @@ 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 may
+    // return |false| to indicate they did not handle the event, in which case
+    // OnIdleTimeout() will be called again in the future.
+    //
+    // Players should typically respond by releasing resources, for example by
+    // discarding their decoders.
+    //
+    // Players MUST NOT call SetIdle(false) from OnIdleTimeout().
+    virtual bool 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;
+  virtual bool IsFrameClosed() = 0;
+
+  // Returns |true| if background video playback has been enabled, for example
+  // by a media session 'play' command.
+  virtual bool IsBackgroundVideoPlaybackAllowed() = 0;
 
-  // Subscribe or unsubscribe from observer callbacks respectively. A client
-  // must use the delegate id returned by AddObserver() for all other calls.
+  // 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.
+  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.
+  virtual void SetIdle(int player_id, bool is_idle) = 0;
+
+  // Get the player's idle state. A player is idle after SetIdle(true), even if
+  // it has gone stale.

[watk, 2016/12/10 00:01:31]

Sounds a bit like SetIdle can move you from stale to idle. I think you should
mention that IsStale() implies IsIdle().

[sandersd (OOO until July 31), 2017/01/05 23:12:20]

Done.

+  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(), which may allow us to eliminate the return value from
+  // OnIdleTimeout() as well.
+  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