Expand suspension of idle media players to all platforms.

media/blink/webmediaplayer_impl.h

Index: media/blink/webmediaplayer_impl.h
diff --git a/media/blink/webmediaplayer_impl.h b/media/blink/webmediaplayer_impl.h
index 0283efb40fad6819c0b6a47852e72994de7f3bf2..ee3198a03e4c60503c755575411b018de64ac7ad 100644
--- a/media/blink/webmediaplayer_impl.h
+++ b/media/blink/webmediaplayer_impl.h
@@ -167,8 +167,9 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   void exitedFullscreen() override;
 
   // WebMediaPlayerDelegate::Observer implementation.
-  void OnHidden(bool must_suspend) override;
+  void OnHidden() override;
   void OnShown() override;
+  void OnSuspend(bool must_suspend) override;

[sandersd (OOO until July 31), 2016/03/07 23:27:51]

I keep getting tripped up on this bool, so +1 for an enum here.

[DaleCurtis, 2016/03/09 02:56:14]

I kept going around on this and can't find names I liked. So instead I've kept
the bool but renamed this to OnSuspendRequested().

   void OnPlay() override;
   void OnPause() override;
   void OnVolumeMultiplierUpdate(double multiplier) override;
@@ -279,6 +280,9 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   void ReportMemoryUsage();
   void FinishMemoryUsageReport(int64_t demuxer_memory_usage);
 
+  // Indicates if Resume() or Suspend() are allowed to be called respectively.

[sandersd (OOO until July 31), 2016/03/07 23:27:51]

I'm not clear on this comment given that there is only one method.

Perhaps better would be "Indicates if automatic resumption is allowed in the
current state", or something similar that indicates when this method is called.
(Without that the name sounds like it should be const.)

[DaleCurtis, 2016/03/09 02:56:14]

Done.

+  bool IsResumeAllowed();
+
   blink::WebLocalFrame* frame_;
 
   // TODO(hclam): get rid of these members and read from the pipeline directly.
@@ -421,6 +425,15 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
 
   scoped_ptr<RendererFactory> renderer_factory_;
 
+  // Indicates if playback may be started from the background via a to play from

[mlamouri (slow - plz ping), 2016/03/05 22:37:36]

nit: "via a to play from JavaScript" might be missing "call"? (and maybe "play"
should be "play()")

+  // JavaScript or the media session.
+  enum class BackgroundPlaybackState {
+    PLAY_FORBIDDEN_NEVER_CALLED_IN_FOREGROUND,

[xhwang, 2016/03/07 19:13:58]

Can you provide more comments around these states? It's not super obvious to me,
especially the "FORBIDDEN" part here.

[DaleCurtis, 2016/03/07 19:23:07]

Yeah my names are bad :( I was trying to capture "playback has never been
started in the foreground before, so don't let autoplay / js actually start
playback in the background."  This is similar to what we do where we never load
the media until the page is in the foreground unless playback has ever started
in that tab before.

The others are essentially "don't play video in the background" and "play
whatever you want in response to autoplay / js starting playback."

[DaleCurtis, 2016/03/09 02:56:14]

After some more thought, I don't need these values. Removed.

+    PLAY_FORBIDDEN_WHEN_SUSPENDED,
+    PLAY_ALLOWED_WHEN_SUSPENDED,
+  };
+  BackgroundPlaybackState background_playback_state_;
+
   // For requesting surfaces on behalf of the Android H/W decoder in fullscreen.
   // This will be null everywhere but Android.
   SurfaceManager* surface_manager_;