Decouple ScriptPromise creation from ScriptPromiseResolver.

Decouple ScriptPromise creation from ScriptPromiseResolver.

It is error-prone that ScriptPromiseResolver has ScriptPromise and
users must explicitly call detachPromise() to detach it.
Now ScriptPromiseResolver does NOT have ScriptPromise from users perspective.

Also replace deprecated |fulfill| with |resolve|.

BUG=304154
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159698

[yhirano, 2013-10-04 12:52:37]

PTAL

abarth: bindings/ (and overall)
alecflett: modules/serviceworker
ksakamoto: core/css
junov: modules/imagebitmap
eroman: modules/crypto

[Justin Novosad, 2013-10-04 14:09:51]

On 2013/10/04 12:52:37, yhirano wrote:
> PTAL
> 
> abarth: bindings/ (and overall)
> alecflett: modules/serviceworker
> ksakamoto: core/css
> junov: modules/imagebitmap
> eroman: modules/crypto

lgtm for modules/imagebitmap

[abarth-chromium, 2013-10-04 15:46:35]

abarth: bindings <--- LGTM (the rest looks fine to me too, but I didn't review
it super carefully)

[alecflett, 2013-10-04 19:29:40]

lgtm for serviceworker

[Kunihiko Sakamoto, 2013-10-06 01:53:09]

lgtm for FontFaceSet

[yhirano, 2013-10-08 23:57:18]

eroman, can you take a look at this CL?

[yhirano, 2013-10-11 02:04:25]

Adam, Eric seems busy.
You reviewed many CLs for Source/modules/crypto/CryptoResultImpl.{cpp, h}, can I
ask you to review the changes on them as well?

[abarth-chromium, 2013-10-11 18:18:01]

https://codereview.chromium.org/26004002/diff/18001/Source/bindings/v8/Script...
File Source/bindings/v8/ScriptPromise.h (right):

https://codereview.chromium.org/26004002/diff/18001/Source/bindings/v8/Script...
Source/bindings/v8/ScriptPromise.h:99: static ScriptPromise
create(ScriptExecutionContext*);
Rather than write comments like these, we prefer if you put those ideas into the
names of the functions.  These function should be called:

ScriptPromise::createPending

Then all the callers will know that they're creating pending promises.

https://codereview.chromium.org/26004002/diff/18001/Source/modules/crypto/Cry...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/26004002/diff/18001/Source/modules/crypto/Cry...
Source/modules/crypto/CryptoResultImpl.h:63: ScriptPromise m_promise;
Does this create a memory leak?  If CryptoResult is holding m_promise alive, is
it possible that m_promise is holding CryptoResult alive?  Can we remove this
member?

[yhirano, 2013-10-14 02:49:52]

https://codereview.chromium.org/26004002/diff/18001/Source/bindings/v8/Script...
File Source/bindings/v8/ScriptPromise.h (right):

https://codereview.chromium.org/26004002/diff/18001/Source/bindings/v8/Script...
Source/bindings/v8/ScriptPromise.h:99: static ScriptPromise
create(ScriptExecutionContext*);
On 2013/10/11 18:18:02, abarth wrote:
> Rather than write comments like these, we prefer if you put those ideas into
the
> names of the functions.  These function should be called:
> 
> ScriptPromise::createPending
> 
> Then all the callers will know that they're creating pending promises.

Done.

https://codereview.chromium.org/26004002/diff/18001/Source/modules/crypto/Cry...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/26004002/diff/18001/Source/modules/crypto/Cry...
Source/modules/crypto/CryptoResultImpl.h:63: ScriptPromise m_promise;
On 2013/10/11 18:18:02, abarth wrote:
> Does this create a memory leak?  If CryptoResult is holding m_promise alive,
is
> it possible that m_promise is holding CryptoResult alive?  Can we remove this
> member?

Done.

[abarth-chromium, 2013-10-15 17:40:51]

lgtm

https://codereview.chromium.org/26004002/diff/51001/Source/bindings/v8/Script...
File Source/bindings/v8/ScriptPromise.h (right):

https://codereview.chromium.org/26004002/diff/51001/Source/bindings/v8/Script...
Source/bindings/v8/ScriptPromise.h:98: // Creates a pending promise.
These comments aren't needed anymore.  We can see from the name what it does.

[commit-bot: I haz the power, 2013-10-15 17:41:40]

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

[commit-bot: I haz the power, 2013-10-15 23:30:17]

Change committed as 159698