Implementing preload=metadata for video

webkit/glue/media/buffered_resource_loader.cc

Index: webkit/glue/media/buffered_resource_loader.cc
diff --git a/webkit/glue/media/buffered_resource_loader.cc b/webkit/glue/media/buffered_resource_loader.cc
index c6295bfd12f87e331f2c5fdb74494e50cde6053e..33319f2c990c8ea3e863d144b27d5f0aaad05422 100644
--- a/webkit/glue/media/buffered_resource_loader.cc
+++ b/webkit/glue/media/buffered_resource_loader.cc
@@ -11,6 +11,8 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebKitClient.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebURLError.h"
+#include "webkit/glue/media/buffered_data_source.h"
+#include "webkit/glue/media/defer_strategy.h"
 #include "webkit/glue/multipart_response_delegate.h"
 #include "webkit/glue/webkit_glue.h"
 
@@ -50,7 +52,7 @@ BufferedResourceLoader::BufferedResourceLoader(
     int64 last_byte_position)
     : buffer_(new media::SeekableBuffer(kBackwardCapcity, kForwardCapacity)),
       deferred_(false),
-      defer_allowed_(true),
+      defer_strategy_(ReadThenDeferStrategy::GetInstance()),
       completed_(false),
       range_requested_(false),
       partial_response_(false),
@@ -184,6 +186,8 @@ void BufferedResourceLoader::Read(int64 position,
     return;
   }
 
+  DisableDeferIfNeeded();

[acolwell GONE FROM CHROMIUM, 2011/03/25 04:35:28]

Why is this call needed now?

[vrk (LEFT CHROMIUM), 2011/03/25 21:33:32]

Ah, a little hard to explain :) 

This call is needed for the case when you cannot fulfill the read because you
don't have enough data, and you're deferred.

In the old version, the buffer would defer only if there was >5mb of data in the
buffer. This means that if you don't have enough data to fulfill the read, you
would have to be in process of loading data (not deferred) - unless your read
request was > 5 mb (which is a bug! issue 42956)

But now that I have the "ReadThenDefer" buffering scheme that reads only as
little as possible before deferring, it's much more likely that you're deferred
and won't be able to get more data.

Added a comment to try to explain. Let me know if you still have questions or
have another idea for implementation.

+
   // If we expected the read request to be fulfilled later, returns
   // immediately and let more data to flow in.
   if (WillFulfillRead())
@@ -199,9 +203,14 @@ int64 BufferedResourceLoader::GetBufferedPosition() {
   return kPositionNotSpecified;
 }
 
-void BufferedResourceLoader::SetAllowDefer(bool is_allowed) {
-  defer_allowed_ = is_allowed;
-  DisableDeferIfNeeded();
+void BufferedResourceLoader::UpdateDeferStrategy(
+    BufferedDataSource* data_source) {
+  DeferStrategy* strategy = data_source->ChooseDeferStrategy();

[acolwell GONE FROM CHROMIUM, 2011/03/25 04:35:28]

Why don't you do the ChooseDeferStrategy() in the call to this function? That
way this code doesn't have to know about BufferedDataSource.

[vrk (LEFT CHROMIUM), 2011/03/25 21:33:32]

Done.

+  if (strategy != defer_strategy_) {
+    defer_strategy_ = strategy;
+    EnableDeferIfNeeded();
+    DisableDeferIfNeeded();

[acolwell GONE FROM CHROMIUM, 2011/03/25 04:35:28]

The EnableDeferIfNeeded() followed by DisableDeferIfNeeded() is confusing. Is it
true that one of them will do something and the other is a noop? Will the defer
status get toggled on and then off?

[vrk (LEFT CHROMIUM), 2011/03/25 21:33:32]

Redid this to something more intuitive.

+  }
 }
 
 int64 BufferedResourceLoader::content_length() {
@@ -336,7 +345,7 @@ void BufferedResourceLoader::didReceiveData(
   // If there is an active read request, try to fulfill the request.
   if (HasPendingRead() && CanFulfillRead()) {
     ReadInternal();
-  } else if (!defer_allowed_) {
+  } else if (!defer_strategy_->AllowsDeferring()) {
     // If we're not allowed to defer, slide the buffer window forward instead
     // of deferring.
     if (buffer_->forward_bytes() > buffer_->forward_capacity()) {
@@ -432,31 +441,15 @@ bool BufferedResourceLoader::HasSingleOrigin() const {
 /////////////////////////////////////////////////////////////////////////////
 // Helper methods.
 void BufferedResourceLoader::EnableDeferIfNeeded() {
-  if (!defer_allowed_)
-    return;
-
-  if (!deferred_ &&
-      buffer_->forward_bytes() >= buffer_->forward_capacity()) {
-    deferred_ = true;
-
-  if (url_loader_.get())
-    url_loader_->setDefersLoading(true);
-
+  bool event_occurred = defer_strategy_->EnableDeferIfNeeded(this);
+  if (event_occurred)
     NotifyNetworkEvent();
-  }
 }
 
 void BufferedResourceLoader::DisableDeferIfNeeded() {
-  if (deferred_ &&
-      (!defer_allowed_ ||
-       buffer_->forward_bytes() < buffer_->forward_capacity() / 2)) {
-    deferred_ = false;
-
-    if (url_loader_.get())
-      url_loader_->setDefersLoading(false);
-
+  bool event_occurred = defer_strategy_->DisableDeferIfNeeded(this);
+  if (event_occurred)
     NotifyNetworkEvent();
-  }
 }
 
 bool BufferedResourceLoader::CanFulfillRead() {