You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Ryan Baxter <rb...@gmail.com> on 2011/11/02 00:39:26 UTC

Re: Review Request: Add callback function to print timing information for gadget preloading.

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

(Updated 2011-11-01 23:39:26.583345)


Review request for shindig.


Changes
-------

I refactored the timing code so it makes more sense.  Instead of navigating we are preloading.  I also added some gadget loading timing.


Summary
-------

We already have a callback function for navigating gadgets in the common container which can be used to print timing information. Problem is if you preload the gadget before navigating the timing information during navigation is 0. It would be helpful to print timing information for preloading as well.


This addresses bug SHINDIG-1653.
    https://issues.apache.org/jira/browse/SHINDIG-1653


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/util_test.js 1190425 

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


Testing
-------

Modified sample container to log this information.


Thanks,

Ryan


Re: Review Request: Add additional lifecycle handlers

Posted by Jesse Ciancetta <jc...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3391
-----------------------------------------------------------

Ship it!


LGTM

- Jesse


On 2011-11-18 21:56:17, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-18 21:56:17)
> 
> 
> Review request for shindig, Matthew Hatem and Igor Belakovskiy.
> 
> 
> Summary
> -------
> 
> I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Add additional lifecycle handlers

Posted by Matthew Hatem <mh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3405
-----------------------------------------------------------

Ship it!


I only reviewed the changes to the actions feature.  LFTM.

- Matthew


On 2011-11-18 21:56:17, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-18 21:56:17)
> 
> 
> Review request for shindig, Matthew Hatem and Igor Belakovskiy.
> 
> 
> Summary
> -------
> 
> I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Add additional lifecycle handlers

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3534
-----------------------------------------------------------

Ship it!


LGTM

- Dan


On 2011-11-18 21:56:17, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-18 21:56:17)
> 
> 
> Review request for shindig, Matthew Hatem and Igor Belakovskiy.
> 
> 
> Summary
> -------
> 
> I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Add additional lifecycle handlers

Posted by Igor Belakovskiy <bu...@alum.mit.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3400
-----------------------------------------------------------

Ship it!


LGTM

- Igor


On 2011-11-18 21:56:17, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-18 21:56:17)
> 
> 
> Review request for shindig, Matthew Hatem and Igor Belakovskiy.
> 
> 
> Summary
> -------
> 
> I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Add additional lifecycle handlers

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/
-----------------------------------------------------------

(Updated 2011-11-18 21:56:17.267012)


Review request for shindig, Matthew Hatem and Igor Belakovskiy.


Changes
-------

Adding Igor and Matt for their awareness.


Summary
-------

I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.


This addresses bug SHINDIG-1653.
    https://issues.apache.org/jira/browse/SHINDIG-1653


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 

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


Testing
-------

Modified sample container to log this information and updated unit tests.


Thanks,

Ryan


Re: Review Request: Add additional lifecycle handlers

Posted by Ryan Baxter <rb...@gmail.com>.

> On 2011-11-18 21:51:25, Stanton Sievers wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js, line 556
> > <https://reviews.apache.org/r/2616/diff/4/?file=59402#file59402line556>
> >
> >     Do actions not care about the ON_CLOSED event anymore?

ON_CLOSED used to get called before the site was actually closed, so I moved it after we close the site.  Now ON_BEFORE_CLOSE gets called before site.close so in order to not regress anything actions and open search should listen for on_before_close.  I will add Igor and Matt to the review so they are aware.


- Ryan


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


On 2011-11-18 21:16:31, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-18 21:16:31)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Add additional lifecycle handlers

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3363
-----------------------------------------------------------

Ship it!


A few questions, but LGTM.


http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/2616/#comment7542>

    Do actions not care about the ON_CLOSED event anymore?



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js
<https://reviews.apache.org/r/2616/#comment7543>

    Same as actions... do we not care about ON_CLOSED anymore?


- Stanton


On 2011-11-18 21:16:31, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-18 21:16:31)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Add additional lifecycle handlers

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/
-----------------------------------------------------------

(Updated 2011-11-18 21:16:31.247556)


Review request for shindig.


Changes
-------

Updated description


Summary (updated)
-------

I have added additional lifecycle handlers.  My primary use case here is to be able to time the different events that take place as a gadget is rendered, but these additional lifecycle handlers may be used for other things as well.


This addresses bug SHINDIG-1653.
    https://issues.apache.org/jira/browse/SHINDIG-1653


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 

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


Testing
-------

Modified sample container to log this information and updated unit tests.


Thanks,

Ryan


Re: Review Request: Refactor timing code in the common container

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/
-----------------------------------------------------------

