Changing constructor of BufferedDataSource to accept GURL and CORSMode

BufferedDataSource should accept the 'url' and the 'CORS mode' parameters
in the constructor instead via the initialize() method. This patch makes
this change and now only the 'initializeCallBack' is passed to initialize()
method. Also now the WebMediaPlayerImpl directly queries the BufferedDataSource
whether the resource is local or not, instead of finding it out itself.

Review URL: https://codereview.chromium.org/306953005

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

content/renderer/media/buffered_data_source.cc

@@ -79,14 +79,16 @@ void BufferedDataSource::ReadOperation::Run(
 }
 
 BufferedDataSource::BufferedDataSource(
+    const GURL& url,
+    BufferedResourceLoader::CORSMode cors_mode,
     const scoped_refptr<base::MessageLoopProxy>& render_loop,
     WebFrame* frame,
     media::MediaLog* media_log,
     BufferedDataSourceHost* host,
     const DownloadingCB& downloading_cb)
-    : cors_mode_(BufferedResourceLoader::kUnspecified),
+    : url_(url),
+      cors_mode_(cors_mode),
       total_bytes_(kPositionNotSpecified),
-      assume_fully_buffered_(false),
       streaming_(false),
       frame_(frame),
       intermediate_read_buffer_(new uint8[kInitialReadBufferSize]),
@@ -128,19 +130,14 @@ BufferedResourceLoader* BufferedDataSource::CreateResourceLoader(
                                     media_log_.get());
 }
 
