Per-browser rpc.js

Implementation of JsFeatureLoader that loads only the rpc.js transport code needed by the target browser.

To use this, a concrete implementation of UserAgent.Parser must be provided and bound. In addition, JsFeatureLoader must be bound to implementation SmartRpcJsFeatureLoader.

This code is not widely tested, and should be deemed experimental.

[replaced by http://codereview.appspot.com/63225]

[Paul Lindner, 2009-06-09 17:48:06]

Looks interesting, however it doesn't appear that this sets the Vary or
Cache-Control headers to insure that content does not get cached in a CDN
inappropriately.

Use

Vary: User-Agent

or 

Cache-Control: private

to insure that the correct browser-based version is cached in the middle tier.

http://codereview.appspot.com/63210/diff/1/8
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/SmartRpcJsFeatureLoader.java
(right):

http://codereview.appspot.com/63210/diff/1/8#newcode136
Line 136: String getRpcContent(String keySuffix) {
It might be interesting to move this capability logic inside the userAgent
class/enum.  Also since 'new' browsers support html5 it might also be
interesting to have a fast lookup map that handles the common use cases.

[etnu00, 2009-06-09 17:48:08]

This code completely fails to take into account caches out in front of the
gadget renderer, which will cause this to break for anyone using a reverse proxy
that does caching. Given that everyone who wants to deploy separate servers for
gadget rendering and social data access needs a reverse proxy, there needs to be
much more done here.

http://codereview.appspot.com/63210/diff/1/8
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/SmartRpcJsFeatureLoader.java
(right):

http://codereview.appspot.com/63210/diff/1/8#newcode104
Line 104: StringBuffer allOpt = new StringBuffer();
There is no good reason why we should be using StringBuffers here.

[johnfargo, 2009-06-09 20:25:45]

Hi Paul:

You're absolutely right. Vary: User-Agent; is preferable to me. Do you have any
reason in mind for why Cache-Control: private in addition to or instead of Vary
would be preferable? Ie. common support by various CDNs.

I've been thinking how best to link service of custom rpc to setting this
header. Ultimately JsServlet or a Filter needs to do so. Thoughts on this? The
best idea I had was to add something like isBrowserSpecific() to JsLibrary so
that the relevant output code (gadget rendering, js servlet) could act
accordingly.

Kevin - re: StringBuffer, comment isn't particularly prescriptive so I'll assume
the complaint is use of StringBuffer rather than StringBuilder. If so, fixed. If
not, let me know.