On Mon, Jun 8, 2009 at 7:20 PM, John Hjelmstad <[email protected]> wrote:
> Hello all, > Many of you have likely seen a series of rpc.js-related changes I've made > in > the past few weeks. These implemented several rpc.js improvements: > > 1. RMR, new transport for Safari < 4, Chrome < 2, and should work as a > backup on several other browsers (the latter claim requires more testing). > - Paves the way to sunsetting IFPC, in turn sunsetting the need for > containers to host "active" relay file rpc_relay.html and configure it > properly. > > 2. Refactoring transport code into separate "classes". This cleans up the > code a bit and makes it possible to optimize later by emitting only the JS > a > given browser needs rather than JS for all techniques. > > 3. Early-message queueing. Makes it possible to call gadgets.rpc.call(...) > in a container to a gadget before the gadget is initialized (or even > rendered), with the call still going through when the gadget is ready. It > also ensures that G -> C calls made before G -> C initialization is > complete > also succeed (this is much less prevalent). > > 4. Various cleanups including tentative backward-setup support. As a > convenience and to prevent user error, makes it possible to call > gadgets.rpc.setAuthToken(gadget, token); before the gadget IFRAME is > rendered. While not recommended in any case, this is intended to make the > library easier to use. > > At Google we found several pain points and subtle bugs associated with the > combination of these features, so I've rolled back the code for the moment > to give others another look at it. The code is here: > http://codereview.appspot.com/63209 > > > A few things learned during this process: > > * During integration testing of this library, it became exceedingly clear > that having automated integration testing for as many configurations of it > as possible is a must. In particular, one should test each browser, > container config, gadget server build, and container build (ideally the > matrix of all). We have implemented this within Google and will be > subjecting all rpc improvements to it. > > * Containers taking an rpc.js "snapshot" make improvements/upgrades > exceedingly difficult to impossible. As has been noted in several places, > the rpc.js version that the container loads should always be the exact same > as what the gadget loads. This is often achieved by the container including > <script src=" > http://gadgetserver.com/gadgets/js/rpc.js?c=1&container=1[&debug=1][&v=<http://gadgetserver.com/gadgets/js/rpc.js?c=1&container=1%5B&debug=1%5D%5B&v=> > <hash>]"></script>. You forgot the most important lesson: Nobody is supposed to be committing large patches without review. That is completely unacceptable and has always been. This code wasn't hosed because of the extent of the changes or the nature of rpc.js itself, it was hosed because it was not properly peer reviewed. Even the most alert and bright person will make mistakes in any patch of this size, and that's why we review big patches before we commit them. > > > In addition, I've implemented the base support for per-browser rpc.js > optimization on the server side, mentioned as the rationale for rpc.js > refactoring (#2 above). This code is experimental for the moment, and any > real-world deployment should be handled very carefully. Code reviewer link: > http://codereview.appspot.com/63210 > > Best, > John >