-void BufferedDataSource::Initialize(
-    const GURL& url,
-    BufferedResourceLoader::CORSMode cors_mode,
-    const InitializeCB& init_cb) {
+void BufferedDataSource::Initialize(const InitializeCB& init_cb) {
   DCHECK(render_loop_->BelongsToCurrentThread());
   DCHECK(!init_cb.is_null());
   DCHECK(!loader_.get());
-  url_ = url;
-  cors_mode_ = cors_mode;
 
   init_cb_ = init_cb;
 
-  if (url_.SchemeIs(url::kHttpScheme) || url_.SchemeIs(url::kHttpsScheme)) {
+  if (url_.SchemeIsHTTPOrHTTPS()) {
     // Do an unbounded range request starting at the beginning.  If the server
     // responds with 200 instead of 206 we'll fall back into a streaming mode.
     loader_.reset(CreateResourceLoader(0, kPositionNotSpecified));
@@ -150,7 +147,6 @@ void BufferedDataSource::Initialize(
     // we won't be served HTTP headers.
     loader_.reset(CreateResourceLoader(kPositionNotSpecified,
                                        kPositionNotSpecified));
-    assume_fully_buffered_ = true;
   }
 
   base::WeakPtr<BufferedDataSource> weak_this = weak_factory_.GetWeakPtr();
@@ -353,12 +349,13 @@ void BufferedDataSource::StartCallback(
   // All responses must be successful. Resources that are assumed to be fully
   // buffered must have a known content length.
   bool success = status == BufferedResourceLoader::kOk &&
-      (!assume_fully_buffered_ ||
-       loader_->instance_size() != kPositionNotSpecified);
+                 (!assume_fully_buffered() ||
+                  loader_->instance_size() != kPositionNotSpecified);
 
   if (success) {
     total_bytes_ = loader_->instance_size();
-    streaming_ = !assume_fully_buffered_ &&
+    streaming_ =
+        !assume_fully_buffered() &&
         (total_bytes_ == kPositionNotSpecified || !loader_->range_supported());
 
     media_log_->SetDoubleProperty("total_bytes",
@@ -377,7 +374,7 @@ void BufferedDataSource::StartCallback(
   if (success) {
     if (total_bytes_ != kPositionNotSpecified) {
       host_->SetTotalBytes(total_bytes_);
-      if (assume_fully_buffered_)
+      if (assume_fully_buffered())
         host_->AddBufferedByteRange(0, total_bytes_);
     }
 
@@ -473,7 +470,7 @@ void BufferedDataSource::LoadingStateChangedCallback(
     BufferedResourceLoader::LoadingState state) {
   DCHECK(render_loop_->BelongsToCurrentThread());
 
-  if (assume_fully_buffered_)
+  if (assume_fully_buffered())
     return;
 
   bool is_downloading_data;
@@ -501,7 +498,7 @@ void BufferedDataSource::LoadingStateChangedCallback(
 void BufferedDataSource::ProgressCallback(int64 position) {
   DCHECK(render_loop_->BelongsToCurrentThread());
 
-  if (assume_fully_buffered_)
+  if (assume_fully_buffered())
     return;
 
   // TODO(scherkus): we shouldn't have to lock to signal host(), see

content/renderer/media/buffered_data_source.h

@@ -50,24 +50,23 @@ class CONTENT_EXPORT BufferedDataSource : public media::DataSource {
  public:
   typedef base::Callback<void(bool)> DownloadingCB;
 
-  // Buffered byte range changes will be reported to |host|. |downloading_cb|
-  // will be called whenever the downloading/paused state of the source changes.
-  BufferedDataSource(const scoped_refptr<base::MessageLoopProxy>& render_loop,
+  // |url| and |cors_mode| are passed to the object. Buffered byte range changes
+  // will be reported to |host|. |downloading_cb| will be called whenever the
+  // downloading/paused state of the source changes.
+  BufferedDataSource(const GURL& url,
+                     BufferedResourceLoader::CORSMode cors_mode,
+                     const scoped_refptr<base::MessageLoopProxy>& render_loop,
                      blink::WebFrame* frame,
                      media::MediaLog* media_log,
                      BufferedDataSourceHost* host,
                      const DownloadingCB& downloading_cb);
   virtual ~BufferedDataSource();
 
-  // Initialize this object using |url| and |cors_mode|, executing |init_cb|
-  // with the result of initialization when it has completed.
+  // Executes |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 InitializeCB& init_cb);
+  void Initialize(const InitializeCB& init_cb);
 
   // Adjusts the buffering algorithm based on the given preload value.
   void SetPreload(Preload preload);
@@ -94,6 +93,9 @@ class CONTENT_EXPORT BufferedDataSource : public media::DataSource {
   void MediaIsPlaying();
   void MediaIsPaused();
 
+  // Returns true if the resource is local.
+  bool assume_fully_buffered() { return !url_.SchemeIsHTTPOrHTTPS(); }
+
   // media::DataSource implementation.
   // Called from demuxer thread.
   virtual void Stop(const base::Closure& closure) OVERRIDE;
@@ -157,10 +159,6 @@ class CONTENT_EXPORT BufferedDataSource : public media::DataSource {
   // determined by reaching EOF.
   int64 total_bytes_;
 
-  // Some resources are assumed to be fully buffered (i.e., file://) so we don't
-  // need to report what |loader_| has buffered.
-  bool assume_fully_buffered_;
-
   // This value will be true if this data source can only support streaming.
   // i.e. range request is not supported.
   bool streaming_;

content/renderer/media/buffered_data_source_unittest.cc

@@ -48,15 +48,20 @@ class MockBufferedDataSourceHost : public BufferedDataSourceHost {
 class MockBufferedDataSource : public BufferedDataSource {
  public:
   MockBufferedDataSource(
+      const GURL& url,
       const scoped_refptr<base::MessageLoopProxy>& message_loop,
       WebLocalFrame* frame,
       BufferedDataSourceHost* host)
-      : BufferedDataSource(message_loop, frame, new media::MediaLog(), host,
+      : BufferedDataSource(url,
+                           BufferedResourceLoader::kUnspecified,
+                           message_loop,
+                           frame,
+                           new media::MediaLog(),
+                           host,
                            base::Bind(&MockBufferedDataSource::set_downloading,
                                       base::Unretained(this))),
         downloading_(false),
-        loading_(false) {
-  }
+        loading_(false) {}
   virtual ~MockBufferedDataSource() {}
 
   MOCK_METHOD2(CreateResourceLoader, BufferedResourceLoader*(int64, int64));
@@ -107,11 +112,6 @@ class BufferedDataSourceTest : public testing::Test {
   BufferedDataSourceTest()
       : view_(WebView::create(NULL)), frame_(WebLocalFrame::create(&client_)) {
     view_->setMainFrame(frame_);
-
-    data_source_.reset(
-        new MockBufferedDataSource(message_loop_.message_loop_proxy(),
-                                   view_->mainFrame()->toWebLocalFrame(),
-                                   &host_));
   }
 
   virtual ~BufferedDataSourceTest() {
@@ -123,17 +123,20 @@ class BufferedDataSourceTest : public testing::Test {
 
   void Initialize(const char* url, bool expected) {
     GURL gurl(url);
-    response_generator_.reset(new TestResponseGenerator(gurl, kFileSize));
+    data_source_.reset(
+        new MockBufferedDataSource(gurl,
+                                   message_loop_.message_loop_proxy(),
+                                   view_->mainFrame()->toWebLocalFrame(),
+                                   &host_));
 
+    response_generator_.reset(new TestResponseGenerator(gurl, kFileSize));
     ExpectCreateResourceLoader();
     EXPECT_CALL(*this, OnInitialize(expected));
-    data_source_->Initialize(
-        gurl, BufferedResourceLoader::kUnspecified, base::Bind(
-            &BufferedDataSourceTest::OnInitialize, base::Unretained(this)));
+    data_source_->Initialize(base::Bind(&BufferedDataSourceTest::OnInitialize,
+                                        base::Unretained(this)));
     message_loop_.RunUntilIdle();
 
-    bool is_http =
-        gurl.SchemeIs(url::kHttpScheme) || gurl.SchemeIs(url::kHttpsScheme);
+    bool is_http = gurl.SchemeIsHTTPOrHTTPS();
     EXPECT_EQ(data_source_->downloading(), is_http);
   }
 

content/renderer/media/webmediaplayer_impl.cc

@@ -167,7 +167,6 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       accelerated_compositing_reported_(false),
       incremented_externally_allocated_memory_(false),
       gpu_factories_(RenderThreadImpl::current()->GetGpuFactories()),
-      is_local_source_(false),
       supports_save_(true),
       starting_(false),
       chunk_demuxer_(NULL),
@@ -318,18 +317,15 @@ void WebMediaPlayerImpl::DoLoad(LoadType load_type,
 
   // Otherwise it's a regular request which requires resolving the URL first.
   data_source_.reset(new BufferedDataSource(
+      url,
+      static_cast<BufferedResourceLoader::CORSMode>(cors_mode),
       main_loop_,
       frame_,
       media_log_.get(),
       &buffered_data_source_host_,
       base::Bind(&WebMediaPlayerImpl::NotifyDownloading, AsWeakPtr())));
   data_source_->Initialize(
-      url, static_cast<BufferedResourceLoader::CORSMode>(cors_mode),
-      base::Bind(
-          &WebMediaPlayerImpl::DataSourceInitialized,
-          AsWeakPtr(), gurl));
-
-  is_local_source_ = !gurl.SchemeIsHTTPOrHTTPS();
+      base::Bind(&WebMediaPlayerImpl::DataSourceInitialized, AsWeakPtr()));
 }
 
 void WebMediaPlayerImpl::play() {
@@ -1112,7 +1108,7 @@ void WebMediaPlayerImpl::OnKeyMessage(const std::string& session_id,
       destination_url);
 }
 
-void WebMediaPlayerImpl::DataSourceInitialized(const GURL& gurl, bool success) {
+void WebMediaPlayerImpl::DataSourceInitialized(bool success) {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
   if (!success) {
@@ -1248,8 +1244,8 @@ void WebMediaPlayerImpl::SetReadyState(WebMediaPlayer::ReadyState state) {
   DCHECK(main_loop_->BelongsToCurrentThread());
   DVLOG(1) << "SetReadyState: " << state;
 
-  if (state == WebMediaPlayer::ReadyStateHaveEnoughData &&
-      is_local_source_ &&
+  if (state == WebMediaPlayer::ReadyStateHaveEnoughData && data_source_ &&
+      data_source_->assume_fully_buffered() &&
       network_state_ == WebMediaPlayer::NetworkStateLoading)
     SetNetworkState(WebMediaPlayer::NetworkStateLoaded);
 

content/renderer/media/webmediaplayer_impl.h

@@ -191,7 +191,7 @@ class WebMediaPlayerImpl
               CORSMode cors_mode);
 
   // Called after asynchronous initialization of a data source completed.
-  void DataSourceInitialized(const GURL& gurl, bool success);
+  void DataSourceInitialized(bool success);
 
   // Called when the data source is downloading or paused.
   void NotifyDownloading(bool is_downloading);
@@ -310,7 +310,6 @@ class WebMediaPlayerImpl
   // Routes audio playback to either AudioRendererSink or WebAudio.
   scoped_refptr<WebAudioSourceProviderImpl> audio_source_provider_;
 
-  bool is_local_source_;
   bool supports_save_;
 
   bool starting_;