Refactor WebMediaPlayerDelegate interface.

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 6e01840d71c6e0e05129c139d91354fd04f2f4d9..0770f0c5916590719edbd4c89d9006d6d97f8b6d 100644
--- a/media/blink/webmediaplayer_impl.h
+++ b/media/blink/webmediaplayer_impl.h
@@ -184,9 +184,10 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   void becameDominantVisibleContent(bool isDominant) override;
 
   // WebMediaPlayerDelegate::Observer implementation.
-  void OnHidden() override;
-  void OnShown() override;
-  bool OnSuspendRequested(bool must_suspend) override;
+  void OnFrameHidden() override;
+  void OnFrameClosed() override;
+  void OnFrameShown() override;
+  bool OnIdleTimeout() override;
   void OnPlay() override;
   void OnPause() override;
   void OnVolumeMultiplierUpdate(double multiplier) override;
@@ -220,20 +221,18 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // then seek to resume playback at the current position.
   void ScheduleRestart();
 
-  // Distinct states that |delegate_| can be in.
-  // TODO(sandersd): This should move into WebMediaPlayerDelegate.

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

Can it not move in?

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

It's not really necessary any more, separating the state into multiple types
fixed the main issues.

-  // (Public for testing.)
+  // Distinct states that |delegate_| can be in. (Public for testing.)
   enum class DelegateState {
     GONE,
     PLAYING,
     PAUSED,
-    ENDED,
   };
 
   // Playback state variables computed together in UpdatePlayState().
   // (Public for testing.)
   struct PlayState {
     DelegateState delegate_state;
+    bool is_idle;
     bool is_memory_reporting_enabled;
     bool is_suspended;
   };
@@ -338,7 +337,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
                                              bool is_streaming,
                                              bool is_suspended,
                                              bool is_backgrounded);
-  void SetDelegateState(DelegateState new_state);
+  void SetDelegateState(DelegateState new_state, bool is_idle);
   void SetMemoryReportingState(bool is_memory_reporting_enabled);
   void SetSuspendState(bool is_suspended);
 
@@ -353,6 +352,11 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // paused state after some idle timeout.
   void ScheduleIdlePauseTimer();
 
+  // Returns |true| before HaveFutureData whenever there has been loading
+  // progress and we have not been resumed for at least kLoadingToIdleTimeout
+  // since then.

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

I don't think this is clear if you don't understand the issue. Maybe copy some
of the comment at line 1370 LHS of wmpi.cc 

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

Done.

+  bool IsPrerollAttemptNeeded();
+
   void CreateWatchTimeReporter();
 
   // Returns true if the player is hidden.
@@ -360,16 +364,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
 
   blink::WebLocalFrame* frame_;
 
-  // The playback state last reported to |delegate_|, to avoid setting duplicate
-  // states. (Which can have undesired effects like resetting the idle timer.)
-  DelegateState delegate_state_;
-
-  // Set when OnSuspendRequested() is called with |must_suspend| unset.
-  bool is_idle_;
-
-  // Set when OnSuspendRequested() is called with |must_suspend| set.
-  bool must_suspend_;
-
   blink::WebMediaPlayer::NetworkState network_state_;
   blink::WebMediaPlayer::ReadyState ready_state_;
   blink::WebMediaPlayer::ReadyState highest_ready_state_;
@@ -556,8 +550,8 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   GURL fallback_url_;
   bool use_fallback_path_;
 
-  // Called some-time after OnHidden() if the media was suspended in a playing
-  // state as part of the call to OnHidden().
+  // Called sometime after the media is suspended in a playing state in
+  // OnFrameHidden(), causing the state to change to paused.
   base::OneShotTimer background_pause_timer_;
 
   // Monitors the watch time of the played content.
@@ -568,17 +562,18 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   int underflow_count_;
   std::unique_ptr<base::ElapsedTimer> underflow_timer_;
 
-  // The last time didLoadingProgress() returned true.
-  base::TimeTicks last_time_loading_progressed_;
+  // Used to track loading progress, used by IsPrerollAttemptNeeded().
+  // |preroll_attempt_pending_| indicates that the clock has been reset
+  // (awaiting a resume to start), while |preroll_attempt_start_time_| tracks
+  // when a preroll attempt began.
+  bool preroll_attempt_pending_;
+  base::TimeTicks preroll_attempt_start_time_;
 
   std::unique_ptr<base::TickClock> tick_clock_;
 
   // Monitors the player events.
   base::WeakPtr<MediaObserver> observer_;
 
-  // Whether the player is currently in autoplay muted state.
-  bool autoplay_muted_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(WebMediaPlayerImpl);
 };