[d8] Use ValueSerializer for postMessage (instead of ad-hoc serializer)

Review-Url: https://codereview.chromium.org/2643723010
Cr-Commit-Position: refs/heads/master@{#42749}

include/v8.h

@@ -1740,6 +1740,10 @@ class V8_EXPORT ValueSerializer {
      * Allocates memory for the buffer of at least the size provided. The actual
      * size (which may be greater or equal) is written to |actual_size|. If no
      * buffer has been allocated yet, nullptr will be provided.
+     *
+     * If the memory cannot be allocated, nullptr should be returned.
+     * |actual_size| will be ignored. It is assumed that |old_buffer| is still
+     * valid in this case and has not been modified.
      */
     virtual void* ReallocateBufferMemory(void* old_buffer, size_t size,
                                          size_t* actual_size);

src/d8.h

@@ -5,9 +5,13 @@
 #ifndef V8_D8_H_
 #define V8_D8_H_
 
+#include <memory>
 #include <string>
+#include <unordered_set>
+#include <vector>
 
 #include "src/allocation.h"
+#include "src/base/functional.h"
 #include "src/base/hashmap.h"
 #include "src/base/platform/time.h"
 #include "src/list.h"
@@ -143,68 +147,51 @@ class SourceGroup {
   int end_offset_;
 };
 
-enum SerializationTag {
-  kSerializationTagUndefined,
-  kSerializationTagNull,
-  kSerializationTagTrue,
-  kSerializationTagFalse,
-  kSerializationTagNumber,
-  kSerializationTagString,
-  kSerializationTagArray,
-  kSerializationTagObject,
-  kSerializationTagArrayBuffer,
-  kSerializationTagTransferredArrayBuffer,
-  kSerializationTagTransferredSharedArrayBuffer,
-};
-
 
 class SerializationData {
  public:
-  SerializationData() {}
+  SerializationData() : data_(nullptr), size_(0) {}
   ~SerializationData();
 
-  void WriteTag(SerializationTag tag);
-  void WriteMemory(const void* p, int length);
-  void WriteArrayBufferContents(const ArrayBuffer::Contents& contents);
-  void WriteSharedArrayBufferContents(
-      const SharedArrayBuffer::Contents& contents);
-
-  template <typename T>
-  void Write(const T& data) {
-    WriteMemory(&data, sizeof(data));
+  uint8_t* data() { return data_.get(); }
+  size_t size() { return size_; }
+  const std::vector<ArrayBuffer::Contents>& array_buffer_contents() {
+    return array_buffer_contents_;
   }
-
-  SerializationTag ReadTag(int* offset) const;
-  void ReadMemory(void* p, int length, int* offset) const;
-  void ReadArrayBufferContents(ArrayBuffer::Contents* contents,
-                               int* offset) const;
-  void ReadSharedArrayBufferContents(SharedArrayBuffer::Contents* contents,
-                                     int* offset) const;
-
-  template <typename T>
-  T Read(int* offset) const {
-    T value;
-    ReadMemory(&value, sizeof(value), offset);
-    return value;
+  const std::vector<SharedArrayBuffer::Contents>&
+  shared_array_buffer_contents() {
+    return shared_array_buffer_contents_;
   }
 
+  void ClearTransferredArrayBuffers();
+
+ private:
+  struct DataDeleter {
+    void operator()(uint8_t* p) const { free(p); }
+  };
+
+  std::unique_ptr<uint8_t, DataDeleter> data_;
+  size_t size_;
+  std::vector<ArrayBuffer::Contents> array_buffer_contents_;
+  std::vector<SharedArrayBuffer::Contents> shared_array_buffer_contents_;
+
  private:
-  i::List<uint8_t> data_;
-  i::List<ArrayBuffer::Contents> array_buffer_contents_;
-  i::List<SharedArrayBuffer::Contents> shared_array_buffer_contents_;
+  friend class Serializer;
+
+  DISALLOW_COPY_AND_ASSIGN(SerializationData);
 };
 
 
 class SerializationDataQueue {
  public:
-  void Enqueue(SerializationData* data);
-  bool Dequeue(SerializationData** data);
+  void Enqueue(std::unique_ptr<SerializationData> data);
+  bool Dequeue(std::unique_ptr<SerializationData>* data);
   bool IsEmpty();
   void Clear();
 
  private:
   base::Mutex mutex_;
-  i::List<SerializationData*> data_;
+  std::vector<std::unique_ptr<SerializationData>> data_;
 };
 
 
@@ -219,13 +206,13 @@ class Worker {
   // Post a message to the worker's incoming message queue. The worker will
   // take ownership of the SerializationData.
   // This function should only be called by the thread that created the Worker.
-  void PostMessage(SerializationData* data);
+  void PostMessage(std::unique_ptr<SerializationData> data);
   // Synchronously retrieve messages from the worker's outgoing message queue.
   // If there is no message in the queue, block until a message is available.
   // If there are no messages in the queue and the worker is no longer running,
   // return nullptr.
   // This function should only be called by the thread that created the Worker.
-  SerializationData* GetMessage();
+  std::unique_ptr<SerializationData> GetMessage();
   // Terminate the worker's event loop. Messages from the worker that have been
   // queued can still be read via GetMessage().
   // This function can be called by any thread.
@@ -335,16 +322,10 @@ class Shell : public i::AllStatic {
   static void CollectGarbage(Isolate* isolate);
   static void EmptyMessageQueues(Isolate* isolate);
 
-  // TODO(binji): stupid implementation for now. Is there an easy way to hash an
-  // object for use in base::HashMap? By pointer?
-  typedef i::List<Local<Object>> ObjectList;
-  static bool SerializeValue(Isolate* isolate, Local<Value> value,
-                             const ObjectList& to_transfer,
-                             ObjectList* seen_objects,
-                             SerializationData* out_data);
-  static MaybeLocal<Value> DeserializeValue(Isolate* isolate,
-                                            const SerializationData& data,
-                                            int* offset);
+  static std::unique_ptr<SerializationData> SerializeValue(
+      Isolate* isolate, Local<Value> value, Local<Value> transfer);
+  static MaybeLocal<Value> DeserializeValue(
+      Isolate* isolate, std::unique_ptr<SerializationData> data);
   static void CleanupWorkers();
   static int* LookupCounter(const char* name);
   static void* CreateHistogram(const char* name,
@@ -443,10 +424,26 @@ class Shell : public i::AllStatic {
   static base::LazyMutex context_mutex_;
   static const base::TimeTicks kInitialTicks;
 
+  struct SharedArrayBufferContentsHash {
+    size_t operator()(const v8::SharedArrayBuffer::Contents& contents) const {
+      return base::hash_combine(contents.Data(), contents.ByteLength());
+    }
+  };
+
+  struct SharedArrayBufferContentsIsEqual {
+    bool operator()(const SharedArrayBuffer::Contents& a,
+                    const SharedArrayBuffer::Contents& b) const {
+      return a.Data() == b.Data() && a.ByteLength() == b.ByteLength();
+    }
+  };
+
   static base::LazyMutex workers_mutex_;
   static bool allow_new_workers_;
   static i::List<Worker*> workers_;
-  static i::List<SharedArrayBuffer::Contents> externalized_shared_contents_;
+  static std::unordered_set<SharedArrayBuffer::Contents,
+                            SharedArrayBufferContentsHash,
+                            SharedArrayBufferContentsIsEqual>
+      externalized_shared_contents_;
 
   static void WriteIgnitionDispatchCountersFile(v8::Isolate* isolate);
   static Counter* GetCounter(const char* name, bool is_histogram);

src/messages.h

@@ -676,6 +676,7 @@ class ErrorUtils : public AllStatic {
   T(AsmJsInstantiated, "Instantiated asm.js: %")                               \
   /* DataCloneError messages */                                                \
   T(DataCloneError, "% could not be cloned.")                                  \
+  T(DataCloneErrorOutOfMemory, "Data cannot be cloned, out of memory.")        \
   T(DataCloneErrorNeuteredArrayBuffer,                                         \
     "An ArrayBuffer is neutered and could not be cloned.")                     \
   T(DataCloneErrorSharedArrayBufferTransferred,                                \

src/value-serializer.h

@@ -86,7 +86,7 @@ class ValueSerializer {
 
  private:
   // Managing allocations of the internal buffer.
-  void ExpandBuffer(size_t required_capacity);
+  Maybe<bool> ExpandBuffer(size_t required_capacity);
 
   // Writing the wire format.
   void WriteTag(SerializationTag tag);
@@ -96,7 +96,7 @@ class ValueSerializer {
   void WriteZigZag(T value);
   void WriteOneByteString(Vector<const uint8_t> chars);
   void WriteTwoByteString(Vector<const uc16> chars);
-  uint8_t* ReserveRawBytes(size_t bytes);
+  Maybe<uint8_t*> ReserveRawBytes(size_t bytes);
 
   // Writing V8 objects of various kinds.
   void WriteOddball(Oddball* oddball);
@@ -134,11 +134,14 @@ class ValueSerializer {
   V8_NOINLINE void ThrowDataCloneError(MessageTemplate::Template template_index,
                                        Handle<Object> arg0);
 
+  Maybe<bool> ThrowIfOutOfMemory();
+
   Isolate* const isolate_;
   v8::ValueSerializer::Delegate* const delegate_;
   uint8_t* buffer_ = nullptr;
   size_t buffer_size_ = 0;
   size_t buffer_capacity_ = 0;
+  bool out_of_memory_ = false;
   Zone zone_;
 
   // To avoid extra lookups in the identity map, ID+1 is actually stored in the

test/mjsunit/d8-worker-sharedarraybuffer.js

@@ -28,20 +28,19 @@
 // Flags: --harmony-sharedarraybuffer
 
 if (this.Worker) {
-
   (function TestTransfer() {
     var workerScript =
       `onmessage = function(m) {
-       var sab = m;
-       var ta = new Uint32Array(sab);
-       if (sab.byteLength !== 16) {
-         throw new Error('SharedArrayBuffer transfer byteLength');
-       }
-       for (var i = 0; i < 4; ++i) {
-         if (ta[i] !== i) {
-           throw new Error('SharedArrayBuffer transfer value ' + i);
-         }
-       }
+        var sab = m;
+        var ta = new Uint32Array(sab);
+        if (sab.byteLength !== 16) {
+          throw new Error('SharedArrayBuffer transfer byteLength');
+        }
+        for (var i = 0; i < 4; ++i) {
+          if (ta[i] !== i) {
+            throw new Error('SharedArrayBuffer transfer value ' + i);
+          }
+        }
         // Atomically update ta[0]
         Atomics.store(ta, 0, 100);
       };`;
@@ -55,7 +54,7 @@ if (this.Worker) {
     }
 
     // Transfer SharedArrayBuffer
-    w.postMessage(sab, [sab]);
+    w.postMessage(sab);
     assertEquals(16, sab.byteLength);  // ArrayBuffer should not be neutered.
 
     // Spinwait for the worker to update ta[0]
@@ -86,7 +85,7 @@ if (this.Worker) {
     var workers = [];
     for (id = 0; id < 4; ++id) {
       workers[id] = new Worker(workerScript);
-      workers[id].postMessage({sab: sab, id: id}, [sab]);
+      workers[id].postMessage({sab: sab, id: id});
     }
 
     // Spinwait for each worker to update ta[id]

test/mjsunit/d8-worker.js

@@ -61,8 +61,8 @@ var workerScript =
          }
          break;
        case 7:
-         if (JSON.stringify(m) !== \"{'a':1,'b':2.5,'c':'three'}\")
-           throw new Error('Object');
+         if (JSON.stringify(m) !== '{"a":1,"b":2.5,"c":"three"}')
+           throw new Error('Object' + JSON.stringify(m));
          break;
        case 8:
          var ab = m;
@@ -88,7 +88,6 @@ var workerScript =
      }
    };`;
 
-
 if (this.Worker) {
   function createArrayBuffer(byteLength) {
     var ab = new ArrayBuffer(byteLength);
@@ -111,6 +110,17 @@ if (this.Worker) {
   w.postMessage([4, true, "bye"]);
   w.postMessage({a: 1, b: 2.5, c: "three"});
 
+  // Test bad get in transfer list.
+  var transferList = [undefined];
+  Object.defineProperty(transferList, '0', {
+    get: function() {
+      throw 'unexpected!';
+    }
+  });
+  assertThrows(function() {
+    w.postMessage([], transferList);
+  });
+
   // Clone ArrayBuffer
   var ab1 = createArrayBuffer(16);
   w.postMessage(ab1);

test/mjsunit/harmony/futex.js

@@ -120,7 +120,7 @@ if (this.Worker) {
        };`;
 
     var worker = new Worker(workerScript);
-    worker.postMessage({sab: sab, offset: offset}, [sab]);
+    worker.postMessage({sab: sab, offset: offset});
 
     // Spin until the worker is waiting on the futex.
     while (%AtomicsNumWaitersForTesting(i32a, 0) != 1) {}
@@ -132,7 +132,7 @@ if (this.Worker) {
     var worker2 = new Worker(workerScript);
     var offset = 8;
     var i32a2 = new Int32Array(sab, offset);
-    worker2.postMessage({sab: sab, offset: offset}, [sab]);
+    worker2.postMessage({sab: sab, offset: offset});
 
     // Spin until the worker is waiting on the futex.
     while (%AtomicsNumWaitersForTesting(i32a2, 0) != 1) {}
@@ -144,7 +144,7 @@ if (this.Worker) {
     // the real address is the same.
     var worker3 = new Worker(workerScript);
     i32a2 = new Int32Array(sab, 4);
-    worker3.postMessage({sab: sab, offset: 8}, [sab]);
+    worker3.postMessage({sab: sab, offset: 8});
 
     // Spin until the worker is waiting on the futex.
     while (%AtomicsNumWaitersForTesting(i32a2, 1) != 1) {}
@@ -190,7 +190,7 @@ if (this.Worker) {
     var workers = [];
     for (id = 0; id < 4; id++) {
       workers[id] = new Worker(workerScript);
-      workers[id].postMessage({sab: sab, id: id}, [sab]);
+      workers[id].postMessage({sab: sab, id: id});
     }
 
     // Spin until all workers are waiting on the futex.

test/mjsunit/regress/regress-crbug-514081.js

@@ -4,10 +4,15 @@
 
 if (this.Worker) {
   var __v_7 = new Worker('onmessage = function() {};');
+  var e;
   try {
     var ab = new ArrayBuffer(2147483648);
-    // If creating the ArrayBuffer succeeded, then postMessage should fail.
-    assertThrows(function() { __v_7.postMessage(ab); });
+    try {
+      __v_7.postMessage(ab);
+    } catch (e) {
+      // postMessage failed, should be a DataCloneError message.
+      assertContains('cloned', e.message);
+    }
   } catch (e) {
     // Creating the ArrayBuffer failed.
     assertInstanceof(e, RangeError);