[Webkit-unassigned] [Bug 63481] Bring V8's SerializedScriptValue implementation up to date

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63481


Luke Zarko <lukezarko+bugzilla at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #100738|0                           |1
        is obsolete|                            |
 Attachment #100881|                            |review?
               Flag|                            |




--- Comment #27 from Luke Zarko <lukezarko+bugzilla at gmail.com>  2011-07-14 15:56:05 PST ---
Created an attachment (id=100881)
 --> (https://bugs.webkit.org/attachment.cgi?id=100881&action=review)
Promote array buffer passing test and address comments.

> (array-message-passing)

I've moved this into the common tests directory and configured expectations appropriately.

> (Super low priority but) Want to patch one of them to make the error text the same? (Different results are painful.)

I will look into this; I hope it won't require too many changes to expectations.

> Do you think the ArrayBufferView::is* should change to an enum value (in a future patch)?

Yes, to permit exactly this kind of typecasing.

> What happens if the length given here doesn't agree with the length given when doing "newArray"?
> This algorithm in general seems very paranoid (about possible corrupted data), but in this case depth could go negative if given a bad length but it appears ok with that. Should it check?

length <= stackDepth() (otherwise we would have returned). Hence:
  length >= 0 (length is uint32_t), stackDepth() >= 0 (invariant)
  const int depth = stackDepth() - length;  // depth >= 0; depth + length = stackDepth()
  for (unsigned int i = 0; i < length; ++i) // i = [0..length - 1]
      array->Set(i, element(depth + i));    // depth + i = [depth .. depth + length - 1]
                                            // depth + i = [depth .. stackDepth() - 1]

so we'll never try to grab a bad element().

length may <> the length given to newArray if the object is corrupted or if the array was mutated during serialization (and we didn't decide how many elements
we were going to write before we started). Our behavior then depends on v8::Object::Set(uint32_t, v8::Handle<Value>), and we're worried about the index >= length
case (because otherwise we get the default initialized value). If we assume that the array is still a dense array internally, we'll eventually get down to 
JSObject::SetElementWithoutInterceptor and JSObject::SetFastElement (v8/src/objects.cc:8263), the effect of which appears to be to rearrange the array's storage to
fit the new element (and not to throw an exception or do something even less pleasant). (Thanks to vegorov@ for confirming that this API sticks to JS semantics.)

For this patch the above is kind of moot, though, because it does not address the FIXME in newArrayState() from the old code-- so arrays are serialized as sparse arrays and that code doesn't get hit. Another future patch in the making!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.