Split BufferedResourceLoader's network callback into separate loading state and progress callbacks.

webkit/media/buffered_resource_loader.h

 // Copyright (c) 2012 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.
 
 #ifndef WEBKIT_MEDIA_BUFFERED_RESOURCE_LOADER_H_
 #define WEBKIT_MEDIA_BUFFERED_RESOURCE_LOADER_H_
 
 #include <string>
 
 #include "base/callback.h"
@@ skipping 47 matching lines @@
     //   - Connection was terminated
     //
     // At this point you should delete the loader.
     kFailed,
 
     // The loader will never be able to satisfy the read request. Please stop,
     // delete, create a new loader, and try again.
     kCacheMiss,
   };
 
+  enum LoadingState {
+    kLoading,  // Actively attempting to download data.
+    kLoadingDeferred,  // Loading intentionally deferred.
+    kLoadingFinished,  // Loading finished normally; no more data will arrive.
+    kLoadingFailed,  // Loading finished abnormally; no more data will arrive.
+  };
+
   // |url| - URL for the resource to be loaded.
   // |first_byte_position| - First byte to start loading from,
   // |kPositionNotSpecified| for not specified.
   // |last_byte_position| - Last byte to be loaded,
   // |kPositionNotSpecified| for not specified.
   // |strategy| is the initial loading strategy to use.
   // |bitrate| is the bitrate of the media, 0 if unknown.
   // |playback_rate| is the current playback rate of the media.
   BufferedResourceLoader(const GURL& url,
                          int64 first_byte_position,
                          int64 last_byte_position,
                          DeferStrategy strategy,
                          int bitrate,
                          float playback_rate,
                          media::MediaLog* media_log);
   virtual ~BufferedResourceLoader();
 
   // Start the resource loading with the specified URL and range.
   //
-  // |event_cb| is called to notify the client of network activity in the
-  // following situations:
-  //   - Data was received
-  //   - Reading was suspended/resumed
-  //   - Loading completed
-  //   - Loading failed
+  // |progress_cb| is executed when additional data has arrived.
+  // |loading_cb| is executed when the loading state has changed.

[Ami GONE FROM CHROMIUM, 2012/07/09 00:25:46]

Flip order of these two lines to match the orders below.

[scherkus (not reviewing), 2012/07/09 18:53:30]

Done.

   typedef base::Callback<void(Status)> StartCB;
+  typedef base::Callback<void(LoadingState)> LoadingCB;
+  typedef base::Callback<void(int64)> ProgressCB;
   void Start(const StartCB& start_cb,
-             const base::Closure& event_cb,
+             const LoadingCB& loading_cb,
+             const ProgressCB& progress_cb,
              WebKit::WebFrame* frame);
 
   // Stops everything associated with this loader, including active URL loads
   // and pending callbacks.
   //
   // It is safe to delete a BufferedResourceLoader after calling Stop().
   void Stop();
 
   // Copies |read_size| bytes from |position| into |buffer|, executing |read_cb|
   // when the operation has completed.
   //
   // The callback will contain the number of bytes read iff the status is kOk,
   // zero otherwise.
   //
   // If necessary will temporarily increase forward capacity of buffer to
   // accomodate an unusually large read.
   typedef base::Callback<void(Status, int)> ReadCB;
   void Read(int64 position, int read_size,
             uint8* buffer, const ReadCB& read_cb);
 
-  // Returns the position of the last byte buffered. Returns
-  // |kPositionNotSpecified| if such value is not available.
-  int64 GetBufferedPosition();

[Ami GONE FROM CHROMIUM, 2012/07/09 00:25:46]

hawt!

-
   // Gets the content length in bytes of the instance after this loader has been
   // started. If this value is |kPositionNotSpecified|, then content length is
   // unknown.
   int64 content_length();
 
   // Gets the original size of the file requested. If this value is
   // |kPositionNotSpecified|, then the size is unknown.
   int64 instance_size();
 
   // Returns true if the server supports byte range requests.
@@ skipping 109 matching lines @@
   std::string GenerateHeaders(int64 first_byte_position,
                               int64 last_byte_position);
 
   // Done with read. Invokes the read callback and reset parameters for the
   // read request.
   void DoneRead(Status status, int bytes_read);
 
   // Done with start. Invokes the start callback and reset it.
   void DoneStart(Status status);
 
-  // Calls |event_cb_| in terms of a network event.
-  void NotifyNetworkEvent();
-
   bool HasPendingRead() { return !read_cb_.is_null(); }
 
   // Helper function that returns true if a range request was specified.
   bool IsRangeRequest() const;
 
   // Log everything interesting to |media_log_|.
   void Log();
 
   // A sliding window of buffer.
   media::SeekableBuffer buffer_;
@@ skipping 16 matching lines @@
   bool range_supported_;
 
   // Forward capacity to reset to after an extension.
   size_t saved_forward_capacity_;
 
   GURL url_;
   int64 first_byte_position_;
   int64 last_byte_position_;
   bool single_origin_;
 
-  // Closure that listens to network events.
-  base::Closure event_cb_;
+  // Executed whenever the state of resource loading has changed.
+  LoadingCB loading_cb_;

[Ami GONE FROM CHROMIUM, 2012/07/09 00:25:46]

LoadingStateChangedCB?
LoadingCB sounds to me like something that'll fire exactly once (when loading
kicks off).  But LSCCB is a mouthful...
Up to you, just wanted to flag opportunity for improved naming.

[scherkus (not reviewing), 2012/07/09 18:53:30]

It's a mouthful but I agree that LoadingCB is a bit deceptive

+
+  // Executed whenever additional data has been downloaded and reports the
+  // zero-indexed file offset of the leading buffered byte.

[Ami GONE FROM CHROMIUM, 2012/07/09 00:25:46]

s/leading/furthest/?

[scherkus (not reviewing), 2012/07/09 18:53:30]

Done.

+  ProgressCB progress_cb_;
 
   // Members used during request start.
   StartCB start_cb_;
   int64 offset_;
   int64 content_length_;
   int64 instance_size_;
 
   // Members used during a read operation. They should be reset after each
   // read has completed or failed.
   ReadCB read_cb_;
@@ skipping 16 matching lines @@
   float playback_rate_;
 
   scoped_refptr<media::MediaLog> media_log_;
 
   DISALLOW_COPY_AND_ASSIGN(BufferedResourceLoader);
 };
 
 }  // namespace webkit_media
 
 #endif  // WEBKIT_MEDIA_BUFFERED_RESOURCE_LOADER_H_