For HashTable-backed JS objects, use the proper type in the table() accessor

For HashTable-backed JS objects, use the proper type in the table() accessor

Currently nearly all code using JSSet/JSMap/JSWeakMap does a cast
whenever calling table(). This patch simply puts that cast inside
the accessor. The only possible way for table() to not be valid
is during heap verification (which has been fixed up to not call
the accessor) or if GC occurs during construction of the hash table
itself (where a call to HeapObject::RawField has been moved up to
do the read instead of going through the accessor).

This is a precursor to making a similar change to JSArray::length(),
but is hopefully less dramatic, since there are many fewer uses of
sets and maps than arrays.

[Michael Starzinger, 2013-02-06 11:43:12]

This essentially introduces a lie into our type-system, because there are
observable points in time where the kTableOffset field does not contain an
ObjectHashTable. The fact that the heap verifier observes this heap-state proves
that these points exist. Even though these time-windows are small they exist and
just because there are more use-sites that see an ObjectHashTable than use-sites
that see "undefined" in there, IMHO doesn't justify hiding this fact.

[Michael Starzinger, 2013-02-06 11:46:34]

I could be convinced to change the return-type of the table() accessor to
HeapObject, because "undefined" as an Oddball is a HeapObject. But I doubt that
this would simplify use-sites.

[adamk, 2013-02-06 16:53:15]

On 2013/02/06 11:43:12, Michael Starzinger wrote:
> This essentially introduces a lie into our type-system, because there are
> observable points in time where the kTableOffset field does not contain an
> ObjectHashTable. The fact that the heap verifier observes this heap-state
proves
> that these points exist. Even though these time-windows are small they exist
and
> just because there are more use-sites that see an ObjectHashTable than
use-sites
> that see "undefined" in there, IMHO doesn't justify hiding this fact.

Fair enough, I put this together on a whim because it seemed weird that all
these callers would know something about the objects that they didn't know about
themselves. The short window of non-ObjectHashTable-ness seems like simply an
accident (see, e.g., the TODO in Heap::InitializeJSObjectFromMap) that should be
_internal_ to the JSWeakMap/JSMap/JSSet classes. But I'm sympathetic that it's
not worth "lying" in this case (and yeah, I don't think it's worth changing to
HeapObject).

Thanks for providing your perspective on this, I'm slowly learning the v8
way-of-doing-things by asking questions like this.