Revert 94316 - Fix invalid pointer dereference in WebDataSourceFactory when playback is cancelled during init.

Removed Clone() from DemuxerFactory since it isn't being used.

BUG=90393
TEST=WebDataSourceFactoryTest.*

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

TBR=acolwell@chromium.org
Review URL: http://codereview.chromium.org/7482028

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

5 files changed, 35 insertions, 310 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 27f7e0b..6550fb5 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -4,7 +4,6 @@
 
 #include "webkit/glue/media/buffered_data_source.h"
 
-#include "base/bind.h"
 #include "media/base/filter_host.h"
 #include "net/base/net_errors.h"
 #include "webkit/glue/media/web_data_source_factory.h"
@@ -33,8 +32,7 @@ media::DataSourceFactory* BufferedDataSource::CreateFactory(
     MessageLoop* render_loop,
     WebKit::WebFrame* frame,
     WebDataSourceBuildObserverHack* build_observer) {
-  return new WebDataSourceFactory(render_loop, frame,
-                                  base::Bind(&NewBufferedDataSource),
+  return new WebDataSourceFactory(render_loop, frame, &NewBufferedDataSource,
                                   build_observer);
 }
 
diff --git a/webkit/glue/media/simple_data_source.cc b/webkit/glue/media/simple_data_source.cc
index 9dcb949..06927a9 100644
--- a/webkit/glue/media/simple_data_source.cc
+++ b/webkit/glue/media/simple_data_source.cc
@@ -4,7 +4,6 @@
 
 #include "webkit/glue/media/simple_data_source.h"
 
-#include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
 #include "media/base/filter_host.h"
@@ -34,8 +33,7 @@ media::DataSourceFactory* SimpleDataSource::CreateFactory(
     MessageLoop* render_loop,
     WebKit::WebFrame* frame,
     WebDataSourceBuildObserverHack* build_observer) {
-  return new WebDataSourceFactory(render_loop, frame,
-                                  base::Bind(&NewSimpleDataSource),
+  return new WebDataSourceFactory(render_loop, frame, &NewSimpleDataSource,
                                   build_observer);
 }
 
diff --git a/webkit/glue/media/web_data_source_factory.cc b/webkit/glue/media/web_data_source_factory.cc
index f71fd92..7d3ff12 100644
--- a/webkit/glue/media/web_data_source_factory.cc
+++ b/webkit/glue/media/web_data_source_factory.cc
@@ -4,87 +4,29 @@
 
 #include "webkit/glue/media/web_data_source_factory.h"
 
-#include <set>
-
 #include "base/bind.h"
 #include "base/logging.h"
-#include "base/synchronization/lock.h"
 
 namespace webkit_glue {
 
-// The state shared between a factory and its clones. The pointers
-// passed to the original factory are only valid for the lifetime of
-// that factory. This object allows the clones to detect when the original
-// factory goes away and prevents them from dereferencing invalid pointers.
-class WebDataSourceFactory::SharedState
-    : public base::RefCountedThreadSafe<SharedState> {
- public:
-  SharedState(MessageLoop* render_loop, WebKit::WebFrame* frame,
-              FactoryFunction factory_function,
-              WebDataSourceBuildObserverHack* build_observer);
-  virtual ~SharedState();
-
-  // Start building a source for |url|.
-  void Build(const std::string& url, const BuildCB& callback);
-
-  // Clears state so pending build's are aborted and new Build() calls
-  // report an error.
-  void Clear();
-
- private:
-  // Called when initialization for a source completes.
-  void InitDone(
-      const media::DataSourceFactory::BuildCB& callback,
-      const scoped_refptr<WebDataSource>& data_source,
-      media::PipelineStatus status);
-
-  base::Lock lock_;
-  MessageLoop* render_loop_;
-  WebKit::WebFrame* frame_;
-  FactoryFunction factory_function_;
+static void DataSourceInitDone(
+    WebDataSourceBuildObserverHack* build_observer,
+    const media::DataSourceFactory::BuildCB& callback,
+    const scoped_refptr<WebDataSource>& data_source,
+    media::PipelineStatus status) {
 
-  WebDataSourceBuildObserverHack* build_observer_;
-  typedef std::set<scoped_refptr<WebDataSource> > SourceSet;
-  SourceSet pending_sources_;
+  if (status != media::PIPELINE_OK) {
+    callback.Run(status, NULL);
+    return;
+  }
 
-  DISALLOW_IMPLICIT_CONSTRUCTORS(SharedState);
-};
+  if (build_observer)
+    build_observer->Run(data_source.get());
 
-WebDataSourceFactory::WebDataSourceFactory(
-    MessageLoop* render_loop,
-    WebKit::WebFrame* frame,
-    FactoryFunction factory_function,
-    WebDataSourceBuildObserverHack* build_observer)
-    : is_clone_(false),
-      shared_state_(new SharedState(render_loop, frame, factory_function,
-                                    build_observer)) {
-  DCHECK(render_loop);
-  DCHECK(frame);
-  DCHECK(!factory_function.is_null());
+  callback.Run(status, data_source.get());
 }
 
 WebDataSourceFactory::WebDataSourceFactory(
-    const scoped_refptr<SharedState>& shared_state)
-    : is_clone_(true),
-      shared_state_(shared_state) {
-  DCHECK(shared_state);
-}
-
-WebDataSourceFactory::~WebDataSourceFactory() {
-  if (!is_clone_)
-    shared_state_->Clear();
-}
-
-void WebDataSourceFactory::Build(const std::string& url,
-                                 const BuildCB& callback) {
-  shared_state_->Build(url, callback);
-}
-
-media::DataSourceFactory* WebDataSourceFactory::Clone() const {
-  return new WebDataSourceFactory(shared_state_);
-}
-
-WebDataSourceFactory::SharedState::SharedState(
     MessageLoop* render_loop,
     WebKit::WebFrame* frame,
     FactoryFunction factory_function,
@@ -95,68 +37,26 @@ WebDataSourceFactory::SharedState::SharedState(
       build_observer_(build_observer) {
   DCHECK(render_loop_);
   DCHECK(frame_);
-  DCHECK(!factory_function_.is_null());
+  DCHECK(factory_function_);
 }
 
-WebDataSourceFactory::SharedState::~SharedState() {}
-
-void WebDataSourceFactory::SharedState::Clear() {
-  base::AutoLock auto_lock(lock_);
-  render_loop_ = NULL;
-  frame_ = NULL;
-  factory_function_.Reset();
-  build_observer_ = NULL;
-
-  for (SourceSet::iterator itr = pending_sources_.begin();
-       itr != pending_sources_.end();
-       ++itr) {
-    (*itr)->CancelInitialize();
-  }
-
-  pending_sources_.clear();
-}
+WebDataSourceFactory::~WebDataSourceFactory() {}
 
-void WebDataSourceFactory::SharedState::Build(const std::string& url,
-                                              const BuildCB& callback) {
-  scoped_refptr<WebDataSource> data_source;
-  media::PipelineStatusCB init_cb;
-  {
-    base::AutoLock auto_lock(lock_);
-
-    if (!factory_function_.is_null())
-      data_source = factory_function_.Run(render_loop_, frame_);
-
-    if (data_source.get()) {
-      pending_sources_.insert(data_source.get());
-      init_cb = base::Bind(&SharedState::InitDone, this, callback, data_source);
-    }
-  }
-
-  if (init_cb.is_null()) {
-    callback.Run(media::PIPELINE_ERROR_URL_NOT_FOUND, NULL);
-    return;
-  }
+void WebDataSourceFactory::Build(const std::string& url,
+                                 const BuildCB& callback) {
+  scoped_refptr<WebDataSource> data_source =
+      factory_function_(render_loop_, frame_);
 
-  data_source->Initialize(url, init_cb);
+  data_source->Initialize(
+      url, base::Bind(&DataSourceInitDone,
+                      build_observer_,
+                      callback,
+                      data_source));
 }
 
-void WebDataSourceFactory::SharedState::InitDone(
-    const media::DataSourceFactory::BuildCB& callback,
-    const scoped_refptr<WebDataSource>& data_source,
-    media::PipelineStatus status) {
-  {
-    base::AutoLock auto_lock(lock_);
-
-    // Make sure we don't get one of these calls after Clear() has been called.
-    DCHECK(!factory_function_.is_null());
-
-    pending_sources_.erase(data_source.get());
-
-    if (build_observer_ && status == media::PIPELINE_OK)
-      build_observer_->Run(data_source.get());
-  }
-
-  callback.Run(status, data_source.get());
+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 73cbaa8..0676d3a 100644
--- a/webkit/glue/media/web_data_source_factory.h
+++ b/webkit/glue/media/web_data_source_factory.h
@@ -18,13 +18,12 @@ namespace webkit_glue {
 
 class WebDataSourceFactory : public media::DataSourceFactory {
  public:
-  typedef base::Callback<WebDataSource*(MessageLoop*,
-                                        WebKit::WebFrame*)> FactoryFunction;
+  typedef WebDataSource* (*FactoryFunction)(MessageLoop* render_loop,
+                                            WebKit::WebFrame* frame);
 
   WebDataSourceFactory(MessageLoop* render_loop, WebKit::WebFrame* frame,
                        FactoryFunction factory_function,
                        WebDataSourceBuildObserverHack* build_observer);
-
   virtual ~WebDataSourceFactory();
 
   // DataSourceFactory methods.
@@ -32,17 +31,12 @@ class WebDataSourceFactory : public media::DataSourceFactory {
   virtual media::DataSourceFactory* Clone() const OVERRIDE;
 
  private:
-  class SharedState;
-
-  // Constructor used by Clone().
-  explicit WebDataSourceFactory(
-      const scoped_refptr<SharedState>& shared_state);
-
-  // Keeps track of whether this is a factory created by Clone().
-  bool is_clone_;
+  class BuildRequest;
 
-  // State shared between original factory and its clones.
-  scoped_refptr<SharedState> shared_state_;
+  MessageLoop* render_loop_;
+  WebKit::WebFrame* frame_;
+  FactoryFunction factory_function_;
+  WebDataSourceBuildObserverHack* build_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(WebDataSourceFactory);
 };
diff --git a/webkit/glue/media/web_data_source_factory_unittest.cc b/webkit/glue/media/web_data_source_factory_unittest.cc
deleted file mode 100644
index 077c5da..0000000
--- a/webkit/glue/media/web_data_source_factory_unittest.cc
+++ /dev/null
@@ -1,165 +0,0 @@
-// Copyright (c) 2011 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.
-
-#include "base/bind.h"
-#include "base/message_loop.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
-#include "webkit/glue/media/web_data_source.h"
-#include "webkit/glue/media/web_data_source_factory.h"
-#include "webkit/mocks/mock_webframeclient.h"
-
-using ::testing::_;
-using ::testing::InSequence;
-using ::testing::MockFunction;
-using ::testing::SaveArg;
-using ::testing::StrictMock;
-
-namespace webkit_glue {
-
-class MockWebDataSource : public WebDataSource {
- public:
-  virtual ~MockWebDataSource() {}
-
-  // WebDataSource methods
-  MOCK_METHOD2(Initialize, void(const std::string& url,
-                                const media::PipelineStatusCB& callback));
-  MOCK_METHOD0(CancelInitialize, void());
-  MOCK_METHOD0(HasSingleOrigin, bool());
-  MOCK_METHOD0(Abort, void());
-
-  // DataSource methods.
-  MOCK_METHOD4(Read, void(int64 position, size_t size, uint8* data,
-                          media::DataSource::ReadCallback* callback));
-  MOCK_METHOD1(GetSize, bool(int64* size_out));
-  MOCK_METHOD1(SetPreload, void(media::Preload preload));
-  MOCK_METHOD0(IsStreaming, bool());
-};
-
-class WebDataSourceFactoryTest : public testing::Test {
- public:
-  WebDataSourceFactoryTest()
-      : message_loop_(MessageLoop::current()),
-        observer_called_(false),
-        view_(WebKit::WebView::create(NULL)) {
-    view_->initializeMainFrame(&client_);
-  }
-
-  virtual ~WebDataSourceFactoryTest() {
-    view_->close();
-  }
-
-  void OnBuildObserverCall(WebDataSource* source) {
-    observer_called_ = true;
-  }
-
-  static WebDataSource* OnBuild(
-      const scoped_refptr<WebDataSource>& data_source,
-      MessageLoop* render_loop,
-      WebKit::WebFrame* frame) {
-    return data_source.get();
-  }
-
-  void CreateFactory(const scoped_refptr<WebDataSource>& data_source) {
-    observer_cb_.reset(
-        NewCallback(this,
-                    &WebDataSourceFactoryTest::OnBuildObserverCall));
-    observer_called_ = false;
-
-    factory_.reset(new WebDataSourceFactory(
-        message_loop_,
-        view_->mainFrame(),
-        base::Bind(&WebDataSourceFactoryTest::OnBuild,
-                   data_source),
-        observer_cb_.get()));
-  }
-
-  MOCK_METHOD1(BuildDoneStatus, void(media::PipelineStatus));
-
-  void OnBuildDone(media::PipelineStatus status,
-                   media::DataSource* data_source) {
-    build_done_mock_.Call(status);
-  }
-
-  // Helper function that creates a BuildCB that expects to be
-  // called with a particular status.
-  media::DataSourceFactory::BuildCB ExpectBuildDone(
-      media::PipelineStatus expected_status) {
-    EXPECT_CALL(build_done_mock_, Call(expected_status));
-    return base::Bind(&WebDataSourceFactoryTest::OnBuildDone,
-                      base::Unretained(this));
-  }
-
-  // Helper function that creates a BuildCB that isn't expected
-  // to be called.
-  media::DataSourceFactory::BuildCB ExpectBuildDoneIsNotCalled() {
-    return base::Bind(&WebDataSourceFactoryTest::OnBuildDone,
-                      base::Unretained(this));
-  }
-
- protected:
-  MessageLoop* message_loop_;
-  scoped_ptr<WebDataSourceFactory> factory_;
-
-  scoped_ptr<WebDataSourceBuildObserverHack> observer_cb_;
-  bool observer_called_;
-
-  MockWebFrameClient client_;
-  WebKit::WebView* view_;
-
-  StrictMock<MockFunction<void(media::PipelineStatus)> > build_done_mock_;
-};
-
-TEST_F(WebDataSourceFactoryTest, NormalBuild) {
-
-  InSequence s;
-
-  std::string url = "http://blah.com";
-  MockWebDataSource* mock_data_source = new MockWebDataSource();
-  media::PipelineStatusCB init_cb;
-  EXPECT_CALL(*mock_data_source, Initialize(url, _))
-      .WillOnce(SaveArg<1>(&init_cb));
-  CreateFactory(mock_data_source);
-
-  factory_->Build(url, ExpectBuildDone(media::PIPELINE_OK));
-
-  EXPECT_FALSE(init_cb.is_null());
-  init_cb.Run(media::PIPELINE_OK);
-}
-
-// Test the behavior when the factory is destroyed while there is a pending
-// Build() request.
-TEST_F(WebDataSourceFactoryTest, DestroyedDuringBuild) {
-  InSequence s;
-
-  std::string url = "http://blah.com";
-  MockWebDataSource* mock_data_source = new MockWebDataSource();
-  EXPECT_CALL(*mock_data_source, Initialize(url, _));
-  CreateFactory(mock_data_source);
-
-  factory_->Build(url, ExpectBuildDoneIsNotCalled());
-
-  EXPECT_CALL(*mock_data_source, CancelInitialize());
-  factory_.reset();
-}
-
-// Test the functionality of a cloned factory after the original
-// factory has been destroyed.
-TEST_F(WebDataSourceFactoryTest, DestroyAfterClone) {
-  InSequence s;
-
-  std::string url = "http://blah.com";
-  MockWebDataSource* mock_data_source = new MockWebDataSource();
-  CreateFactory(mock_data_source);
-
-  scoped_ptr<media::DataSourceFactory> factory2(factory_->Clone());
-
-  factory_.reset();
-
-  // Expect the cloned factory build to fail because the original factory
-  // is gone.
-  factory2->Build(url, ExpectBuildDone(media::PIPELINE_ERROR_URL_NOT_FOUND));
-}
-
-}  // namespace webkit_glue