You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@shindig.apache.org by Felix Lee <fe...@gmail.com> on 2011/05/27 19:14:29 UTC

Review Request: Fix some osapi+caja onload timing issues

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/795/
-----------------------------------------------------------

Review request for shindig.

Summary
-------

1. The caja guest frame calls eval on the cajoled script, which is saved in a variable in the parent frame. I'm stupid, and I wrote the code in the wrong order. In some cases, it might be possible for the eval to happen before the cajoled script is saved. The result is that none of the gadget's js gets run, and no error is reported.

I don't think I've actually seen this happening, and it might actually be impossible due to javascript's lack-of-concurrency model, but there's no reason to leave this code in the wrong order, so this patch fixes the order.

2. osapi initialization requires doing a listMethods rpc, which is asynchronous. Since osapi isn't ready until that finishes, osapi replaces gadgets.util.runOnLoadHandlers with its own function that waits for listMethods to finish before firing off the onload handlers.

So, the current logic fires onload handlers as soon as two of these three events happen:
A. the gadget calls runOnLoadHandlers.
B. the listMethods rpc succeeds.
C. 500 ms after calling listMethods.

The problem with that is, in some cases, B and C happen before A. In particular, Firefox with Firebug enabled can add enough overhead that B and C happen before the gadget executes at all, which means the gadget doesn't get to register its onload handlers in time. The result is the gadget's onload handlers never get called, and no error is reported.

This patch fixes the logic so that the runOnLoadHandlers gets called only after the gadget calls runOnLoadHandlers.

Diffs
-----

/trunk/features/src/main/javascript/features/caja/taming.js 1128387
/trunk/features/src/main/javascript/features/osapi/gadgetsrpctransport.js 1128387

Diff: https://reviews.apache.org/r/795/diff

Testing
-------

'mvn test' succeeds.

SocialHelloWorld demo seems to work reliably in Firefox now, even with Firebug's delay.

Thanks,

Felix

Re: Review Request: Fix some osapi+caja onload timing issues

Posted by Chirag Shah <ch...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/795/#review845
-----------------------------------------------------------

Ship it!


- Chirag


On 2011-05-27 20:06:44, Felix Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/795/
> -----------------------------------------------------------
> 
> (Updated 2011-05-27 20:06:44)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> osapi initialization requires doing a listMethods rpc, which is asynchronous.  Since osapi isn't ready until that finishes, osapi replaces gadgets.util.runOnLoadHandlers with its own function that waits for listMethods to finish before firing off the onload handlers.
> 
> So, the current logic fires onload handlers as soon as two of these three events happen:
> A. the gadget calls runOnLoadHandlers.
> B. the listMethods rpc succeeds.
> C. 500 ms after calling listMethods.
> 
> The problem with that is, in some cases, B and C happen before A.  In particular, Firefox with Firebug enabled can add enough overhead that B and C happen before the gadget executes at all, which means the gadget doesn't get to register its onload handlers in time.  The result is the gadget's onload handlers never get called, and no error is reported.
> 
> This patch fixes the logic so that the runOnLoadHandlers gets called only after the gadget calls runOnLoadHandlers.
> 
> 
> Diffs
> -----
> 
>   /trunk/features/src/main/javascript/features/osapi/gadgetsrpctransport.js 1128387 
> 
> Diff: https://reviews.apache.org/r/795/diff
> 
> 
> Testing
> -------
> 
> 'mvn test' succeeds.
> 
> SocialHelloWorld demo seems to work reliably in Firefox now, even with Firebug's delay.
> 
> 
> Thanks,
> 
> Felix
> 
>

Re: Review Request: Fix some osapi+caja onload timing issues

Posted by Felix Lee <fe...@gmail.com>.

(Updated 2011-05-27 20:06:44.733464)

Review request for shindig.

Changes
-------

I'm separating out the caja/taming.js change, because it needs more extensive revision to work on Opera.

Summary (updated)
-------

osapi initialization requires doing a listMethods rpc, which is asynchronous. Since osapi isn't ready until that finishes, osapi replaces gadgets.util.runOnLoadHandlers with its own function that waits for listMethods to finish before firing off the onload handlers.