Remove use of PipelineStatus from BufferedDataSource and FileDataSource.

You know what's better than passing back PIPELINE_OK and not-PIPELINE_OK? A boolean.

BUG=126067
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10808087

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

media/base/data_buffer.cc

@@ -2,9 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/logging.h"
 #include "media/base/data_buffer.h"
 
+#include "base/logging.h"
+
 namespace media {
 
 DataBuffer::DataBuffer(scoped_array<uint8> buffer, int buffer_size)

media/filters/ffmpeg_demuxer_unittest.cc

@@ -182,7 +182,7 @@ class FFmpegDemuxerTest : public testing::Test {
         .AppendASCII(name);
 
     data_source_ = new FileDataSource(disable_file_size);
-    EXPECT_EQ(PIPELINE_OK, data_source_->Initialize(file_path.MaybeAsASCII()));
+    EXPECT_TRUE(data_source_->Initialize(file_path.MaybeAsASCII()));
   }
 
   DISALLOW_COPY_AND_ASSIGN(FFmpegDemuxerTest);

media/filters/file_data_source.cc

@@ -2,13 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "media/filters/file_data_source.h"
+
 #include <limits>
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
-#include "media/base/pipeline.h"
-#include "media/filters/file_data_source.h"
 
 namespace media {
 
@@ -24,7 +24,7 @@ FileDataSource::FileDataSource(bool disable_file_size)
       disable_file_size_(disable_file_size) {
 }
 
-PipelineStatus FileDataSource::Initialize(const std::string& url) {
+bool FileDataSource::Initialize(const std::string& url) {
   DCHECK(!file_);
 #if defined(OS_WIN)
   FilePath file_path(UTF8ToWide(url));
@@ -36,11 +36,11 @@ PipelineStatus FileDataSource::Initialize(const std::string& url) {
   }
   if (!file_) {
     file_size_ = 0;
-    return PIPELINE_ERROR_URL_NOT_FOUND;
+    return false;
   }
   UpdateHostBytes();
 
-  return PIPELINE_OK;
+  return true;
 }
 
 void FileDataSource::set_host(DataSourceHost* host) {

media/filters/file_data_source.h

@@ -20,7 +20,7 @@ class MEDIA_EXPORT FileDataSource : public DataSource {
   FileDataSource();
   FileDataSource(bool disable_file_size);
 
-  PipelineStatus Initialize(const std::string& url);
+  bool Initialize(const std::string& url);
 
   // Implementation of DataSource.
   virtual void set_host(DataSourceHost* host) OVERRIDE;

media/filters/file_data_source_unittest.cc

@@ -56,7 +56,7 @@ TEST(FileDataSourceTest, OpenFile) {
 
   scoped_refptr<FileDataSource> filter(new FileDataSource());
   filter->set_host(&host);
-  EXPECT_EQ(PIPELINE_OK, filter->Initialize(TestFileURL()));
+  EXPECT_TRUE(filter->Initialize(TestFileURL()));
 
   filter->Stop(NewExpectedClosure());
 }
@@ -71,7 +71,7 @@ TEST(FileDataSourceTest, ReadData) {
   scoped_refptr<FileDataSource> filter(new FileDataSource());
 
   filter->set_host(&host);
-  EXPECT_EQ(PIPELINE_OK, filter->Initialize(TestFileURL()));
+  EXPECT_TRUE(filter->Initialize(TestFileURL()));
 
   EXPECT_TRUE(filter->GetSize(&size));
   EXPECT_EQ(10, size);

media/filters/pipeline_integration_test_base.cc

@@ -170,7 +170,7 @@ bool PipelineIntegrationTestBase::WaitUntilCurrentTimeIsAfter(
 scoped_ptr<FilterCollection>
 PipelineIntegrationTestBase::CreateFilterCollection(const std::string& url) {
   scoped_refptr<FileDataSource> data_source = new FileDataSource();
-  CHECK_EQ(PIPELINE_OK, data_source->Initialize(url));
+  CHECK(data_source->Initialize(url));
   return CreateFilterCollection(new FFmpegDemuxer(&message_loop_, data_source));
 }
 

media/tools/player_wtl/movie.cc

@@ -66,7 +66,7 @@ bool Movie::Open(const wchar_t* url, VideoRendererBase* video_renderer) {
   // Open the file.
   std::string url_utf8 = WideToUTF8(string16(url));
   scoped_refptr<FileDataSource> data_source = new FileDataSource();
-  if (data_source->Initialize(url_utf8) != PIPELINE_OK) {
+  if (!data_source->Initialize(url_utf8)) {
     return false;
   }
 

media/tools/player_x11/player_x11.cc

@@ -50,7 +50,7 @@ scoped_refptr<media::FileDataSource> CreateFileDataSource(
     const std::string& file) {
   scoped_refptr<media::FileDataSource> file_data_source(
       new media::FileDataSource());
-  CHECK_EQ(file_data_source->Initialize(file), media::PIPELINE_OK);
+  CHECK(file_data_source->Initialize(file));
   return file_data_source;
 }
 

media/tools/seek_tester/seek_tester.cc

@@ -61,7 +61,7 @@ int main(int argc, char** argv) {
   CHECK(base::StringToUint64(argv[2], &seek_target_ms));
   scoped_refptr<media::FileDataSource> file_data_source(
       new media::FileDataSource());
-  CHECK_EQ(file_data_source->Initialize(argv[1]), media::PIPELINE_OK);
+  CHECK(file_data_source->Initialize(argv[1]));
 
   DemuxerHostImpl host;
   MessageLoop loop;

webkit/media/buffered_data_source.cc

@@ -5,6 +5,7 @@
 #include "webkit/media/buffered_data_source.h"
 
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/message_loop.h"
 #include "media/base/media_log.h"
 #include "net/base/net_errors.h"
@@ -89,14 +90,14 @@ void BufferedDataSource::set_host(media::DataSourceHost* host) {
 void BufferedDataSource::Initialize(
     const GURL& url,
     BufferedResourceLoader::CORSMode cors_mode,
-    const media::PipelineStatusCB& initialize_cb) {
+    const InitializeCB& init_cb) {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(!initialize_cb.is_null());
+  DCHECK(!init_cb.is_null());
   DCHECK(!loader_.get());
   url_ = url;
   cors_mode_ = cors_mode;
 
-  initialize_cb_ = initialize_cb;
+  init_cb_ = init_cb;
 
   if (url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme)) {
     // Do an unbounded range request starting at the beginning.  If the server
@@ -125,7 +126,7 @@ void BufferedDataSource::SetPreload(Preload preload) {
 
 bool BufferedDataSource::HasSingleOrigin() {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(initialize_cb_.is_null() && loader_.get())
+  DCHECK(init_cb_.is_null() && loader_.get())
       << "Initialize() must complete before calling HasSingleOrigin()";
   return loader_->HasSingleOrigin();
 }
@@ -232,7 +233,7 @@ void BufferedDataSource::CleanupTask() {
 
   {
     base::AutoLock auto_lock(lock_);
-    initialize_cb_.Reset();
+    init_cb_.Reset();
     if (stopped_on_render_loop_)
       return;
 
@@ -322,16 +323,6 @@ void BufferedDataSource::DoneRead_Locked(int bytes_read) {
   read_buffer_ = 0;
 }
 
-void BufferedDataSource::DoneInitialization_Locked(
-    media::PipelineStatus status) {
-  DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(!initialize_cb_.is_null());
-  lock_.AssertAcquired();
-
-  initialize_cb_.Run(status);
-  initialize_cb_.Reset();
-}
-
 /////////////////////////////////////////////////////////////////////////////
 // BufferedResourceLoader callback methods.
 void BufferedDataSource::StartCallback(
@@ -339,12 +330,12 @@ void BufferedDataSource::StartCallback(
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
-  bool initialize_cb_is_null = false;
+  bool init_cb_is_null = false;
   {
     base::AutoLock auto_lock(lock_);
-    initialize_cb_is_null = initialize_cb_.is_null();
+    init_cb_is_null = init_cb_.is_null();
   }
-  if (initialize_cb_is_null) {
+  if (init_cb_is_null) {
     loader_->Stop();
     return;
   }
@@ -371,13 +362,10 @@ void BufferedDataSource::StartCallback(
     if (stop_signal_received_)
       return;
 
-    if (!success) {
-      DoneInitialization_Locked(media::PIPELINE_ERROR_NETWORK);
-      return;
-    }
-
+    if (success)
       UpdateHostState_Locked();
-    DoneInitialization_Locked(media::PIPELINE_OK);
+
+    base::ResetAndReturn(&init_cb_).Run(success);
   }
 }
 

webkit/media/buffered_data_source.h

@@ -12,7 +12,6 @@
 #include "base/synchronization/lock.h"
 #include "googleurl/src/gurl.h"
 #include "media/base/data_source.h"
-#include "media/base/pipeline_status.h"
 #include "webkit/media/buffered_resource_loader.h"
 #include "webkit/media/preload.h"
 
@@ -40,14 +39,15 @@ class BufferedDataSource : public media::DataSource {
                      media::MediaLog* media_log,
                      const DownloadingCB& downloading_cb);
 
-  // Initialize this object using |url| and |cors_mode|, and call |status_cb|
-  // when initialization has completed.
+  // Initialize this object using |url| and |cors_mode|, executing |init_cb|
+  // with the result of initialization when it has completed.
   //
   // Method called on the render thread.
+  typedef base::Callback<void(bool)> InitializeCB;
   void Initialize(
       const GURL& url,
       BufferedResourceLoader::CORSMode cors_mode,
-      const media::PipelineStatusCB& status_cb);
+      const InitializeCB& init_cb);
 
   // Adjusts the buffering algorithm based on the given preload value.
   void SetPreload(Preload preload);
@@ -117,9 +117,6 @@ class BufferedDataSource : public media::DataSource {
   // kReadError.
   void DoneRead_Locked(int bytes_read);
 
-  // Calls |initialize_cb_| and reset it.
-  void DoneInitialization_Locked(media::PipelineStatus status);
-
   // BufferedResourceLoader::Start() callback for initial load.
   void StartCallback(BufferedResourceLoader::Status status);
 
@@ -159,7 +156,7 @@ class BufferedDataSource : public media::DataSource {
   scoped_ptr<BufferedResourceLoader> loader_;
 
   // Callback method from the pipeline for initialization.
-  media::PipelineStatusCB initialize_cb_;
+  InitializeCB init_cb_;
 
   // Read parameters received from the Read() method call.
   media::DataSource::ReadCB read_cb_;
@@ -185,7 +182,7 @@ class BufferedDataSource : public media::DataSource {
   MessageLoop* render_loop_;
 
   // Protects |stop_signal_received_|, |stopped_on_render_loop_| and
-  // |initialize_cb_|.
+  // |init_cb_|.
   base::Lock lock_;
 
   // Stop signal to suppressing activities. This variable is set on the pipeline

webkit/media/buffered_data_source_unittest.cc

@@ -104,14 +104,17 @@ class BufferedDataSourceTest : public testing::Test {
     view_->close();
   }
 
-  void Initialize(const char* url, media::PipelineStatus expected) {
+  MOCK_METHOD1(OnInitialize, void(bool));
+
+  void Initialize(const char* url, bool expected) {
     GURL gurl(url);
     response_generator_.reset(new TestResponseGenerator(gurl, kFileSize));
 
     ExpectCreateResourceLoader();
-    data_source_->Initialize(gurl,
-                             BufferedResourceLoader::kUnspecified,
-                             media::NewExpectedStatusCB(expected));
+    EXPECT_CALL(*this, OnInitialize(expected));
+    data_source_->Initialize(
+        gurl, BufferedResourceLoader::kUnspecified, base::Bind(
+            &BufferedDataSourceTest::OnInitialize, base::Unretained(this)));
     message_loop_.RunAllPending();
 
     bool is_http = gurl.SchemeIs(kHttpScheme) || gurl.SchemeIs(kHttpsScheme);
@@ -120,7 +123,7 @@ class BufferedDataSourceTest : public testing::Test {
 
   // Helper to initialize tests with a valid 206 response.
   void InitializeWith206Response() {
-    Initialize(kHttpUrl, media::PIPELINE_OK);
+    Initialize(kHttpUrl, true);
 
     EXPECT_CALL(host_, SetTotalBytes(response_generator_->content_length()));
     Respond(response_generator_->Generate206(0));
@@ -128,7 +131,7 @@ class BufferedDataSourceTest : public testing::Test {
 
   // Helper to initialize tests with a valid file:// response.
   void InitializeWithFileResponse() {
-    Initialize(kFileUrl, media::PIPELINE_OK);
+    Initialize(kFileUrl, true);
 
     EXPECT_CALL(host_, SetTotalBytes(kFileSize));
     EXPECT_CALL(host_, AddBufferedByteRange(0, kFileSize));
@@ -218,7 +221,7 @@ class BufferedDataSourceTest : public testing::Test {
 };
 
 TEST_F(BufferedDataSourceTest, Range_Supported) {
-  Initialize(kHttpUrl, media::PIPELINE_OK);
+  Initialize(kHttpUrl, true);
 
   EXPECT_CALL(host_, SetTotalBytes(response_generator_->content_length()));
   Respond(response_generator_->Generate206(0));
@@ -229,7 +232,7 @@ TEST_F(BufferedDataSourceTest, Range_Supported) {
 }
 
 TEST_F(BufferedDataSourceTest, Range_InstanceSizeUnknown) {
-  Initialize(kHttpUrl, media::PIPELINE_OK);
+  Initialize(kHttpUrl, true);
 
   Respond(response_generator_->Generate206(
       0, TestResponseGenerator::kNoContentRangeInstanceSize));
@@ -240,7 +243,7 @@ TEST_F(BufferedDataSourceTest, Range_InstanceSizeUnknown) {
 }
 
 TEST_F(BufferedDataSourceTest, Range_NotFound) {
-  Initialize(kHttpUrl, media::PIPELINE_ERROR_NETWORK);
+  Initialize(kHttpUrl, false);
   Respond(response_generator_->Generate404());
 
   EXPECT_FALSE(data_source_->loading());
@@ -248,7 +251,7 @@ TEST_F(BufferedDataSourceTest, Range_NotFound) {
 }
 
 TEST_F(BufferedDataSourceTest, Range_NotSupported) {
-  Initialize(kHttpUrl, media::PIPELINE_OK);
+  Initialize(kHttpUrl, true);
   EXPECT_CALL(host_, SetTotalBytes(response_generator_->content_length()));
   Respond(response_generator_->Generate200());
 
@@ -260,7 +263,7 @@ TEST_F(BufferedDataSourceTest, Range_NotSupported) {
 // Special carve-out for Apache versions that choose to return a 200 for
 // Range:0- ("because it's more efficient" than a 206)
 TEST_F(BufferedDataSourceTest, Range_SupportedButReturned200) {
-  Initialize(kHttpUrl, media::PIPELINE_OK);
+  Initialize(kHttpUrl, true);
   EXPECT_CALL(host_, SetTotalBytes(response_generator_->content_length()));
   WebURLResponse response = response_generator_->Generate200();
   response.setHTTPHeaderField(WebString::fromUTF8("Accept-Ranges"),
@@ -273,7 +276,7 @@ TEST_F(BufferedDataSourceTest, Range_SupportedButReturned200) {
 }
 
 TEST_F(BufferedDataSourceTest, Range_MissingContentRange) {
-  Initialize(kHttpUrl, media::PIPELINE_ERROR_NETWORK);
+  Initialize(kHttpUrl, false);
   Respond(response_generator_->Generate206(
       0, TestResponseGenerator::kNoContentRange));
 
@@ -282,7 +285,7 @@ TEST_F(BufferedDataSourceTest, Range_MissingContentRange) {
 }
 
 TEST_F(BufferedDataSourceTest, Range_MissingContentLength) {
-  Initialize(kHttpUrl, media::PIPELINE_OK);
+  Initialize(kHttpUrl, true);
 
   // It'll manage without a Content-Length response.
   EXPECT_CALL(host_, SetTotalBytes(response_generator_->content_length()));
@@ -295,7 +298,7 @@ TEST_F(BufferedDataSourceTest, Range_MissingContentLength) {
 }
 
 TEST_F(BufferedDataSourceTest, Range_WrongContentRange) {
-  Initialize(kHttpUrl, media::PIPELINE_ERROR_NETWORK);
+  Initialize(kHttpUrl, false);
 
   // Now it's done and will fail.
   Respond(response_generator_->Generate206(1337));
@@ -452,7 +455,7 @@ TEST_F(BufferedDataSourceTest, File_TooManyRetries) {
 }
 
 TEST_F(BufferedDataSourceTest, File_InstanceSizeUnknown) {
-  Initialize(kFileUrl, media::PIPELINE_ERROR_NETWORK);
+  Initialize(kFileUrl, false);
   EXPECT_FALSE(data_source_->downloading());
 
   Respond(response_generator_->GenerateFileResponse(-1));

webkit/media/webmediaplayer_impl.cc

@@ -951,13 +951,10 @@ void WebMediaPlayerImpl::SetOpaque(bool opaque) {
   GetClient()->setOpaque(opaque);
 }
 
-void WebMediaPlayerImpl::DataSourceInitialized(
-    const GURL& gurl,
-    media::PipelineStatus status) {
+void WebMediaPlayerImpl::DataSourceInitialized(const GURL& gurl, bool success) {
   DCHECK_EQ(main_loop_, MessageLoop::current());
 
-  if (status != media::PIPELINE_OK) {
-    DVLOG(1) << "DataSourceInitialized status: " << status;
+  if (!success) {
     SetNetworkState(WebMediaPlayer::NetworkStateFormatError);
     Repaint();
     return;

webkit/media/webmediaplayer_impl.h

@@ -256,7 +256,7 @@ class WebMediaPlayerImpl
 
  private:
   // Called after asynchronous initialization of a data source completed.
-  void DataSourceInitialized(const GURL& gurl, media::PipelineStatus status);
+  void DataSourceInitialized(const GURL& gurl, bool success);
 
   // Called when the data source is downloading or paused.
   void NotifyDownloading(bool is_downloading);