You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2011/08/16 18:50:18 UTC
Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
Diff: https://reviews.apache.org/r/1525/diff
Testing
-------
Thanks,
Dan
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1527
-----------------------------------------------------------
Ship it!
Thanks for the explanation Dan. LGTM. +1
- Henry
On 2011-08-17 15:18:59, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-17 15:18:59)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> This addresses bug SHINDIG-1578.
> https://issues.apache.org/jira/browse/SHINDIG-1578
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1518
-----------------------------------------------------------
I like this change, makes sense with your explanation. What do you think Henry?
- Ryan
On 2011-08-17 15:18:59, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-17 15:18:59)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> This addresses bug SHINDIG-1578.
> https://issues.apache.org/jira/browse/SHINDIG-1578
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/
-----------------------------------------------------------
(Updated 2011-08-17 15:18:59.396755)
Review request for shindig.
Changes
-------
Added jira
Summary
-------
If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
This addresses bug SHINDIG-1578.
https://issues.apache.org/jira/browse/SHINDIG-1578
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
Diff: https://reviews.apache.org/r/1525/diff
Testing
-------
Thanks,
Dan
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Dan Dumont <dd...@us.ibm.com>.
> On 2011-08-16 17:24:01, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js, line 47
> > <https://reviews.apache.org/r/1525/diff/1/?file=32902#file32902line47>
> >
> > Why does the definition registerOnLoadHandler here?
It is redefined to immediately execute any onload callbacks.
The sequence of events is:
1) Gadget starts loading and registers some onload.
2) Gadget fires an async operation that takes an unknown amount of time
3) Open Social api is ready and runOnLoadHandlers is called.
4) Gadget's async operation is done. To be safe it uses registerOnLoadHandler to make sure everything is all set (in case the async operation beat the runOnLoadHandlers call)
At this point, before the change, the call to registerOnLoadHandler would never result in the callback being executed.
This change makes it so once runOnLoadHandlers is called, registerOnLoadHandler executes the callback immediately.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1479
-----------------------------------------------------------
On 2011-08-16 16:50:18, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-16 16:50:18)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Henry Saputra <hs...@apache.org>.
> On 2011-08-16 17:24:01, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js, line 47
> > <https://reviews.apache.org/r/1525/diff/1/?file=32902#file32902line47>
> >
> > Why does the definition registerOnLoadHandler here?
>
> Dan Dumont wrote:
> It is redefined to immediately execute any onload callbacks.
>
> The sequence of events is:
> 1) Gadget starts loading and registers some onload.
> 2) Gadget fires an async operation that takes an unknown amount of time
> 3) Open Social api is ready and runOnLoadHandlers is called.
> 4) Gadget's async operation is done. To be safe it uses registerOnLoadHandler to make sure everything is all set (in case the async operation beat the runOnLoadHandlers call)
>
> At this point, before the change, the call to registerOnLoadHandler would never result in the callback being executed.
> This change makes it so once runOnLoadHandlers is called, registerOnLoadHandler executes the callback immediately.
In step 2, the async operation is called outside of the function registered with registerOnLoadHandler?
So in step 4, why would the gadget's async handler call registerOnLoadHandler?
- Henry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1479
-----------------------------------------------------------
On 2011-08-17 15:18:59, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-17 15:18:59)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> This addresses bug SHINDIG-1578.
> https://issues.apache.org/jira/browse/SHINDIG-1578
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Jesse Ciancetta <jc...@mitre.org>.
> On 2011-08-16 17:24:01, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js, line 47
> > <https://reviews.apache.org/r/1525/diff/1/?file=32902#file32902line47>
> >
> > Why does the definition registerOnLoadHandler here?
>
> Dan Dumont wrote:
> It is redefined to immediately execute any onload callbacks.
>
> The sequence of events is:
> 1) Gadget starts loading and registers some onload.
> 2) Gadget fires an async operation that takes an unknown amount of time
> 3) Open Social api is ready and runOnLoadHandlers is called.
> 4) Gadget's async operation is done. To be safe it uses registerOnLoadHandler to make sure everything is all set (in case the async operation beat the runOnLoadHandlers call)
>
> At this point, before the change, the call to registerOnLoadHandler would never result in the callback being executed.
> This change makes it so once runOnLoadHandlers is called, registerOnLoadHandler executes the callback immediately.
>
> Henry Saputra wrote:
> In step 2, the async operation is called outside of the function registered with registerOnLoadHandler?
>
> So in step 4, why would the gadget's async handler call registerOnLoadHandler?
>
> Dan Dumont wrote:
> This is important with some of the changes in dojo 1.7 where the bootstrap may be very small and included in the gadget page but asynchronously load more code in. When the application is ready, it should be able to rely on registerOnLoadHandler to provide them a means of executing gadget code safely.
>
> Really, the gadget should be able to do any non-gadget-api async operations while the page is loading. To sync everything back up, the gadget should be able to rely on registerOnLoadHandler executing their callbacks that rely on gadget-api operations so that we don't introduce race conditions.
+1 for this change. I think this is pretty much how all of the "ready" functions I've seen in other JS libraries work (dojo, jquery, ...) -- in any of those libraries registering code after the trigger has already fired results in immediate execution, so I think this is probably something people would expect.
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1479
-----------------------------------------------------------
On 2011-08-17 15:18:59, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-17 15:18:59)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> This addresses bug SHINDIG-1578.
> https://issues.apache.org/jira/browse/SHINDIG-1578
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Henry Saputra <hs...@apache.org>.
> On 2011-08-16 17:24:01, Henry Saputra wrote:
> >
Please attach the final patch to the jira entry.
- Henry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1479
-----------------------------------------------------------
On 2011-08-17 15:18:59, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-17 15:18:59)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> This addresses bug SHINDIG-1578.
> https://issues.apache.org/jira/browse/SHINDIG-1578
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Dan Dumont <dd...@us.ibm.com>.
> On 2011-08-16 17:24:01, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js, line 47
> > <https://reviews.apache.org/r/1525/diff/1/?file=32902#file32902line47>
> >
> > Why does the definition registerOnLoadHandler here?
>
> Dan Dumont wrote:
> It is redefined to immediately execute any onload callbacks.
>
> The sequence of events is:
> 1) Gadget starts loading and registers some onload.
> 2) Gadget fires an async operation that takes an unknown amount of time
> 3) Open Social api is ready and runOnLoadHandlers is called.
> 4) Gadget's async operation is done. To be safe it uses registerOnLoadHandler to make sure everything is all set (in case the async operation beat the runOnLoadHandlers call)
>
> At this point, before the change, the call to registerOnLoadHandler would never result in the callback being executed.
> This change makes it so once runOnLoadHandlers is called, registerOnLoadHandler executes the callback immediately.
>
> Henry Saputra wrote:
> In step 2, the async operation is called outside of the function registered with registerOnLoadHandler?
>
> So in step 4, why would the gadget's async handler call registerOnLoadHandler?
This is important with some of the changes in dojo 1.7 where the bootstrap may be very small and included in the gadget page but asynchronously load more code in. When the application is ready, it should be able to rely on registerOnLoadHandler to provide them a means of executing gadget code safely.
Really, the gadget should be able to do any non-gadget-api async operations while the page is loading. To sync everything back up, the gadget should be able to rely on registerOnLoadHandler executing their callbacks that rely on gadget-api operations so that we don't introduce race conditions.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1479
-----------------------------------------------------------
On 2011-08-17 15:18:59, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-17 15:18:59)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> This addresses bug SHINDIG-1578.
> https://issues.apache.org/jira/browse/SHINDIG-1578
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Allow slow-to-init gadgets to safely rely on
gadgets.util.registerOnLoadHandler
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1525/#review1479
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js
<https://reviews.apache.org/r/1525/#comment3389>
Why does the definition registerOnLoadHandler here?
- Henry
On 2011-08-16 16:50:18, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1525/
> -----------------------------------------------------------
>
> (Updated 2011-08-16 16:50:18)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If gadgets.util.registerOnLoadHandler was called after the runOnLoad counterpart was called, the callback would sit in the array... forever alone.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.onload/onload.js 1157211
>
> Diff: https://reviews.apache.org/r/1525/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan
>
>