Suspend idle WebMediaPlayer instances after some time.

content/renderer/media/renderer_webmediaplayer_delegate.cc

Index: content/renderer/media/renderer_webmediaplayer_delegate.cc
diff --git a/content/renderer/media/renderer_webmediaplayer_delegate.cc b/content/renderer/media/renderer_webmediaplayer_delegate.cc
index 9788a6545c0196358c3bab81003e492390fed4d6..748bd9958a8d0462af54d2d87f9c22aec40a4ffe 100644
--- a/content/renderer/media/renderer_webmediaplayer_delegate.cc
+++ b/content/renderer/media/renderer_webmediaplayer_delegate.cc
@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 
+#include "base/auto_reset.h"
 #include "content/common/media/media_player_delegate_messages.h"
 #include "content/public/renderer/render_frame.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayer.h"
@@ -14,17 +15,30 @@ namespace media {
 
 RendererWebMediaPlayerDelegate::RendererWebMediaPlayerDelegate(
     content::RenderFrame* render_frame)
-    : RenderFrameObserver(render_frame) {}
+    : RenderFrameObserver(render_frame),
+      default_tick_clock_(new base::DefaultTickClock()),
+      tick_clock_(default_tick_clock_.get()) {
+#if defined(OS_ANDROID)
+  // On Android the idle cleanup timer is enabled by default.
+  // TODO(dalecurtis): Eventually this should be enabled on all platforms.
+  idle_cleanup_enabled_ = true;
+  idle_cleanup_interval_ = base::TimeDelta::FromSeconds(5);
+  idle_timeout_ = base::TimeDelta::FromSeconds(15);
+#endif
+}
 
 RendererWebMediaPlayerDelegate::~RendererWebMediaPlayerDelegate() {}
 
 int RendererWebMediaPlayerDelegate::AddObserver(Observer* observer) {
-  return id_map_.Add(observer);
+  const int delegate_id = id_map_.Add(observer);
+  AddIdleDelegate(delegate_id);
+  return delegate_id;
 }
 
 void RendererWebMediaPlayerDelegate::RemoveObserver(int delegate_id) {
   DCHECK(id_map_.Lookup(delegate_id));
   id_map_.Remove(delegate_id);
+  RemoveIdleDelegate(delegate_id);
 }
 
 void RendererWebMediaPlayerDelegate::DidPlay(int delegate_id,
@@ -34,6 +48,7 @@ void RendererWebMediaPlayerDelegate::DidPlay(int delegate_id,
                                              base::TimeDelta duration) {
   DCHECK(id_map_.Lookup(delegate_id));
   has_played_media_ = true;
+  RemoveIdleDelegate(delegate_id);
   Send(new MediaPlayerDelegateHostMsg_OnMediaPlaying(
       routing_id(), delegate_id, has_video, has_audio, is_remote, duration));
 }
@@ -41,12 +56,14 @@ void RendererWebMediaPlayerDelegate::DidPlay(int delegate_id,
 void RendererWebMediaPlayerDelegate::DidPause(int delegate_id,
                                               bool reached_end_of_stream) {
   DCHECK(id_map_.Lookup(delegate_id));
+  AddIdleDelegate(delegate_id);
   Send(new MediaPlayerDelegateHostMsg_OnMediaPaused(routing_id(), delegate_id,
                                                     reached_end_of_stream));
 }
 
 void RendererWebMediaPlayerDelegate::PlayerGone(int delegate_id) {
   DCHECK(id_map_.Lookup(delegate_id));
+  RemoveIdleDelegate(delegate_id);
   Send(new MediaPlayerDelegateHostMsg_OnMediaDestroyed(routing_id(),
                                                        delegate_id));
 }
@@ -69,17 +86,26 @@ bool RendererWebMediaPlayerDelegate::OnMessageReceived(
     const IPC::Message& msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RendererWebMediaPlayerDelegate, msg)
-  IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_Pause, OnMediaDelegatePause)
-  IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_Play, OnMediaDelegatePlay)
-  IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_SuspendAllMediaPlayers,
-                      OnMediaDelegateSuspendAllMediaPlayers)
-  IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_UpdateVolumeMultiplier,
-                      OnMediaDelegateVolumeMultiplierUpdate)
-  IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_Pause, OnMediaDelegatePause)
+    IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_Play, OnMediaDelegatePlay)
+    IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_SuspendAllMediaPlayers,
+                        OnMediaDelegateSuspendAllMediaPlayers)
+    IPC_MESSAGE_HANDLER(MediaPlayerDelegateMsg_UpdateVolumeMultiplier,
+                        OnMediaDelegateVolumeMultiplierUpdate)
+    IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
+void RendererWebMediaPlayerDelegate::EnableInstantIdleCleanupForTesting(
+    base::TimeDelta idle_timeout,
+    base::TickClock* tick_clock) {
+  idle_cleanup_enabled_ = true;
+  idle_cleanup_interval_ = base::TimeDelta();
+  idle_timeout_ = idle_timeout;
+  tick_clock_ = tick_clock;
+}
+
 void RendererWebMediaPlayerDelegate::OnMediaDelegatePause(int delegate_id) {
   Observer* observer = id_map_.Lookup(delegate_id);
   if (observer)
@@ -105,4 +131,54 @@ void RendererWebMediaPlayerDelegate::OnMediaDelegateVolumeMultiplierUpdate(
     observer->OnVolumeMultiplierUpdate(multiplier);
 }
 
+void RendererWebMediaPlayerDelegate::AddIdleDelegate(int delegate_id) {
+  if (!idle_cleanup_enabled_)
+    return;
+
+  idle_delegate_map_[delegate_id] = tick_clock_->NowTicks();
+  if (!idle_cleanup_timer_.IsRunning()) {
+    idle_cleanup_timer_.Start(
+        FROM_HERE, idle_cleanup_interval_, this,
+        &RendererWebMediaPlayerDelegate::CleanupIdleDelegates);
+  }
+}
+
+void RendererWebMediaPlayerDelegate::RemoveIdleDelegate(int delegate_id) {
+  if (!idle_cleanup_enabled_)
+    return;
+
+  // To avoid invalidating the iterator, just mark the delegate for deletion
+  // using a sentinel value of an empty TimeTicks.

[sandersd (OOO until July 31), 2016/02/27 00:22:20]

Please comment on which thread methods run on such that this can be a problem.

[DaleCurtis, 2016/02/27 00:55:23]

Added a comment below about how calls might trigger during idle cleanup.

+  if (idle_cleanup_running_) {
+    idle_delegate_map_[delegate_id] = base::TimeTicks();
+    return;
+  }
+
+  idle_delegate_map_.erase(delegate_id);
+  if (idle_delegate_map_.empty())
+    idle_cleanup_timer_.Stop();
+}
+
+void RendererWebMediaPlayerDelegate::CleanupIdleDelegates() {
+  base::AutoReset<bool> scoper(&idle_cleanup_running_, true);
+
+  const base::TimeTicks now = tick_clock_->NowTicks();
+  for (const auto& kv : idle_delegate_map_) {
+    if (now - kv.second > idle_timeout_)
+      id_map_.Lookup(kv.first)->OnHidden(true);

[sandersd (OOO until July 31), 2016/02/27 00:22:20]

I'm going to keep commenting that this is the wrong method and/or method name on
every CL.

[DaleCurtis, 2016/02/27 00:55:23]

Ignoring per offline discussion.

+  }
+
+  // Take care of any removals that happened during the above iteration.
+  for (auto it = idle_delegate_map_.begin(); it != idle_delegate_map_.end();) {
+    if (it->second.is_null())
+      it = idle_delegate_map_.erase(it);

[sandersd (OOO until July 31), 2016/02/27 00:22:20]

It looks like this can race with line 153.

[DaleCurtis, 2016/02/27 00:55:23]

Ignoring per offline discussion.

+    else
+      ++it;
+  }
+
+  // Shutdown the timer if no delegates are left.
+  if (idle_delegate_map_.empty())
+    idle_cleanup_timer_.Stop();
+}
+
 }  // namespace media