Refactor WebMediaPlayerDelegate interface.

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 {
 
 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;
+    // Called when the host frame is hidden (usually by tab switching).
+    virtual void OnFrameHidden() = 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 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;
 
-  // 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,
+  // 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 has_video,
-                       bool has_audio,
-                       bool is_remote,
                        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;
+  // 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 the render frame is currently hidden.
-  virtual bool IsHidden() = 0;
+  // Set the player's idle state. While idle, a player may recieve an

[DaleCurtis, 2016/12/20 22:34:04]

Seems this is always called after one of the above methods? Should idleness
instead be a bit on all of those calls?

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

There is one counterexample, webmediaplayer_ms calls PlayerGone() without
calling SetIdle(). We could combine them as you say and then have
webmediaplayer_ms use PlayerGone(IsIdle()). WDYT?

[DaleCurtis, 2017/01/05 23:20:36]

Does that make sense? Isn't IsIdle() always true when PlayerGone() is called?

+  // OnIdleTimeout() callback.
+  virtual void SetIdle(int player_id, bool is_idle) = 0;
 
-  // Returns whether there's a video playing in background within the render
-  // frame.
-  virtual bool IsPlayingBackgroundVideo() = 0;
+  // Get the player's idle state. A player is idle after SetIdle(true), even if

[DaleCurtis, 2016/12/20 22:34:05]

Unused?

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

WMPI uses this in one case to basically result in SetIdle(IsIdle()).

After the planned followup to improve WMPI's computation, it should indeed be
entirely unused and will be removed. (Depending on the outcome of SetIdle()
above.)

+  // it has gone stale.
+  virtual bool IsIdle(int player_id) = 0;
+
+  // Returns a stale player to an idle state, and resumes OnIdleTimeout() calls

[DaleCurtis, 2016/12/20 22:34:04]

Hmm, don't really love exposing the stale concept outside of the delegate. If
you roll idleness into player methods and let IsIdle == IsStale (as it exists
today), does that solve the use cases?

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

(See above.)

+  // 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
 
 #endif  // MEDIA_BLINK_WEBMEDIAPLAYER_DELEGATE_H_