Add NetworkEventCB to media pipeline and remove IsNetworkActive()

Network events and the network status associated with the event (active or
in-) should be coupled together. WebMediaPlayerImpl should not ask the
PipelineImpl for network activity.

BUG=100652
TEST=none

Review URL: http://codereview.chromium.org/8342013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@106369 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 21 insertions, 32 deletions

diff --git a/media/base/filter_host.h b/media/base/filter_host.h
index d6977b6..412fdd7 100644
--- a/media/base/filter_host.h
+++ b/media/base/filter_host.h
@@ -67,7 +67,7 @@ class MEDIA_EXPORT FilterHost {
   virtual void SetLoaded(bool loaded) = 0;
 
   // Sets the flag to indicate current network activity.
-  virtual void SetNetworkActivity(bool network_activity) = 0;
+  virtual void SetNetworkActivity(bool is_downloading_data) = 0;
 
   // Disable audio renderer by calling OnAudioRendererDisabled() on all
   // filters.
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 1945010..7cf5e73 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -38,6 +38,12 @@ class FilterCollection;
 
 class MEDIA_EXPORT Pipeline : public base::RefCountedThreadSafe<Pipeline> {
  public:
+  // Callback that executes when a network event occurrs.
+  // The parameter represents the state of network activity: true if the network
+  // is downloading data, false if it is not. If the callback never runs, it is
+  // assumed the network is not downloading data.
+  typedef base::Callback<void(bool)> NetworkEventCB;
+
   // Initializes pipeline. Pipeline takes ownership of all callbacks passed
   // into this method.
   // |ended_callback| will be executed when the media reaches the end.
@@ -45,7 +51,7 @@ class MEDIA_EXPORT Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   // |network_callback_| will be executed when there's a network event.
   virtual void Init(const PipelineStatusCB& ended_callback,
                     const PipelineStatusCB& error_callback,
-                    const PipelineStatusCB& network_callback) = 0;
+                    const NetworkEventCB& network_callback) = 0;
 
   // Build a pipeline to render the given URL using the given filter collection
   // to construct a filter chain.  Returns true if successful, false otherwise
@@ -92,9 +98,6 @@ class MEDIA_EXPORT Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   // for a pipeline to be started but not initialized (i.e., an error occurred).
   virtual bool IsInitialized() const = 0;
 
-  // Returns true if there has been network activity.
-  virtual bool IsNetworkActive() const = 0;
-
   // Returns true if the media has audio.
   virtual bool HasAudio() const = 0;
 
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 1eb4416..cebab46 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -64,7 +64,6 @@ class PipelineImpl::PipelineInitState {
 PipelineImpl::PipelineImpl(MessageLoop* message_loop, MediaLog* media_log)
     : message_loop_(message_loop),
       media_log_(media_log),
-      network_activity_(false),
       clock_(new Clock(&base::Time::Now)),
       waiting_for_clock_update_(false),
       state_(kCreated),
@@ -87,7 +86,7 @@ PipelineImpl::~PipelineImpl() {
 
 void PipelineImpl::Init(const PipelineStatusCB& ended_callback,
                         const PipelineStatusCB& error_callback,
-                        const PipelineStatusCB& network_callback) {
+                        const NetworkEventCB& network_callback) {
   DCHECK(!IsRunning())
       << "Init() should be called before the pipeline has started";
   ended_callback_ = ended_callback;
@@ -169,11 +168,6 @@ bool PipelineImpl::IsInitialized() const {
   }
 }
 
-bool PipelineImpl::IsNetworkActive() const {
-  base::AutoLock auto_lock(lock_);
-  return network_activity_;
-}
-
 bool PipelineImpl::HasAudio() const {
   base::AutoLock auto_lock(lock_);
   return has_audio_;
@@ -564,18 +558,15 @@ void PipelineImpl::SetLoaded(bool loaded) {
   loaded_ = loaded;
 }
 
-void PipelineImpl::SetNetworkActivity(bool network_activity) {
+void PipelineImpl::SetNetworkActivity(bool is_downloading_data) {
   DCHECK(IsRunning());
-  {
-    base::AutoLock auto_lock(lock_);
-    network_activity_ = network_activity;
-  }
   message_loop_->PostTask(FROM_HERE,
-      base::Bind(&PipelineImpl::NotifyNetworkEventTask, this));
+      base::Bind(
+          &PipelineImpl::NotifyNetworkEventTask, this, is_downloading_data));
   media_log_->AddEvent(
       media_log_->CreateBooleanEvent(
           MediaLogEvent::NETWORK_ACTIVITY_SET,
-          "network_activity", network_activity));
+          "is_downloading_data", is_downloading_data));
 }
 
 void PipelineImpl::DisableAudioRenderer() {
@@ -935,11 +926,10 @@ void PipelineImpl::NotifyEndedTask() {
   }
 }
 
-void PipelineImpl::NotifyNetworkEventTask() {
+void PipelineImpl::NotifyNetworkEventTask(bool is_downloading_data) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  if (!network_callback_.is_null()) {
-    network_callback_.Run(status_);
-  }
+  if (!network_callback_.is_null())
+    network_callback_.Run(is_downloading_data);
 }
 
 void PipelineImpl::DisableAudioRendererTask() {
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index c4c9898..ef52ac2 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -102,7 +102,7 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline, public FilterHost {
   // Pipeline implementation.
   virtual void Init(const PipelineStatusCB& ended_callback,
                     const PipelineStatusCB& error_callback,
-                    const PipelineStatusCB& network_callback) OVERRIDE;
+                    const NetworkEventCB& network_callback) OVERRIDE;
   virtual bool Start(FilterCollection* filter_collection,
                      const std::string& uri,
                      const PipelineStatusCB& start_callback) OVERRIDE;
@@ -111,7 +111,6 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline, public FilterHost {
                     const PipelineStatusCB& seek_callback) OVERRIDE;
   virtual bool IsRunning() const OVERRIDE;
   virtual bool IsInitialized() const OVERRIDE;
-  virtual bool IsNetworkActive() const OVERRIDE;
   virtual bool HasAudio() const OVERRIDE;
   virtual bool HasVideo() const OVERRIDE;
   virtual float GetPlaybackRate() const OVERRIDE;
@@ -202,7 +201,7 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline, public FilterHost {
   virtual void SetNaturalVideoSize(const gfx::Size& size) OVERRIDE;
   virtual void SetStreaming(bool streamed) OVERRIDE;
   virtual void SetLoaded(bool loaded) OVERRIDE;
-  virtual void SetNetworkActivity(bool network_activity) OVERRIDE;
+  virtual void SetNetworkActivity(bool is_downloading_data) OVERRIDE;
   virtual void NotifyEnded() OVERRIDE;
   virtual void DisableAudioRenderer() OVERRIDE;
   virtual void SetCurrentReadPosition(int64 offset) OVERRIDE;
@@ -262,7 +261,7 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline, public FilterHost {
   void NotifyEndedTask();
 
   // Carries out handling a notification of network event.
-  void NotifyNetworkEventTask();
+  void NotifyNetworkEventTask(bool is_downloading_data);
 
   // Carries out disabling the audio renderer.
   void DisableAudioRendererTask();
@@ -383,9 +382,6 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline, public FilterHost {
   // loaded source.
   bool loaded_;
 
-  // Sets by the filters to indicate whether network is active.
-  bool network_activity_;
-
   // Current volume level (from 0.0f to 1.0f).  This value is set immediately
   // via SetVolume() and a task is dispatched on the message loop to notify the
   // filters.
@@ -457,7 +453,7 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline, public FilterHost {
   PipelineStatusCB stop_callback_;
   PipelineStatusCB ended_callback_;
   PipelineStatusCB error_callback_;
-  PipelineStatusCB network_callback_;
+  NetworkEventCB network_callback_;
 
   // Reference to the filter(s) that constitute the pipeline.
   scoped_refptr<Filter> pipeline_filter_;
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index 4f7fb51..d6b5068 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -65,7 +65,7 @@ class PipelineImplTest : public ::testing::Test {
     pipeline_->Init(
         base::Bind(&CallbackHelper::OnEnded, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnError, base::Unretained(&callbacks_)),
-        PipelineStatusCB());
+        Pipeline::NetworkEventCB());
     mocks_.reset(new MockFilterCollection());
 
     // InitializeDemuxer adds overriding expectations for expected non-NULL