----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/#review3681 -----------------------------------------------------------
Overall this seems OK. Have you looked at the performance impact here using lifecycle logging? The extra HTTP round trip could be costly, especially none of the HEAD request can be cached and reused by the subsequent GET. Just throwing it out there, how much sense would it make to do a GET instead of a HEAD request to check for page availability? My reasoning here is that at least the browser will have cached the page contents from the initial GET making the subsequent GET more performant. In the error case the HEAD and GET will take approximately the same amount of time to fail. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js <https://reviews.apache.org/r/3037/#comment8194> I'd rather be checking for a status of any 2xx code. I'd rather treat redirects and other 3xx codes as failures if they are going to fail at render time anyway. Also, what is the difference between checking response.error and response.status. Shouldn't they indicate the same thing? http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/3037/#comment8195> Same here for the response status as my previous comment. I'd rather check that it's not a 2xx status. - Stanton On 2011-12-06 22:36:38, Ryan Baxter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3037/ > ----------------------------------------------------------- > > (Updated 2011-12-06 22:36:38) > > > Review request for shindig, Dan Dumont and Stanton Sievers. > > > Summary > ------- > > When you call commoncontainer.navigateUrl if the URL cannot be reached the > caller has no way of knowing if the URL was navigated successfully or not. To > solve this we make a head request to the URL we are navigating to and add a > callback to the API. > > It is important to note that we will not be caching the response of the head > request. While this could possibly give us better performance we have no way > of guaranteeing the server will still be up next time and everything may > fail. This is different from the gadget case where we have the gadget XML > cached on the server. > > > This addresses bug SHINDIG-1669. > https://issues.apache.org/jira/browse/SHINDIG-1669 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/embeddedexperiences/PhotoList.xml > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container.url/container_url_test.js > 1211103 > > Diff: https://reviews.apache.org/r/3037/diff > > > Testing > ------- > > Tested in container as well as updating unit tests. > > > Thanks, > > Ryan > >
