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..300138d993b6ba445b8beee7d7b1fe21f9c7676d 100644
--- a/media/blink/webmediaplayer_delegate.h
+++ b/media/blink/webmediaplayer_delegate.h
@@ -12,73 +12,83 @@ 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:
   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().
+    // Called when the host frame is hidden (usually by tab switching).
     virtual void OnHidden() = 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 OnClosed() = 0;
+
+    // Called when the host frame is shown (usually by tab switching).
     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 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.
+    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.
+  virtual bool IsHidden() = 0;
+  virtual bool IsClosed() = 0;

[watk, 2016/11/08 23:34:47]

I wonder if all the {on,is}{hidden,shown} should have frame in the name:
"IsFrameHidden". So it's clear it's not related to visibility on the page.

[sandersd (OOO until July 31), 2016/11/08 23:44:44]

Done.

 
-  // Subscribe or unsubscribe from observer callbacks respectively. A client
-  // must use the delegate id returned by AddObserver() for all other calls.
+  // Returns |true| if there is any player with playing video in the host frame.
+  virtual bool IsPlayingVideo() = 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,

[watk, 2016/11/08 23:34:47]

I like player_id a lot

                        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;
+  // 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;
 
-  // 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;
+  // 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;
 
-  // Returns whether there's a video playing in background within the render
-  // frame.
-  virtual bool IsPlayingBackgroundVideo() = 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;
 
  protected:
-  virtual ~WebMediaPlayerDelegate() {}
+  WebMediaPlayerDelegate() = default;
+  virtual ~WebMediaPlayerDelegate() = default;
 };
 
 }  // namespace media