[2a] Implement FetchDataLoader

Source/modules/fetch/FetchDataLoader.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 FetchDataLoader_h
+#define FetchDataLoader_h
+
+#include "core/dom/DOMArrayBuffer.h"
+#include "modules/fetch/FetchDataConsumerHandle.h"
+#include "platform/blob/BlobData.h"
+#include "wtf/ArrayBufferBuilder.h"

[yhirano, 2015/06/17 05:18:54]

+wtf/Forward.h

[yhirano, 2015/06/17 05:18:54]

Why do we need ArrayBufferBuilder.h?

[hiroshige, 2015/06/17 08:48:55]

Moved to .cpp.

[hiroshige, 2015/06/17 08:48:55]

Done.

+
+namespace blink {
+
+// FetchDataLoader subclasses
+// 1. takes a reader of FetchDataConsumerHandle |handle|,
+// 2. reads all data, and
+// 3. calls either didFetchDataLoaded...() on success or
+//    difFetchDataLoadFailed() otherwise.
+//
+// Client's methods can be called synchronously in start().
+// Client's methods can destruct FetchDataLoader.
+// If FetchDataLoader is destructed before Client's methods are called,
+//     then the reader is also destructed and
+//     Client's methods are not called anymore.
+// Ownership of |handle|, |handle|'s reader, and Client:
+//     FetchDataLoader
+//     - takes the ownership of |handle|'s reader.
+//     - does not take the ownership of |handle|.
+//     - does not take the ownership of Client.
+//       The caller must guarantee Client is alive until
+//       any of Client's method is called or FetchDataLoader is destructed.
+// Threading:
+//     Client's methods are called and FetchDataLoader must be destructed
+//     on the thread where FetchDataLoader is created.
+class FetchDataLoader {

[yhirano, 2015/06/17 05:18:53]

It is possible to make this class GarbageCollectedFinalized and make Client
GarbageCollectedMixin. Then we can simplify the lifetime comments. What do you
think about it?

[hiroshige, 2015/06/17 18:27:29]

Done.

+public:
+    class Client {
+    public:
+        virtual ~Client() { }
+
+        // The method corresponding to createLoaderAs... is called on success.
+        virtual void didFetchDataLoadedBlobHandle(PassRefPtr<BlobDataHandle>)
+            { }
+        virtual void didFetchDataLoadedArrayBuffer(PassRefPtr<DOMArrayBuffer>)
+            { }
+        virtual void didFetchDataLoadedString(const String&) { }
+
+        virtual void didFetchDataLoadFailed() = 0;
+    };
+
+    static PassOwnPtr<FetchDataLoader> createLoaderAsBlobHandle(const String& mimeType);
+    static PassOwnPtr<FetchDataLoader> createLoaderAsArrayBuffer();
+    static PassOwnPtr<FetchDataLoader> createLoaderAsString();
+
+    virtual ~FetchDataLoader() { }
+
+    // start() do not take the ownership of |handle|.

[yhirano, 2015/06/17 05:18:54]

+"|handle| must not be locked when called".

[hiroshige, 2015/06/17 08:48:55]

Done.

+    virtual void start(FetchDataConsumerHandle* /* handle */, Client*) = 0;
+
+protected:
+    FetchDataLoader();

[yhirano, 2015/06/17 05:18:54]

You can make this constructor public (or delete it if you remove members): In
any case this class cannot be instantiated because it has a pure virtual
function.

[hiroshige, 2015/06/17 08:48:55]

Done.

+
+    OwnPtr<FetchDataConsumerHandle::Reader> m_reader;

[yhirano, 2015/06/17 05:18:54]

Can you move these members to derived classes? Then we can remove this protected
section.

[hiroshige, 2015/06/17 08:48:55]

Done.

+    Client* m_client;
+};
+
+} // namespace blink
+
+#endif // FetchDataLoader_h