Count Promise constructor callings.

Count Promise constructor callings.

Promise API is changing (See [1], [2] for details).
Count how Promise constructor functions are called.

[1] https://mail.mozilla.org/pipermail/es-discuss/2014-January/035976.html
[2] https://mail.mozilla.org/pipermail/es-discuss/2014-February/036193.html
BUG=343815
R=tkent, haraken

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

[tkent, 2014-02-14 07:20:53]

We don't know whether the API will be changed or not, right?

The message is not actionable for web authors, and just confuses them. I don't
think we should show such message.

[haraken, 2014-02-14 07:21:38]

Agreed with tkent-san about the message.

https://codereview.chromium.org/166173002/diff/1/Source/bindings/v8/custom/V8...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/166173002/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8PromiseCustom.cpp:491: ASSERT(executionContext);

Remove this ASSERT. This should be always non NULL. (I'll try to remove a path
that returns 0 from toExecutionContext(). As mentioned in a FIXME, it cannot
happen.)

[yhirano, 2014-02-14 07:37:11]

On 2014/02/14 07:20:53, tkent wrote:
> We don't know whether the API will be changed or not, right?
> 
> The message is not actionable for web authors, and just confuses them. I don't
> think we should show such message.

As in [2], Promise.resolve and Promise.cast has changed.
I am not sure if further changes will be made.
We are discussing whether we should unship Promise or not.

[yhirano, 2014-02-14 07:37:54]

https://codereview.chromium.org/166173002/diff/1/Source/bindings/v8/custom/V8...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/166173002/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8PromiseCustom.cpp:491: ASSERT(executionContext);
On 2014/02/14 07:21:39, haraken wrote:
> 
> Remove this ASSERT. This should be always non NULL. (I'll try to remove a path
> that returns 0 from toExecutionContext(). As mentioned in a FIXME, it cannot
> happen.)

Done.

[tkent, 2014-02-14 07:39:59]

On 2014/02/14 07:37:11, yhirano wrote:
> On 2014/02/14 07:20:53, tkent wrote:
> > We don't know whether the API will be changed or not, right?
> > 
> > The message is not actionable for web authors, and just confuses them. I
don't
> > think we should show such message.
> 
> As in [2], Promise.resolve and Promise.cast has changed.
> I am not sure if further changes will be made.
> We are discussing whether we should unship Promise or not.

I see. Anyway, I object to show messages though we don't have alternatives.

[yhirano, 2014-02-14 07:43:04]

On 2014/02/14 07:39:59, tkent wrote:
> On 2014/02/14 07:37:11, yhirano wrote:
> > On 2014/02/14 07:20:53, tkent wrote:
> > > We don't know whether the API will be changed or not, right?
> > > 
> > > The message is not actionable for web authors, and just confuses them. I
> don't
> > > think we should show such message.
> > 
> > As in [2], Promise.resolve and Promise.cast has changed.
> > I am not sure if further changes will be made.
> > We are discussing whether we should unship Promise or not.
> 
> I see. Anyway, I object to show messages though we don't have alternatives.

Is showing messages only in Promise.cast and Promise.resolve acceptable?

[abarth-chromium, 2014-02-14 08:16:42]

Should we discuss this topic on blink-dev?  Now that we've decided to ship
promises, we should be careful when making changes the API.  In particular, I
wonder if we should push back on TC39 not to change the API.  Maybe some
discussion on blink-dev will help guide us here.

[abarth-chromium, 2014-02-14 08:19:08]

not lgtm pending discussion on blink-dev

[yhirano, 2014-02-14 08:32:32]

OK, I removed warning messages.

[yhirano, 2014-02-14 08:33:11]

On 2014/02/14 08:16:42, abarth wrote:
> Should we discuss this topic on blink-dev?  Now that we've decided to ship
> promises, we should be careful when making changes the API.  In particular, I
> wonder if we should push back on TC39 not to change the API.  Maybe some
> discussion on blink-dev will help guide us here.

I will start a thread on blink-dev about this problem.

[abarth-chromium, 2014-02-14 08:39:17]

Thanks!

[yhirano, 2014-02-14 14:56:53]

PTAL?
Sorry to rush you, but I'd like to land this CL before the coming branch point.

[haraken, 2014-02-14 15:09:13]

LGTM

[haraken, 2014-02-14 15:09:19]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-14 15:09:42]

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

[commit-bot: I haz the power, 2014-02-14 16:19:41]

Change committed as 167206

[abarth-chromium, 2014-02-14 17:50:20]

A revert of this CL has been created in
https://codereview.chromium.org/167253002/ by abarth@chromium.org.

The reason for reverting is: This CL was landed with a "not lgtm"..

[abarth-chromium, 2014-02-14 17:53:20]

On 2014/02/14 17:50:20, abarth wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/167253002/ by mailto:abarth@chromium.org.
> 
> The reason for reverting is: This CL was landed with a "not lgtm"..

Oops!!!  Sorry.  I see now that you changed the CL.  I cancelled the revert.

[eseidel, 2014-02-15 16:56:56]

Could we merge this to Beta to get better number sooner?

[yhirano, 2014-02-15 17:39:54]

I see, I've requested merging.