Refactor WebMediaPlayerDelegate interface.

media/blink/webmediaplayer_impl.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_IMPL_H_
 #define MEDIA_BLINK_WEBMEDIAPLAYER_IMPL_H_
 
 #include <stdint.h>
 
 #include <memory>
@@ skipping 166 matching lines @@
   void setContentDecryptionModule(
       blink::WebContentDecryptionModule* cdm,
       blink::WebContentDecryptionModuleResult result) override;
 
   bool supportsOverlayFullscreenVideo() override;
   void enteredFullscreen() override;
   void exitedFullscreen() override;
   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;
 
   void requestRemotePlaybackDisabled(bool disabled) override;
 #if defined(OS_ANDROID)  // WMPI_CAST
   bool isRemote() const override;
   void requestRemotePlayback() override;
   void requestRemotePlaybackControl() override;
   void requestRemotePlaybackStop() override;
@@ skipping 13 matching lines @@
 
   // Called from WebMediaPlayerCast.
   // TODO(hubbe): WMPI_CAST make private.
   void OnPipelineSeeked(bool time_updated);
 
   // Restart the player/pipeline as soon as possible. This will destroy the
   // current renderer, if any, and create a new one via the RendererFactory; and
   // 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;
   };
 
  private:
   friend class WebMediaPlayerImplTest;
 
   void EnableOverlay();
   void DisableOverlay();
 
@@ skipping 84 matching lines @@
   //   - is_idle_, must_suspend_,
   //   - paused_, ended_,
   //   - pending_suspend_resume_cycle_,
   void UpdatePlayState();
 
   // Methods internal to UpdatePlayState().
   PlayState UpdatePlayState_ComputePlayState(bool is_remote,
                                              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);
 
   // Called at low frequency to tell external observers how much memory we're
   // using for video playback.  Called by |memory_usage_reporting_timer_|.
   // Memory usage reporting is done in two steps, because |demuxer_| must be
   // accessed on the media thread.
   void ReportMemoryUsage();
   void FinishMemoryUsageReport(int64_t demuxer_memory_usage);
 
   // Called during OnHidden() when we want a suspended player to enter the
   // 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.
   bool IsHidden() const;
 
   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_;
 
   // Preload state for when |data_source_| is created after setPreload().
   MultibufferDataSource::Preload preload_;
 
   // Buffering strategy for when |data_source_| is created after
   // setBufferingStrategy().
   MultibufferDataSource::BufferingStrategy buffering_strategy_;
@@ skipping 166 matching lines @@
   // from the compositor will be queried to see if suspend is supported; the
   // state will be set to YES or NO respectively if a frame is available.
   enum class CanSuspendState { UNKNOWN, YES, NO };
   CanSuspendState can_suspend_state_;
 
   // Used for HLS playback and in certain fallback paths (e.g. on older devices
   // that can't support the unified media pipeline).
   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.
   std::unique_ptr<WatchTimeReporter> watch_time_reporter_;
   bool is_encrypted_;
 
   // Number of times we've reached BUFFERING_HAVE_NOTHING during playback.
   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);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BLINK_WEBMEDIAPLAYER_IMPL_H_