Implement File constructor.

Source/bindings/v8/custom/V8BlobCustom.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 13 matching lines @@
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "V8Blob.h"
 
-#include "bindings/v8/Dictionary.h"
-#include "bindings/v8/ExceptionMessages.h"
-#include "bindings/v8/V8Binding.h"
-#include "bindings/v8/V8Utilities.h"
-#include "bindings/v8/custom/V8ArrayBufferCustom.h"
-#include "bindings/v8/custom/V8ArrayBufferViewCustom.h"
+#include "bindings/v8/custom/V8BlobCustomHelpers.h"
 #include "core/fileapi/BlobBuilder.h"
 #include "wtf/RefPtr.h"

[haraken, 2013/11/15 08:40:05]

Nit: This won't be needed.

[pwnall-personal, 2013/11/15 10:28:53]

Done.
Thank you!

 
 namespace WebCore {
 
 void V8Blob::constructorCustom(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
     if (!info.Length()) {
         RefPtr<Blob> blob = Blob::create();
         info.GetReturnValue().Set(toV8(blob.get(), info.Holder(), info.GetIsolate()));

[haraken, 2013/11/15 08:40:05]

Use v8SetReturnValue() in V8Binding.h.

[pwnall-personal, 2013/11/15 10:28:53]

Done.
I just looked up how this works. It's really cool! Thank you for pointing it
out!

         return;
     }
 
     uint32_t length = 0;
     if (info[0]->IsArray()) {
         length = v8::Local<v8::Array>::Cast(info[0])->Length();
     } else {
         const int sequenceArgumentIndex = 0;
         if (toV8Sequence(info[sequenceArgumentIndex], length, info.GetIsolate()).IsEmpty()) {

[haraken, 2013/11/15 08:40:05]

I don't fully understand this code. You're calling toV8Sequence() for info[0],
which is not an Array. Then toV8Sequence() will always return false, won't it?

[pwnall-personal, 2013/11/15 10:28:53]

FWIW, I think the toV8Sequence() method name is misleading w.r.t. its behavior.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Actually, it _asserts_ that its argument is not an array, so each caller is
required to have a separate code path for arrays. Our current definition of
"sequence" is an object (that is not RegExp or Date) with a "length" property
that is a 32-bit unsigned integer. I think this matches the current WebIDL spec.
http://heycam.github.io/webidl/#es-sequence

The discussion in the File constructor bug on Mozilla's tracker makes me believe
the spec will change in the future.
https://bugzilla.mozilla.org/show_bug.cgi?id=819900

If it is desirable, I can try to prepare a CL that makes toV8Sequence accept
arrays and pass them through. Would you like me to do that?

[haraken, 2013/11/15 10:39:12]

Ah, got it. I'd be happy if you could make the change in a follow-up. Per the
Web IDL spec (http://www.w3.org/TR/2012/CR-WebIDL-20120419/#es-sequence),
'sequence' should accept an array.

             throwTypeError(ExceptionMessages::failedToConstruct("Blob", ExceptionMessages::notAnArrayTypeArgumentOrValue(sequenceArgumentIndex + 1)), info.GetIsolate());
             return;
         }
     }
 
-    String type;
+    String contentType;
     String endings = "transparent";

[haraken, 2013/11/15 08:40:05]

Nit: How about setting 'endings = "transparent"' in processBlobPropertyBag() ?
It's clearer to set both contentType and endings in processBlobPropertyBag().

[pwnall-personal, 2013/11/15 10:28:53]

I can't get rid of the default easily, because processBlobPropertyBag() is not
called at all if the optional argument is ignored. I added an ASSERT to the
beginning to document that I expect the caller to set endings to "transparent".
Is this good enough?

Alternatively, I can add a struct and move the initialization to its constructor
so we don't need magic values in V8{Blob,File}::constructorCustom, but my
intuition tells me that is overkill. Would you like me to do that? Something
else?

I also moved the ASSERT to the beginning of processBlobParts(), because it
really looks like a precondition check to me.

[haraken, 2013/11/15 10:39:12]

Understood. Let's keep it as is.

-
     if (info.Length() > 1) {
         if (!info[1]->IsObject()) {
             throwTypeError(ExceptionMessages::failedToConstruct("Blob", "The 2nd argument is not of type Object."), info.GetIsolate());
             return;
         }
 
-        V8TRYCATCH_VOID(Dictionary, dictionary, Dictionary(info[1], info.GetIsolate()));
-
-        V8TRYCATCH_VOID(bool, containsEndings, dictionary.get("endings", endings));
-        if (containsEndings) {
-            if (endings != "transparent" && endings != "native") {
-                throwTypeError(ExceptionMessages::failedToConstruct("Blob", "The 2nd argument's \"endings\" property must be either \"transparent\" or \"native\"."), info.GetIsolate());
-                return;
-            }
-        }
-
-        V8TRYCATCH_VOID(bool, containsType, dictionary.get("type", type));
-        UNUSED_PARAM(containsType);
-        if (!type.containsOnlyASCII()) {
-            throwError(v8SyntaxError, ExceptionMessages::failedToConstruct("Blob", "The 2nd argument's \"type\" property must consist of ASCII characters."), info.GetIsolate());
+        if (!V8BlobCustomHelpers::processBlobPropertyBag(info[1], info.GetIsolate(), "Blob", contentType, endings))

[haraken, 2013/11/15 08:40:05]

processBlobPropertyBag => processBlobDictionary ?

[pwnall-personal, 2013/11/15 08:58:42]

I used BlobPropertyBag because that's the name that the File API uses.
http://dev.w3.org/2006/webapi/FileAPI/#dfn-BlobPropertyBag

Should I still change it?

[haraken, 2013/11/15 09:09:40]

Then let's call it processBlobPropertyBag. Sorry about the noise.

[pwnall-personal, 2013/11/15 10:28:53]

FWIW, I also find the name a bit awkward :)

             return;
-        }
-        type = type.lower();
     }
-
     ASSERT(endings == "transparent" || endings == "native");
 
     BlobBuilder blobBuilder;
     v8::Local<v8::Object> blobParts = v8::Local<v8::Object>::Cast(info[0]);
-    for (uint32_t i = 0; i < length; ++i) {
-        v8::Local<v8::Value> item = blobParts->Get(v8::Uint32::New(i, info.GetIsolate()));
-        if (item.IsEmpty())
-            return;
+    if (!V8BlobCustomHelpers::processBlobParts(blobParts, length, info.GetIsolate(), endings, blobBuilder))
+        return;
 
-        if (V8ArrayBuffer::hasInstance(item, info.GetIsolate(), worldType(info.GetIsolate()))) {
-            ArrayBuffer* arrayBuffer = V8ArrayBuffer::toNative(v8::Handle<v8::Object>::Cast(item));
-            ASSERT(arrayBuffer);
-            blobBuilder.append(arrayBuffer);
-        } else if (V8ArrayBufferView::hasInstance(item, info.GetIsolate(), worldType(info.GetIsolate()))) {
-            ArrayBufferView* arrayBufferView = V8ArrayBufferView::toNative(v8::Handle<v8::Object>::Cast(item));
-            ASSERT(arrayBufferView);
-            blobBuilder.append(arrayBufferView);
-        } else if (V8Blob::hasInstance(item, info.GetIsolate(), worldType(info.GetIsolate()))) {
-            Blob* blob = V8Blob::toNative(v8::Handle<v8::Object>::Cast(item));
-            ASSERT(blob);
-            blobBuilder.append(blob);
-        } else {
-            V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<>, stringValue, item);
-            blobBuilder.append(stringValue, endings);
-        }
-    }
-
-    RefPtr<Blob> blob = blobBuilder.getBlob(type);
+    RefPtr<Blob> blob = blobBuilder.createBlob(contentType);
     info.GetReturnValue().Set(toV8(blob.get(), info.Holder(), info.GetIsolate()));

[haraken, 2013/11/15 08:40:05]

Use v8SetReturnValue() in V8Binding.h.

[pwnall-personal, 2013/11/15 10:28:53]

Done.

 }
 
 } // namespace WebCore