Migrate DataSourceFactory to new callback system.

BUG=90214
TEST=BufferedDataSourceTest.*, SimpleDataSourceTest.*

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

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

9 files changed, 83 insertions, 128 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index c125dd6..6550fb5 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -46,7 +46,6 @@ BufferedDataSource::BufferedDataSource(
       frame_(frame),
       loader_(NULL),
       network_activity_(false),
-      initialize_callback_(NULL),
       read_callback_(NULL),
       read_position_(0),
       read_size_(0),
@@ -85,23 +84,23 @@ void BufferedDataSource::set_host(media::FilterHost* host) {
 }
 
 void BufferedDataSource::Initialize(const std::string& url,
-                                    media::PipelineStatusCallback* callback) {
+                                    const media::PipelineStatusCB& callback) {
+  DCHECK(!callback.is_null());
+
   // Saves the url.
   url_ = GURL(url);
 
   // This data source doesn't support data:// protocol so reject it.
   if (url_.SchemeIs(kDataScheme)) {
-    callback->Run(media::DATASOURCE_ERROR_URL_NOT_SUPPORTED);
-    delete callback;
+    callback.Run(media::DATASOURCE_ERROR_URL_NOT_SUPPORTED);
     return;
   } else if (!IsProtocolSupportedForMedia(url_)) {
-    callback->Run(media::PIPELINE_ERROR_NETWORK);
-    delete callback;
+    callback.Run(media::PIPELINE_ERROR_NETWORK);
     return;
   }
 
-  DCHECK(callback);
-  initialize_callback_.reset(callback);
+  DCHECK(initialize_cb_.is_null());
+  initialize_cb_ = callback;
 
   // Post a task to complete the initialization task.
   render_loop_->PostTask(FROM_HERE,
@@ -110,9 +109,9 @@ void BufferedDataSource::Initialize(const std::string& url,
 
 void BufferedDataSource::CancelInitialize() {
   base::AutoLock auto_lock(lock_);
-  DCHECK(initialize_callback_.get());
+  DCHECK(!initialize_cb_.is_null());
 
-  initialize_callback_.reset();
+  initialize_cb_.Reset();
 
   render_loop_->PostTask(
       FROM_HERE, NewRunnableMethod(this, &BufferedDataSource::CleanupTask));
@@ -202,7 +201,7 @@ void BufferedDataSource::Abort() {
 void BufferedDataSource::InitializeTask() {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(!loader_.get());
-  if (stopped_on_render_loop_ || !initialize_callback_.get())
+  if (stopped_on_render_loop_ || initialize_cb_.is_null())
     return;
 
   if (url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme)) {
@@ -379,12 +378,11 @@ void BufferedDataSource::DoneRead_Locked(int error) {
 void BufferedDataSource::DoneInitialization_Locked(
     media::PipelineStatus status) {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(initialize_callback_.get());
+  DCHECK(!initialize_cb_.is_null());
   lock_.AssertAcquired();
 
-  scoped_ptr<media::PipelineStatusCallback> initialize_callback(
-      initialize_callback_.release());
-  initialize_callback->Run(status);
+  initialize_cb_.Run(status);
+  initialize_cb_.Reset();
 }
 
 /////////////////////////////////////////////////////////////////////////////
@@ -396,7 +394,7 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
   int64 instance_size = loader_->instance_size();
   bool success = error == net::OK;
 
-  if (!initialize_callback_.get()) {
+  if (initialize_cb_.is_null()) {
     loader_->Stop();
     return;
   }
@@ -426,7 +424,7 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
     return;
   }
 
-  // Reference to prevent destruction while inside the |initialize_callback_|
+  // Reference to prevent destruction while inside the |initialize_cb_|
   // call. This is a temporary fix to prevent crashes caused by holding the
   // lock and running the destructor.
   // TODO: Review locking in this class and figure out a way to run the callback
@@ -459,7 +457,7 @@ void BufferedDataSource::NonHttpInitialStartCallback(int error) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
-  if (!initialize_callback_.get()) {
+  if (initialize_cb_.is_null()) {
     loader_->Stop();
     return;
   }
@@ -475,7 +473,7 @@ void BufferedDataSource::NonHttpInitialStartCallback(int error) {
     loader_->Stop();
   }
 
-  // Reference to prevent destruction while inside the |initialize_callback_|
+  // Reference to prevent destruction while inside the |initialize_cb_|
   // call. This is a temporary fix to prevent crashes caused by holding the
   // lock and running the destructor.
   // TODO: Review locking in this class and figure out a way to run the callback
@@ -491,7 +489,7 @@ void BufferedDataSource::NonHttpInitialStartCallback(int error) {
     // this object when Stop() method is ever called. Locking this method is
     // safe because |lock_| is only acquired in tasks on render thread.
     base::AutoLock auto_lock(lock_);
-    if (stop_signal_received_ || !initialize_callback_.get())
+    if (stop_signal_received_ || initialize_cb_.is_null())
       return;
 
     if (!success) {
diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h
index 408d095..76166cd 100644
--- a/webkit/glue/media/buffered_data_source.h
+++ b/webkit/glue/media/buffered_data_source.h
@@ -45,7 +45,7 @@ class BufferedDataSource : public WebDataSource {
 
   // webkit_glue::WebDataSource implementation.
   virtual void Initialize(const std::string& url,
-                          media::PipelineStatusCallback* callback);
+                          const media::PipelineStatusCB& callback);
   virtual void CancelInitialize();
   virtual bool HasSingleOrigin();
   virtual void Abort();
@@ -148,7 +148,7 @@ class BufferedDataSource : public WebDataSource {
   bool network_activity_;
 
   // Callback method from the pipeline for initialization.
-  scoped_ptr<media::PipelineStatusCallback> initialize_callback_;
+  media::PipelineStatusCB initialize_cb_;
 
   // Read parameters received from the Read() method call.
   scoped_ptr<media::DataSource::ReadCallback> read_callback_;
diff --git a/webkit/glue/media/buffered_data_source_unittest.cc b/webkit/glue/media/buffered_data_source_unittest.cc
index 5cdde51..3a4c3d5 100644
--- a/webkit/glue/media/buffered_data_source_unittest.cc
+++ b/webkit/glue/media/buffered_data_source_unittest.cc
@@ -196,7 +196,7 @@ class BufferedDataSourceTest : public testing::Test {
 
     // Actual initialization of the data source.
     data_source_->Initialize(url,
-        media::NewExpectedStatusCallback(expected_init_status));
+        media::NewExpectedStatusCB(expected_init_status));
     message_loop_->RunAllPending();
 
     if (initialized_ok) {
diff --git a/webkit/glue/media/simple_data_source.cc b/webkit/glue/media/simple_data_source.cc
index 97c4d7a..06927a9 100644
--- a/webkit/glue/media/simple_data_source.cc
+++ b/webkit/glue/media/simple_data_source.cc
@@ -78,17 +78,17 @@ void SimpleDataSource::Stop(media::FilterCallback* callback) {
 
 void SimpleDataSource::Initialize(
     const std::string& url,
-    media::PipelineStatusCallback* callback) {
-  // Reference to prevent destruction while inside the |initialize_callback_|
+    const media::PipelineStatusCB& callback) {
+  // Reference to prevent destruction while inside the |initialize_cb_|
   // call. This is a temporary fix to prevent crashes caused by holding the
   // lock and running the destructor.
   scoped_refptr<SimpleDataSource> destruction_guard(this);
   {
     base::AutoLock auto_lock(lock_);
     DCHECK_EQ(state_, UNINITIALIZED);
-    DCHECK(callback);
+    DCHECK(!callback.is_null());
     state_ = INITIALIZING;
-    initialize_callback_.reset(callback);
+    initialize_cb_ = callback;
 
     // Validate the URL.
     url_ = GURL(url);
@@ -105,9 +105,9 @@ void SimpleDataSource::Initialize(
 
 void SimpleDataSource::CancelInitialize() {
   base::AutoLock auto_lock(lock_);
-  DCHECK(initialize_callback_.get());
+  DCHECK(!initialize_cb_.is_null());
   state_ = STOPPED;
-  initialize_callback_.reset();
+  initialize_cb_.Reset();
 
   // Post a task to the render thread to cancel loading the resource.
   render_loop_->PostTask(FROM_HERE,
@@ -334,9 +334,8 @@ void SimpleDataSource::DoneInitialization_Locked(bool success) {
     url_loader_.reset();
   }
 
-  scoped_ptr<media::PipelineStatusCallback> initialize_callback(
-      initialize_callback_.release());
-  initialize_callback->Run(status);
+  initialize_cb_.Run(status);
+  initialize_cb_.Reset();
 }
 
 void SimpleDataSource::UpdateHostState() {
diff --git a/webkit/glue/media/simple_data_source.h b/webkit/glue/media/simple_data_source.h
index 721a8bad..63d4956 100644
--- a/webkit/glue/media/simple_data_source.h
+++ b/webkit/glue/media/simple_data_source.h
@@ -88,7 +88,7 @@ class SimpleDataSource
 
   // webkit_glue::WebDataSource implementation.
   virtual void Initialize(const std::string& url,
-                          media::PipelineStatusCallback* callback);
+                          const media::PipelineStatusCB& callback);
   virtual void CancelInitialize();
   virtual bool HasSingleOrigin();
   virtual void Abort();
@@ -133,7 +133,7 @@ class SimpleDataSource
   base::Lock lock_;
 
   // Filter callbacks.
-  scoped_ptr<media::PipelineStatusCallback> initialize_callback_;
+  media::PipelineStatusCB initialize_cb_;
 
   // Used to ensure mocks for unittests are used instead of reset in Start().
   bool keep_test_loader_;
diff --git a/webkit/glue/media/simple_data_source_unittest.cc b/webkit/glue/media/simple_data_source_unittest.cc
index 264b685..e6279b1 100644
--- a/webkit/glue/media/simple_data_source_unittest.cc
+++ b/webkit/glue/media/simple_data_source_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "media/base/filters.h"
 #include "media/base/mock_callback.h"
 #include "media/base/mock_filter_host.h"
@@ -64,7 +65,7 @@ class SimpleDataSourceTest : public testing::Test {
   }
 
   void InitializeDataSource(const char* url,
-                            media::MockStatusCallback* callback) {
+                            const media::PipelineStatusCB& callback) {
     gurl_ = GURL(url);
 
     url_loader_ = new NiceMock<MockWebURLLoader>();
@@ -164,21 +165,21 @@ class SimpleDataSourceTest : public testing::Test {
 
 TEST_F(SimpleDataSourceTest, InitializeHTTP) {
   InitializeDataSource(kHttpUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   RequestSucceeded(false);
   DestroyDataSource();
 }
 
 TEST_F(SimpleDataSourceTest, InitializeHTTPS) {
   InitializeDataSource(kHttpsUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   RequestSucceeded(false);
   DestroyDataSource();
 }
 
 TEST_F(SimpleDataSourceTest, InitializeFile) {
   InitializeDataSource(kFileUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   RequestSucceeded(true);
   DestroyDataSource();
 }
@@ -197,7 +198,7 @@ TEST_F(SimpleDataSourceTest, InitializeData) {
   EXPECT_CALL(host_, SetBufferedBytes(sizeof(kDataUrlDecoded)));
 
   data_source_->Initialize(kDataUrl,
-      media::NewExpectedStatusCallback(media::PIPELINE_OK));
+      media::NewExpectedStatusCB(media::PIPELINE_OK));
   MessageLoop::current()->RunAllPending();
 
   DestroyDataSource();
@@ -205,27 +206,30 @@ TEST_F(SimpleDataSourceTest, InitializeData) {
 
 TEST_F(SimpleDataSourceTest, RequestFailed) {
   InitializeDataSource(kHttpUrl,
-      media::NewExpectedStatusCallback(media::PIPELINE_ERROR_NETWORK));
+      media::NewExpectedStatusCB(media::PIPELINE_ERROR_NETWORK));
   RequestFailed();
   DestroyDataSource();
 }
 
+static void WasCalled(bool* called, media::PipelineStatus status) {
+  *called = true;
+}
+
 TEST_F(SimpleDataSourceTest, StopWhenDownloading) {
   // The callback should be deleted, but not executed.
   // TODO(scherkus): should this really be the behaviour?  Seems strange...
-  StrictMock<media::MockStatusCallback>* callback =
-      new StrictMock<media::MockStatusCallback>();
-  EXPECT_CALL(*callback, Destructor());
+  bool was_called = false;
 
-  InitializeDataSource(kHttpUrl, callback);
+  InitializeDataSource(kHttpUrl, base::Bind(&WasCalled, &was_called));
 
   EXPECT_CALL(*url_loader_, cancel());
   DestroyDataSource();
+  EXPECT_FALSE(was_called);
 }
 
 TEST_F(SimpleDataSourceTest, AsyncRead) {
   InitializeDataSource(kFileUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   RequestSucceeded(true);
   AsyncRead();
   DestroyDataSource();
@@ -237,14 +241,14 @@ TEST_F(SimpleDataSourceTest, AsyncRead) {
 TEST_F(SimpleDataSourceTest, HasSingleOrigin) {
   // Make sure no redirect case works as expected.
   InitializeDataSource(kHttpUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   RequestSucceeded(false);
   EXPECT_TRUE(data_source_->HasSingleOrigin());
   DestroyDataSource();
 
   // Test redirect to the same domain.
   InitializeDataSource(kHttpUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   Redirect(kHttpRedirectToSameDomainUrl1);
   RequestSucceeded(false);
   EXPECT_TRUE(data_source_->HasSingleOrigin());
@@ -252,7 +256,7 @@ TEST_F(SimpleDataSourceTest, HasSingleOrigin) {
 
   // Test redirect twice to the same domain.
   InitializeDataSource(kHttpUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   Redirect(kHttpRedirectToSameDomainUrl1);
   Redirect(kHttpRedirectToSameDomainUrl2);
   RequestSucceeded(false);
@@ -261,7 +265,7 @@ TEST_F(SimpleDataSourceTest, HasSingleOrigin) {
 
   // Test redirect to a different domain.
   InitializeDataSource(kHttpUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   Redirect(kHttpRedirectToDifferentDomainUrl1);
   RequestSucceeded(false);
   EXPECT_FALSE(data_source_->HasSingleOrigin());
@@ -269,7 +273,7 @@ TEST_F(SimpleDataSourceTest, HasSingleOrigin) {
 
   // Test redirect to the same domain and then to a different domain.
   InitializeDataSource(kHttpUrl,
-                       media::NewExpectedStatusCallback(media::PIPELINE_OK));
+                       media::NewExpectedStatusCB(media::PIPELINE_OK));
   Redirect(kHttpRedirectToSameDomainUrl1);
   Redirect(kHttpRedirectToDifferentDomainUrl1);
   RequestSucceeded(false);
diff --git a/webkit/glue/media/web_data_source.h b/webkit/glue/media/web_data_source.h
index 8a581f6..3e5869d 100644
--- a/webkit/glue/media/web_data_source.h
+++ b/webkit/glue/media/web_data_source.h
@@ -21,7 +21,7 @@ class WebDataSource : public media::DataSource {
   // Initialize this object using |url|. This object calls |callback| when
   // initialization has completed.
   virtual void Initialize(const std::string& url,
-                          media::PipelineStatusCallback* callback) = 0;
+                          const media::PipelineStatusCB& callback) = 0;
 
   // Called to cancel initialization. The callback passed in Initialize() will
   // be destroyed and will not be called after this method returns. Once this
diff --git a/webkit/glue/media/web_data_source_factory.cc b/webkit/glue/media/web_data_source_factory.cc
index d98e2da..7d3ff12 100644
--- a/webkit/glue/media/web_data_source_factory.cc
+++ b/webkit/glue/media/web_data_source_factory.cc
@@ -4,30 +4,27 @@
 
 #include "webkit/glue/media/web_data_source_factory.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 
 namespace webkit_glue {
 
-class WebDataSourceFactory::BuildRequest
-    : public media::AsyncDataSourceFactoryBase::BuildRequest {
- public:
-  BuildRequest(const std::string& url, BuildCallback* callback,
-               WebDataSource* data_source,
-               WebDataSourceBuildObserverHack* build_observer);
-  virtual ~BuildRequest();
-
- protected:
-  // AsyncDataSourceFactoryBase::BuildRequest method.
-  virtual void DoStart();
+static void DataSourceInitDone(
+    WebDataSourceBuildObserverHack* build_observer,
+    const media::DataSourceFactory::BuildCB& callback,
+    const scoped_refptr<WebDataSource>& data_source,
+    media::PipelineStatus status) {
 
- private:
-  void InitDone(media::PipelineStatus status);
+  if (status != media::PIPELINE_OK) {
+    callback.Run(status, NULL);
+    return;
+  }
 
-  scoped_refptr<WebDataSource> data_source_;
-  WebDataSourceBuildObserverHack* build_observer_;
+  if (build_observer)
+    build_observer->Run(data_source.get());
 
-  DISALLOW_COPY_AND_ASSIGN(BuildRequest);
-};
+  callback.Run(status, data_source.get());
+}
 
 WebDataSourceFactory::WebDataSourceFactory(
     MessageLoop* render_loop,
@@ -45,59 +42,21 @@ WebDataSourceFactory::WebDataSourceFactory(
 
 WebDataSourceFactory::~WebDataSourceFactory() {}
 
-media::DataSourceFactory* WebDataSourceFactory::Clone() const {
-  return new WebDataSourceFactory(render_loop_, frame_, factory_function_,
-                                  build_observer_);
-}
-
-bool WebDataSourceFactory::AllowRequests() const {
-  return true;
-}
-
-media::AsyncDataSourceFactoryBase::BuildRequest*
-WebDataSourceFactory::CreateRequest(const std::string& url,
-                                    BuildCallback* callback) {
-  WebDataSource* data_source = factory_function_(render_loop_, frame_);
-
-  return new WebDataSourceFactory::BuildRequest(url, callback, data_source,
-                                                build_observer_);
-}
-
-WebDataSourceFactory::BuildRequest::BuildRequest(
-    const std::string& url,
-    BuildCallback* callback,
-    WebDataSource* data_source,
-    WebDataSourceBuildObserverHack* build_observer)
-    : AsyncDataSourceFactoryBase::BuildRequest(url, callback),
-      data_source_(data_source),
-      build_observer_(build_observer) {
-}
+void WebDataSourceFactory::Build(const std::string& url,
+                                 const BuildCB& callback) {
+  scoped_refptr<WebDataSource> data_source =
+      factory_function_(render_loop_, frame_);
 
-WebDataSourceFactory::BuildRequest::~BuildRequest() {
-  if (data_source_.get()) {
-    data_source_->CancelInitialize();
-    data_source_ = NULL;
-  }
+  data_source->Initialize(
+      url, base::Bind(&DataSourceInitDone,
+                      build_observer_,
+                      callback,
+                      data_source));
 }
 
-void WebDataSourceFactory::BuildRequest::DoStart() {
-  data_source_->Initialize(url(), NewCallback(this, &BuildRequest::InitDone));
-}
-
-void WebDataSourceFactory::BuildRequest::InitDone(
-    media::PipelineStatus status) {
-  scoped_refptr<WebDataSource> data_source;
-
-  data_source = (status == media::PIPELINE_OK) ? data_source_ : NULL;
-  data_source_ = NULL;
-
-  if (build_observer_ && data_source.get()) {
-    build_observer_->Run(data_source.get());
-  }
-
-  RequestComplete(status, data_source);
-  // Don't do anything after this line. This object is deleted by
-  // RequestComplete().
+media::DataSourceFactory* WebDataSourceFactory::Clone() const {
+  return new WebDataSourceFactory(render_loop_, frame_, factory_function_,
+                                  build_observer_);
 }
 
 }  // namespace webkit_glue
diff --git a/webkit/glue/media/web_data_source_factory.h b/webkit/glue/media/web_data_source_factory.h
index 7163ef4..0676d3a 100644
--- a/webkit/glue/media/web_data_source_factory.h
+++ b/webkit/glue/media/web_data_source_factory.h
@@ -5,7 +5,7 @@
 #ifndef WEBKIT_GLUE_MEDIA_BUFFERED_DATA_SOURCE_FACTORY_H_
 #define WEBKIT_GLUE_MEDIA_BUFFERED_DATA_SOURCE_FACTORY_H_
 
-#include "media/base/async_filter_factory_base.h"
+#include "media/base/filter_factories.h"
 #include "webkit/glue/media/web_data_source.h"
 
 class MessageLoop;
@@ -16,7 +16,7 @@ class WebFrame;
 
 namespace webkit_glue {
 
-class WebDataSourceFactory : public media::AsyncDataSourceFactoryBase {
+class WebDataSourceFactory : public media::DataSourceFactory {
  public:
   typedef WebDataSource* (*FactoryFunction)(MessageLoop* render_loop,
                                             WebKit::WebFrame* frame);
@@ -26,14 +26,9 @@ class WebDataSourceFactory : public media::AsyncDataSourceFactoryBase {
                        WebDataSourceBuildObserverHack* build_observer);
   virtual ~WebDataSourceFactory();
 
-  // DataSourceFactory method.
-  virtual media::DataSourceFactory* Clone() const;
-
- protected:
-  // AsyncDataSourceFactoryBase methods.
-  virtual bool AllowRequests() const;
-  virtual AsyncDataSourceFactoryBase::BuildRequest* CreateRequest(
-      const std::string& url, BuildCallback* callback);
+  // DataSourceFactory methods.
+  virtual void Build(const std::string& url, const BuildCB& callback) OVERRIDE;
+  virtual media::DataSourceFactory* Clone() const OVERRIDE;
 
  private:
   class BuildRequest;