Implement PromiseResolver.prototype.resolve.

Implement PromiseResolver.prototype.resolve.

We are implementing DOM/Promises. 
This CL implements PromiseResolver.prototype.resolve. 
Again, all methods are marked as Custom and there are many boilerplate. we will fix them in a future CL. 
There are no tests. We will add tests in a future. 

R=abarth, haraken
BUG=243345
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153215

[yhirano, 2013-06-28 01:50:11]

This CL is based on https://codereview.chromium.org/17993004/ which is landing.

[haraken, 2013-06-28 03:33:23]

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:68: v8::HandleScope
handleScope(isolate);

Do you need this? The fact that this method returns a handle implies that caller
sites already have handle scopes. Basically we want to make sure that caller
sites have handle scopes.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:124: ASSERT(args.Length() > 0);

How is it guaranteed that promiseFullfill() is called with one or more
arguments?

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:174: // FIXME: script compilation
should be cached.

I think the compiled script is already cached within one context in the V8 side.
I don't know how much it is faster than caching in the Blink side.

If you want to cache it in the Blink side, you can bind the compiled result with
the object using SetHiddenValue(). See how copyElements() in
V8ArrayBufferViewCustom.cpp is implemented.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:176: v8::Local<v8::String> script
= v8::String::New("(function(f, v1) { return function(v2) { return f(v1, v2); };
})");

v8::String::New() => v8String()

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:185: value =
V8ScriptRunner::callFunction(value.As<v8::Function>(),
getScriptExecutionContext(), receiver, WTF_ARRAY_LENGTH(argv), argv);

Don't you need to check value.IsEmpty() or catch exceptions?

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:258: if (result->IsObject()) {

You might want to add ASSERT(!result.IsEmpty()).

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:260: then =
result.As<v8::Object>()->Get(v8::String::New("then"));

v8::String::New() => v8::String::NewSymbol()

[yhirano, 2013-06-28 04:14:51]

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:68: v8::HandleScope
handleScope(isolate);
On 2013/06/28 03:33:23, haraken wrote:
> 
> Do you need this? The fact that this method returns a handle implies that
caller
> sites already have handle scopes. Basically we want to make sure that caller
> sites have handle scopes.

Done.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:124: ASSERT(args.Length() > 0);
On 2013/06/28 03:33:23, haraken wrote:
> 
> How is it guaranteed that promiseFullfill() is called with one or more
> arguments?
promiseFulfillCallback is used in promiseCallback, which binds the resolver as
the first argument of promiseFulfillCallback and returns it.

In pseudo code, 

promiseCallback(resolver) [algorithm = Fulfill] = 
(function(f, v1) { return function(v2) { return f(v1, v2); };
})(promiseFulfillCallback, resolver);

Hence promiseFulfillCallback will always be called with the resolver.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:174: // FIXME: script compilation
should be cached.
On 2013/06/28 03:33:23, haraken wrote:
> 
> I think the compiled script is already cached within one context in the V8
side.
> I don't know how much it is faster than caching in the Blink side.
> 
> If you want to cache it in the Blink side, you can bind the compiled result
with
> the object using SetHiddenValue(). See how copyElements() in
> V8ArrayBufferViewCustom.cpp is implemented.

Done.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:176: v8::Local<v8::String> script
= v8::String::New("(function(f, v1) { return function(v2) { return f(v1, v2); };
})");
On 2013/06/28 03:33:23, haraken wrote:
> 
> v8::String::New() => v8String()

Done.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:185: value =
V8ScriptRunner::callFunction(value.As<v8::Function>(),
getScriptExecutionContext(), receiver, WTF_ARRAY_LENGTH(argv), argv);
On 2013/06/28 03:33:23, haraken wrote:
> 
> Don't you need to check value.IsEmpty() or catch exceptions?

Done.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:258: if (result->IsObject()) {
On 2013/06/28 03:33:23, haraken wrote:
> 
> You might want to add ASSERT(!result.IsEmpty()).

Done, but I don't understand when we have to check non-emptiness and when we
don't.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:260: then =
result.As<v8::Object>()->Get(v8::String::New("then"));
On 2013/06/28 03:33:23, haraken wrote:
> 
> v8::String::New() => v8::String::NewSymbol()

Done.

[haraken, 2013-06-28 04:17:34]

LGTM from the aspect of binding implementation. I'm not familiar with Promise,
so please wait for an approval from abarth.

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/18119008/diff/3001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:258: if (result->IsObject()) {

Basically we need an x.IsEmpty() check before you access x->XXX(). Otherwise,
x->XXX() will crash.

[abarth-chromium, 2013-06-28 04:35:52]

lgtm

https://codereview.chromium.org/18119008/diff/7001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/18119008/diff/7001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:58: // tag must be a pointer of
one of the above tags.
Should we ASSERT this fact rather than writing a comment?

https://codereview.chromium.org/18119008/diff/7001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:123: ASSERT(args.Length() > 0);
Why is this necessarily true?  I guess that's already checked by the time we get
here?

[yhirano, 2013-06-28 04:51:53]

https://codereview.chromium.org/18119008/diff/7001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/18119008/diff/7001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:58: // tag must be a pointer of
one of the above tags.
On 2013/06/28 04:35:53, abarth wrote:
> Should we ASSERT this fact rather than writing a comment?

Done.

https://codereview.chromium.org/18119008/diff/7001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8PromiseCustom.cpp:123: ASSERT(args.Length() > 0);
On 2013/06/28 04:35:53, abarth wrote:
> Why is this necessarily true?  I guess that's already checked by the time we
get
> here?
promiseFulfillCallback is used in promiseCallback, which binds the resolver as
the first argument of promiseFulfillCallback and returns it.

In pseudo code, 

promiseCallback(resolver) [algorithm = Fulfill] = 
(function(f, v1) { return function(v2) { return f(v1, v2); };
})(promiseFulfillCallback, resolver);

Hence promiseFulfillCallback will always be called with the resolver.

I added comments.

[commit-bot: I haz the power, 2013-06-28 05:51:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/18119008/15001

[commit-bot: I haz the power, 2013-06-28 07:21:38]

Change committed as 153215