Implement Table#length and Table#get

Implement Table#length and Table#get

R=ahaas@chromium.org
BUG=

Committed: https://crrev.com/d95b754319ea84d339ff748680125f39574c1ab1
Cr-Commit-Position: refs/heads/master@{#40273}

[ahaas, 2016-10-13 09:12:59]

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode491
src/wasm/wasm-js.cc:491: if (!BrandCheck(isolate,
Utils::OpenHandle(*args.This()),
you call Utils::OpenHandle(*args.This()) twice here. Would it be worth storing
the result in a variable?

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode499
src/wasm/wasm-js.cc:499: i::Handle<i::Object>
array(receiver->GetInternalField(0), i_isolate);
Can you use a constant here instead of '0'?

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode529
src/wasm/wasm-js.cc:529: if (args.Length() < 1) {
Does the spec actually say that a TypeError should be thrown if args.Length() <
1?

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode539
src/wasm/wasm-js.cc:539: i::Handle<i::Object>
array(receiver->GetInternalField(0), i_isolate);
same here

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode559
src/wasm/wasm-js.cc:559: if (args.Length() < 1) {
This if is not needed, the next if also takes care of it.

[rossberg, 2016-10-13 12:02:07]

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode491
src/wasm/wasm-js.cc:491: if (!BrandCheck(isolate,
Utils::OpenHandle(*args.This()),
On 2016/10/13 09:12:58, ahaas wrote:
> you call Utils::OpenHandle(*args.This()) twice here. Would it be worth storing
> the result in a variable?

It's just an accessor, so should be fast enough.

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode499
src/wasm/wasm-js.cc:499: i::Handle<i::Object>
array(receiver->GetInternalField(0), i_isolate);
On 2016/10/13 09:12:59, ahaas wrote:
> Can you use a constant here instead of '0'?

Done (also for some other field indices).

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode529
src/wasm/wasm-js.cc:529: if (args.Length() < 1) {
On 2016/10/13 09:12:58, ahaas wrote:
> Does the spec actually say that a TypeError should be thrown if args.Length()
<
> 1?

Changed.

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode539
src/wasm/wasm-js.cc:539: i::Handle<i::Object>
array(receiver->GetInternalField(0), i_isolate);
On 2016/10/13 09:12:58, ahaas wrote:
> same here

Done.

https://codereview.chromium.org/2411963003/diff/1/src/wasm/wasm-js.cc#newcode559
src/wasm/wasm-js.cc:559: if (args.Length() < 1) {
On 2016/10/13 09:12:59, ahaas wrote:
> This if is not needed, the next if also takes care of it.

Obsolete.

[ahaas, 2016-10-13 12:27:02]

lgtm

[rossberg, 2016-10-13 13:02:50]

The CQ bit was checked by rossberg@chromium.org

[commit-bot: I haz the power, 2016-10-13 13:02:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 13:38:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 13:38:20]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/26394)

[rossberg, 2016-10-13 13:52:40]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-10-13 13:52:41]

The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org
Link to the patchset: https://codereview.chromium.org/2411963003/#ps40001
(title: "Git format nonsense")

[commit-bot: I haz the power, 2016-10-13 13:52:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 14:43:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-13 14:43:57]

Description was changed from

==========
Implement Table#length and Table#get

R=ahaas@chromium.org
BUG=
==========

to

==========
Implement Table#length and Table#get

R=ahaas@chromium.org
BUG=

Committed: https://crrev.com/d95b754319ea84d339ff748680125f39574c1ab1
Cr-Commit-Position: refs/heads/master@{#40273}
==========

[commit-bot: I haz the power, 2016-10-13 14:43:58]

Patchset 3 (id:??) landed as
https://crrev.com/d95b754319ea84d339ff748680125f39574c1ab1
Cr-Commit-Position: refs/heads/master@{#40273}