(Updated 2011-11-18 21:12:15.796911)


Review request for shindig.


Changes
-------

Updated approach using lifecycle listeners.


Summary
-------

We already have a callback function for navigating gadgets in the common container which can be used to print timing information. Problem is if you preload the gadget before navigating the timing information during navigation is 0. It would be helpful to print timing information for preloading as well.


This addresses bug SHINDIG-1653.
    https://issues.apache.org/jira/browse/SHINDIG-1653


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/extras/src/main/javascript/features-extras/org.openajax.hub-2.0.5/iframe.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js 1203746 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1203746 

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


Testing
-------

Modified sample container to log this information and updated unit tests.


Thanks,

Ryan


Re: Review Request: Refactor timing code in the common container

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3105
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js
<https://reviews.apache.org/r/2616/#comment6912>

    Is there harm in setting this if it *is* undefined?



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js
<https://reviews.apache.org/r/2616/#comment6913>

    Can you check truthy here?
    if (this.gadgetLoadedCallback_)
    
    Empty string is falsey in js, you don't need to explicitly check for it.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js
<https://reviews.apache.org/r/2616/#comment6914>

    Can you use double quotes to quote the single quote strings?
    
    "({'" + blah + "':'"
    
    That could eliminate all of those slashes.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
<https://reviews.apache.org/r/2616/#comment6915>

    Again, is truthiness something that you can use here instead of the typeof and emptystring check?


- Dan


On 2011-11-01 23:49:25, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-01 23:49:25)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> We already have a callback function for navigating gadgets in the common container which can be used to print timing information. Problem is if you preload the gadget before navigating the timing information during navigation is 0. It would be helpful to print timing information for preloading as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/util_test.js 1190425 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Refactor timing code in the common container

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/#review3090
-----------------------------------------------------------


Looks good, I'm not too familiar with the timing stuff as it was..

What does strike me though is how similar in concept it is to life-cycle events. I took a look at the lifecycle events:
    ON_PRELOADED: 'onPreloaded',
    ON_NAVIGATED: 'onNavigated',
    ON_CLOSED: 'onClosed',
    ON_UNLOADED: 'onUnloaded',
    ON_RENDER: 'onRender'

And it looks like they could be enhanced (if no one has any objections) to add the ability to do timings so we don't have to bake the code into shindig (a container impl could provide it)

We would need:
ON_BEFORE_PRELOAD
ON_BEFORE_NAVIGATE
ON_BEFORE_CLOSE
ON_BEFORE_RENDER

Unfortunately, ON_RENDER actually gets fired BEFORE the render call.  It would be nice to replace this with ON_BEFORE_RENDER and make ON_RENDER fire in the onload event of the iframe.

Anyone else have any thoughts?

- Dan


On 2011-11-01 23:49:25, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2616/
> -----------------------------------------------------------
> 
> (Updated 2011-11-01 23:49:25)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> We already have a callback function for navigating gadgets in the common container which can be used to print timing information. Problem is if you preload the gadget before navigating the timing information during navigation is 0. It would be helpful to print timing information for preloading as well.
> 
> 
> This addresses bug SHINDIG-1653.
>     https://issues.apache.org/jira/browse/SHINDIG-1653
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1190425 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/util_test.js 1190425 
> 
> Diff: https://reviews.apache.org/r/2616/diff
> 
> 
> Testing
> -------
> 
> Modified sample container to log this information and updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Refactor timing code in the common container

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/
-----------------------------------------------------------

(Updated 2011-11-01 23:49:25.743392)


Review request for shindig.


Summary
-------

We already have a callback function for navigating gadgets in the common container which can be used to print timing information. Problem is if you preload the gadget before navigating the timing information during navigation is 0. It would be helpful to print timing information for preloading as well.


This addresses bug SHINDIG-1653.
    https://issues.apache.org/jira/browse/SHINDIG-1653


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/util_test.js 1190425 

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


Testing (updated)
-------

Modified sample container to log this information and updated unit tests.


Thanks,

Ryan


Re: Review Request: Refactor timing code in the common container

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2616/
-----------------------------------------------------------

(Updated 2011-11-01 23:39:54.276220)


Review request for shindig.


Summary (updated)
-------

We already have a callback function for navigating gadgets in the common container which can be used to print timing information. Problem is if you preload the gadget before navigating the timing information during navigation is 0. It would be helpful to print timing information for preloading as well.


This addresses bug SHINDIG-1653.
    https://issues.apache.org/jira/browse/SHINDIG-1653


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1190425 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/util_test.js 1190425 

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


Testing
-------

Modified sample container to log this information.


Thanks,

Ryan