Add a simple one shot and repeating timer API for Mojo.js.

Add a simple one shot and repeating timer API for Mojo.js.

Various further rework of bindings stuff to facilitate.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242284

[Aaron Boodman, 2013-12-21 01:44:29]

https://codereview.chromium.org/120043008/diff/1/gin/runner.cc
File gin/runner.cc (right):

https://codereview.chromium.org/120043008/diff/1/gin/runner.cc#newcode40
gin/runner.cc:40: CHECK(false) << try_catch.GetStackTrace();
This seems like a better default behavior than what there was before.

[abarth-chromium, 2013-12-21 03:37:06]

LGTM

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc
File gin/modules/timer.cc (right):

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:16: return gin::StringToSymbol(isolate,
"::mojo::js::Timer");
::gin::Timer probably.  We don't want to mention "mojo" in this directory

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:17: }
I talked with danno a bit about this pattern.  Apparently a better way to do
this is with v8::SymbolObject.  That way you can use the normal Get/Set
functions instead of the hidden values.  We'll probably want to switch over
eventually, but we don't need to do that now.

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:56: Runner::Scope scope(runner_.get());
Should we check the runner_ for null?

https://codereview.chromium.org/120043008/diff/20001/gin/runner.cc
File gin/runner.cc (right):

https://codereview.chromium.org/120043008/diff/20001/gin/runner.cc#newcode68
gin/runner.cc:68: }
This seems redundant.  Won't the overload below catch any exception?

[commit-bot: I haz the power, 2013-12-21 05:39:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/120043008/90001

[commit-bot: I haz the power, 2013-12-21 06:26:36]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-12-21 06:31:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/120043008/90001

[commit-bot: I haz the power, 2013-12-21 06:34:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/120043008/190002

[commit-bot: I haz the power, 2013-12-21 17:54:47]

Change committed as 242284

[Aaron Boodman, 2013-12-21 17:59:11]

I didn't hit the cq button. did you?

[abarth-chromium, 2013-12-21 18:08:54]

On 2013/12/21 17:59:11, Aaron Boodman wrote:
> I didn't hit the cq button. did you?

Yes, sorry.  I figured you were out for vacation so I did it.

[Aaron Boodman, 2013-12-21 21:34:41]

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc
File gin/modules/timer.cc (right):

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:17: }
On 2013/12/21 03:37:06, abarth wrote:
> I talked with danno a bit about this pattern.  Apparently a better way to do
> this is with v8::SymbolObject.  That way you can use the normal Get/Set
> functions instead of the hidden values.  We'll probably want to switch over
> eventually, but we don't need to do that now.

Wow, cool. Note to self:
http://webreflection.blogspot.com/2013/03/simulating-es6-symbols-in-es5.html

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:56: Runner::Scope scope(runner_.get());
On 2013/12/21 03:37:06, abarth wrote:
> Should we check the runner_ for null?

I believe that cannot happen if everyone is playing by the rules:

If runner id deleted, then the isolate we are part of is deleted, which means
our wrapped object is deleted, which means this C++ object is deleted, which
means the weak callback we passed to the timer will not get called.

If someone keeps this object alive some other way, then we will get called, but
that person would be wrong.

Perhaps I should add a comment with this reasoning so that when this crashes we
can find the code that is incorrectly keeping us alive.

https://codereview.chromium.org/120043008/diff/20001/gin/runner.cc
File gin/runner.cc (right):

https://codereview.chromium.org/120043008/diff/20001/gin/runner.cc#newcode68
gin/runner.cc:68: }
On 2013/12/21 03:37:06, abarth wrote:
> This seems redundant.  Won't the overload below catch any exception?

No, New() is responsible for the initial parse. If there are syntax errors they
show up here.

[Aaron Boodman, 2013-12-21 22:02:17]

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc
File gin/modules/timer.cc (right):

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:17: }
On 2013/12/21 21:34:41, Aaron Boodman wrote:
> On 2013/12/21 03:37:06, abarth wrote:
> > I talked with danno a bit about this pattern.  Apparently a better way to do
> > this is with v8::SymbolObject.  That way you can use the normal Get/Set
> > functions instead of the hidden values.  We'll probably want to switch over
> > eventually, but we don't need to do that now.
> 
> Wow, cool. Note to self:
> http://webreflection.blogspot.com/2013/03/simulating-es6-symbols-in-es5.html

Actually, i'm still not clear on how that would work. Somebody needs to own the
Symbol or SymbolObject.

[abarth-chromium, 2013-12-21 22:22:50]

On 2013/12/21 22:02:17, Aaron Boodman wrote:
> https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc
> File gin/modules/timer.cc (right):
> 
>
https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
> gin/modules/timer.cc:17: }
> On 2013/12/21 21:34:41, Aaron Boodman wrote:
> > On 2013/12/21 03:37:06, abarth wrote:
> > > I talked with danno a bit about this pattern.  Apparently a better way to
do
> > > this is with v8::SymbolObject.  That way you can use the normal Get/Set
> > > functions instead of the hidden values.  We'll probably want to switch
over
> > > eventually, but we don't need to do that now.
> > 
> > Wow, cool. Note to self:
> > http://webreflection.blogspot.com/2013/03/simulating-es6-symbols-in-es5.html
> 
> Actually, i'm still not clear on how that would work. Somebody needs to own
the
> Symbol or SymbolObject.

Right, we'll need to let code store a SymbolObject in per-isolate data, much the
same way we let them store ObjectTemplates.

[Aaron Boodman, 2013-12-21 22:32:58]

On Sat, Dec 21, 2013 at 2:22 PM, <abarth@chromium.org> wrote:

> On 2013/12/21 22:02:17, Aaron Boodman wrote:
>
>> https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc
>> File gin/modules/timer.cc (right):
>>
>
>
> https://codereview.chromium.org/120043008/diff/20001/gin/
> modules/timer.cc#newcode17
>
>> gin/modules/timer.cc:17: }
>> On 2013/12/21 21:34:41, Aaron Boodman wrote:
>> > On 2013/12/21 03:37:06, abarth wrote:
>> > > I talked with danno a bit about this pattern.  Apparently a better
>> way to
>>
> do
>
>> > > this is with v8::SymbolObject.  That way you can use the normal
>> Get/Set
>> > > functions instead of the hidden values.  We'll probably want to switch
>>
> over
>
>> > > eventually, but we don't need to do that now.
>> >
>> > Wow, cool. Note to self:
>> > http://webreflection.blogspot.com/2013/03/simulating-es6-
>> symbols-in-es5.html
>>
>
>  Actually, i'm still not clear on how that would work. Somebody needs to
>> own
>>
> the
>
>> Symbol or SymbolObject.
>>
>
> Right, we'll need to let code store a SymbolObject in per-isolate data,
> much the
> same way we let them store ObjectTemplates.
>

Hm. Does not seem like a huge win, but ok.

https://codereview.chromium.org/120043008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[abarth-chromium, 2013-12-21 22:36:30]

On 2013/12/21 22:32:58, Aaron Boodman wrote:
> Hm. Does not seem like a huge win, but ok.

The win is internally in V8 when they can get rid of
SetHiddenValue/GetHiddenValue.  The symbols just go into the normal property
slots.

[Aaron Boodman, 2013-12-21 23:39:11]

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc
File gin/modules/timer.cc (right):

https://codereview.chromium.org/120043008/diff/20001/gin/modules/timer.cc#new...
gin/modules/timer.cc:56: Runner::Scope scope(runner_.get());
I made a test for this to prove I was right. You were right.

https://codereview.chromium.org/100143006/