Make the cube spin in the JS mojo app.

Make the cube spin in the JS mojo app.

R=abarth@chromium.org

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

[abarth-chromium, 2014-01-07 23:01:03]

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/main.js
File mojo/apps/js/main.js (right):

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/main.js#new...
mojo/apps/js/main.js:358: var timeDelta = Date.now() - this.lastTime_;
We should be use Mojo's monotonic clock rather than Data.now.  We probably need
to add a JS binding for MojoGetTimeTicksNow.

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/mojo_runner...
File mojo/apps/js/mojo_runner_delegate.cc (right):

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/mojo_runner...
mojo/apps/js/mojo_runner_delegate.cc:50:
AddBuiltinModule(gin::TimerModule::kName, gin::TimerModule::GetModule);
Should we change the other kModuleNames to kName for consistency?

https://codereview.chromium.org/126743002/diff/20001/mojo/examples/sample_app...
File mojo/examples/sample_app/gles2_client_impl.h (right):

https://codereview.chromium.org/126743002/diff/20001/mojo/examples/sample_app...
mojo/examples/sample_app/gles2_client_impl.h:36:
base::OneShotTimer<GLES2ClientImpl> timer_;
Is this change intentional?

[abarth-chromium, 2014-01-07 23:01:08]

LGTM

[Aaron Boodman, 2014-01-07 23:22:38]

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/main.js
File mojo/apps/js/main.js (right):

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/main.js#new...
mojo/apps/js/main.js:358: var timeDelta = Date.now() - this.lastTime_;
On 2014/01/07 23:01:04, abarth wrote:
> We should be use Mojo's monotonic clock rather than Data.now.  We probably
need
> to add a JS binding for MojoGetTimeTicksNow.

I'm always happy to add more bindings using our shiny new bindings system, but
in this case, I don't see what it adds. The frame rate is 60 hertz. How does the
extra resolution help us?

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/mojo_runner...
File mojo/apps/js/mojo_runner_delegate.cc (right):

https://codereview.chromium.org/126743002/diff/20001/mojo/apps/js/mojo_runner...
mojo/apps/js/mojo_runner_delegate.cc:50:
AddBuiltinModule(gin::TimerModule::kName, gin::TimerModule::GetModule);
On 2014/01/07 23:01:04, abarth wrote:
> Should we change the other kModuleNames to kName for consistency?

The reason Timer is different is because there are two objects involved - the
module object and the object for each timer instance. I would like the other
bindings to also follow this pattern, but it's more work than just changing the
constant name.

https://codereview.chromium.org/126743002/diff/20001/mojo/examples/sample_app...
File mojo/examples/sample_app/gles2_client_impl.h (right):

https://codereview.chromium.org/126743002/diff/20001/mojo/examples/sample_app...
mojo/examples/sample_app/gles2_client_impl.h:36:
base::OneShotTimer<GLES2ClientImpl> timer_;
On 2014/01/07 23:01:04, abarth wrote:
> Is this change intentional?

Heh, no. Thanks.

[commit-bot: I haz the power, 2014-01-07 23:30:11]

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

[abarth-chromium, 2014-01-08 01:17:28]

On 2014/01/07 23:22:38, Aaron Boodman wrote:
> I'm always happy to add more bindings using our shiny new bindings system, but
> in this case, I don't see what it adds. The frame rate is 60 hertz. How does
the
> extra resolution help us?

We chatted on IRC about how it affects the case where the user changes their
system clock during the animation.  In any case, it's not a big deal.

[commit-bot: I haz the power, 2014-01-08 01:37:39]

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

[Aaron Boodman, 2014-01-08 20:31:40]

Committed patchset #3 manually as r243637 (presubmit successful).