Tie multibuffers to URLs

media/blink/resource_multibuffer_data_provider.h

+// Copyright 2015 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 MEDIA_BLINK_RESOURCE_MULTIBUFFER_DATA_PROVIDER_H_
+#define MEDIA_BLINK_RESOURCE_MULTIBUFFER_DATA_PROVIDER_H_
+
+#include <string>
+
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "media/blink/active_loader.h"
+#include "media/blink/media_blink_export.h"
+#include "media/blink/multibuffer.h"
+#include "media/blink/url_index.h"
+#include "third_party/WebKit/public/platform/WebURLLoader.h"
+#include "third_party/WebKit/public/platform/WebURLLoaderClient.h"
+#include "third_party/WebKit/public/platform/WebURLRequest.h"
+#include "third_party/WebKit/public/web/WebFrame.h"
+#include "url/gurl.h"
+
+namespace media {
+class MediaLog;

[xhwang, 2015/11/19 23:34:17]

not used?

[hubbe, 2015/11/20 23:24:23]

Done.

+class ResourceMultiBuffer;

[xhwang, 2015/11/19 23:34:18]

not used as well?

[hubbe, 2015/11/20 23:24:23]

Done.

+
+class MEDIA_BLINK_EXPORT ResourceMultiBufferDataProvider
+    : NON_EXPORTED_BASE(public MultiBuffer::DataProvider),
+      NON_EXPORTED_BASE(public blink::WebURLLoaderClient) {
+ public:
+  ResourceMultiBufferDataProvider(UrlData* url_data, MultiBufferBlockId pos);
+  ~ResourceMultiBufferDataProvider() override;
+
+  // MultiBufferDataProvider implementation

[xhwang, 2015/11/19 23:34:18]

MultiBuffer::DataProvider ?

[hubbe, 2015/11/20 23:24:23]

Done.

+  MultiBufferBlockId Tell() const override;
+  bool Available() const override;
+  scoped_refptr<DataBuffer> Read() override;
+  void SetDeferred(bool defer) override;
+
+  // blink::WebURLLoaderClient implementation.
+  void willFollowRedirect(
+      blink::WebURLLoader* loader,
+      blink::WebURLRequest& newRequest,
+      const blink::WebURLResponse& redirectResponse) override;
+  void didSendData(blink::WebURLLoader* loader,
+                   unsigned long long bytesSent,
+                   unsigned long long totalBytesToBeSent) override;
+  void didReceiveResponse(blink::WebURLLoader* loader,
+                          const blink::WebURLResponse& response) override;
+  void didDownloadData(blink::WebURLLoader* loader,
+                       int data_length,
+                       int encoded_data_length) override;
+  void didReceiveData(blink::WebURLLoader* loader,
+                      const char* data,
+                      int data_length,
+                      int encoded_data_length) override;
+  void didReceiveCachedMetadata(blink::WebURLLoader* loader,
+                                const char* data,
+                                int dataLength) override;
+  void didFinishLoading(blink::WebURLLoader* loader,
+                        double finishTime,
+                        int64_t total_encoded_data_length) override;
+  void didFail(blink::WebURLLoader* loader, const blink::WebURLError&) override;
+
+  // Parse a Content-Range header into its component pieces and return true if
+  // each of the expected elements was found & parsed correctly.
+  // |*instance_size| may be set to kPositionNotSpecified if the range ends in
+  // "/*".
+  // NOTE: only public for testing!  This is an implementation detail of
+  // VerifyPartialResponse (a private method).

[xhwang, 2015/11/19 23:34:18]

Not seeing how this will be tested... 

Given we have two test classes as friend, do we still need this to be public?

Also, I wonder whether we should test implementation detail instead of
interfaces. If the latter, we can just define it as a file scope static function
in the .cc file.

[hubbe, 2015/11/20 23:24:23]

Test is in a later CL I'm afraid.
I don't think it needs to be public, fixed.

+  static bool ParseContentRange(const std::string& content_range_str,
+                                int64* first_byte_position,
+                                int64* last_byte_position,
+                                int64* instance_size);
+
+  // Virtual for testing purposes.

[xhwang, 2015/11/19 23:34:18]

I don't see how this is used for testing and why it needs to be virtual here...

[hubbe, 2015/11/20 23:24:23]

Unfortunately tests are in a later CL.

[xhwang, 2015/11/23 23:09:21]

Then does it make sense to add Start() in that later CL?

[hubbe, 2015/11/24 22:55:10]

No, Start() is needed already, but I could remove "virtual" for now if you like.

+  virtual void Start();
+
+ protected:

[xhwang, 2015/11/19 23:34:18]

Why not private?

[hubbe, 2015/11/20 23:24:23]

Comment added.

+  friend class MultibufferDataSourceTest;
+  friend class MultibufferResourceLoaderTest;
+  friend class MockBufferedDataSource;
+
+  int64_t byte_pos() const;
+  int64_t block_size() const;

[xhwang, 2015/11/19 23:34:18]

nit: empty line here

[hubbe, 2015/11/20 23:24:23]

Done.

+  // If we have made a range request, verify the response from the server.
+  bool VerifyPartialResponse(const blink::WebURLResponse& response);
+
+  // Current Position.
+  MultiBufferBlockId pos_;
+
+  // This is where we actually get read data from.
+  // We don't need (or want) a scoped_refptr for this one, because
+  // we are owned by it.
+  UrlData* url_data_;

[xhwang, 2015/11/19 23:34:18]

const?

[hubbe, 2015/11/20 23:24:23]

Unfortunately no, we change it when we change owners...

[xhwang, 2015/11/23 23:09:21]

Interesting fact! Worth a comment?

[hubbe, 2015/11/24 22:55:10]

Absolutely.

+
+  // Temporary storage for incoming data.
+  std::list<scoped_refptr<DataBuffer>> fifo_;
+
+  // How many retries have we done at the current position.
+  int retries_;
+
+  // Copy of url_data_->cors_mode()
+  // const to make it obvious that redirects cannot change it.
+  const UrlData::CORSMode cors_mode_;
+
+  // The origin for the initial request.
+  // const to make it obvious that redirects cannot change it.
+  const GURL origin_;
+
+  // Keeps track of an active WebURLLoader and associated state.
+  scoped_ptr<ActiveLoader> active_loader_;
+
+  // When we encounter a redirect, this is the source of the redirect.
+  GURL redirects_to_;
+
+  base::WeakPtrFactory<ResourceMultiBufferDataProvider> weak_factory_;
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BLINK_RESOURCE_MULTIBUFFER_RESOURCE_LOADER_H_