Fix out-of-memory crashes related to ArrayBuffer allocation

third_party/WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp

 /*
  * Copyright (C) 2011 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:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 78 matching lines @@
     switch (key->type()) {
     case IDBKey::InvalidType:
     case IDBKey::MinType:
         ASSERT_NOT_REACHED();
         return v8Undefined();
     case IDBKey::NumberType:
         return v8::Number::New(isolate, key->number());
     case IDBKey::StringType:
         return v8String(isolate, key->string());
     case IDBKey::BinaryType:
-        // Experimental feature: binary keys
-        // https://w3c.github.io/IndexedDB/#steps-to-convert-a-key-to-a-value
-        return toV8(DOMArrayBuffer::create(reinterpret_cast<const unsigned char*>(key->binary()->data()), key->binary()->size()), creationContext, isolate);
+        {
+            // Experimental feature: binary keys
+            // https://w3c.github.io/IndexedDB/#steps-to-convert-a-key-to-a-value
+            RefPtr<DOMArrayBuffer> buffer = DOMArrayBuffer::createOrNull(reinterpret_cast<const unsigned char*>(key->binary()->data()), key->binary()->size());
+            if (!buffer) {
+                isolate->ThrowException(v8::Exception::RangeError(v8::String::NewFromUtf8(isolate, "Out of memory. Failed to allocate ArrayBuffer.")));

[jsbell, 2015/10/16 22:12:24]

This is going to be weird behavior! The scenario will be something like this:

var request = index.getKey(query); // assume result has binary key
request.onerror = () => console.log(request.error);
request.onsuccess = () => console.log(request.result); // throws here

The request will fire 'success' rather than 'error' since the query succeeded.
It's only at the point where `request.result` is called that we hit this code
path to convert the internal IDBKey type to an array buffer, and boom!

What's even stranger is that the result of the conversion is *cached* so if
`request.result` is called again... I'm not sure what will happen. Relevant code
is:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Also, we're thinking of adding Promise helpers here, so it might read as:

index.getKey(query).ready.then((result) => {
  ...
});

That's not spec'd/coded yet but... wow, not sure what should happen there.
Convert the fulfill into a reject halfway through processing??? Fun!

(The good news is that binary key support is experimental, so we have
flexibility - it might make the most sense to artificially cap the key size and
crash on OOM here etc)

[jsbell, 2015/10/16 22:16:26]

Hrm, I'm an idiot... this *also* applies to *values* returned by Indexed DB,
that's just hidden inside the SerializedScriptValue code, no module-specific
bindings code. Same basic code flows will be affected though, e.g. 

var request = store.get(key); // assume result contains cloned arraybuffer
request.onerror = () => console.log(request.error);
request.onsuccess = () => console.log(request.result); // throws here

At the point where `request.result` is accessed we dive into the
SerializedScriptValue code to deserialize, and boom. So... not just an
"experimental" issue. Fun.

[Justin Novosad, 2015/10/19 16:42:52]

For this initial CL, I want to stay clear of anything controversial, So I could
just avoid the change in behavior here by calling
DOMArrayBuffer::deprecatedCreateOrCrash, and file a separate issue so that this
case can be discussed and resolved in isolation.

+                return v8Undefined();
+            }
+            return toV8(buffer, creationContext, isolate);
+        }
     case IDBKey::DateType:
         return v8::Date::New(context, key->date()).ToLocalChecked();
     case IDBKey::ArrayType:
         {
             v8::Local<v8::Array> array = v8::Array::New(isolate, key->array().size());
             for (size_t i = 0; i < key->array().size(); ++i) {
                 v8::Local<v8::Value> value = toV8(key->array()[i].get(), creationContext, isolate);
                 if (value.IsEmpty())
                     value = v8::Undefined(isolate);
                 if (!v8CallBoolean(array->CreateDataProperty(context, i, value)))
@@ skipping 66 matching lines @@
             if (buffer->isNeutered()) {
                 exceptionState.throwTypeError("The ArrayBuffer is neutered.");
                 return nullptr;
             }
             const char* start = static_cast<const char*>(buffer->data());
             size_t length = buffer->byteLength();
             return IDBKey::createBinary(SharedBuffer::create(start, length));
         }
         if (value->IsArrayBufferView()) {
             DOMArrayBufferView* view = V8ArrayBufferView::toImpl(value.As<v8::Object>());
-            if (view->buffer()->isNeutered()) {
+            RefPtr<DOMArrayBuffer> buffer = view->bufferOrNull();
+            if (!buffer) {

[jsbell, 2015/10/16 22:12:24]

The usage here is that script has passed a view in and we're getting at the
underlying buffer. Is it a supported scenario with this CL that script is now
able to hold on to views with null buffers? That seems odd - ISTM that the
intent is that a RangeError would be thrown before such a broken view could ever
be created.

(i.e. I'd expect this to be an ASSERT instead)

[Justin Novosad, 2015/10/19 16:42:52]

Acknowledged.

+                exceptionState.throwRangeError("Out of Momory.");

[binji, 2015/10/16 22:12:39]

sp: memory

[Justin Novosad, 2015/10/19 16:42:52]

Acknowledged.

+                return nullptr;
+            }
+            if (buffer->isNeutered()) {
                 exceptionState.throwTypeError("The viewed ArrayBuffer is neutered.");
                 return nullptr;
             }
             const char* start = static_cast<const char*>(view->baseAddress());
             size_t length = view->byteLength();
             return IDBKey::createBinary(SharedBuffer::create(start, length));
         }
     }
     if (value->IsArray()) {
         v8::Local<v8::Array> array = value.As<v8::Array>();
@@ skipping 312 matching lines @@
     ASSERT(!exceptionState.hadException());
     if (expectedKey && expectedKey->isEqual(value->primaryKey()))
         return;
 
     bool injected = injectV8KeyIntoV8Value(isolate, keyValue.v8Value(), scriptValue.v8Value(), value->keyPath());
     ASSERT_UNUSED(injected, injected);
 }
 #endif
 
 } // namespace blink