You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Kevin Brown <et...@google.com> on 2009/08/19 05:29:45 UTC

Re: svn commit: r805532 - in /incubator/shindig/trunk: features/src/main/javascript/features/caja/taming.js java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java pom.xml

It looks to me that at this point the only major barriers left to caja
adoption are taming related. I think we need:
1. Tame osapi and various other libraries not covered by the big taming blob
currently in the caja feature
2. Refactor that big blob so that each feature does its own taming (the caja
feature shouldn't know anything about specific libraries)
3. Provide some clear instruction on the preferred way to deal with common
third party libraries. Are we going with a model where the entire DOM is
tamed and most stuff should just work, or will we need developers to use
tamed versions of each library (and somehow provide them). This is a very
confusing topic.

I think if we can address those 3 issues, most gadgets should be able to
work with valija. I'll be happy to take on 1 and 2 if caja folks could lend
some insight into #3.

On Tue, Aug 18, 2009 at 11:36 AM, <lr...@apache.org> wrote:

> Author: lryan
> Date: Tue Aug 18 18:36:29 2009
> New Revision: 805532
>
> URL: http://svn.apache.org/viewvc?rev=805532&view=rev
> Log:
> Applied patch from Jasvir Nagra to avoid reparsing document while Cajoling.
> SHINDIG-1146. Includes update to Caja r3638
>
> Modified:
>
>  incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>
>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>    incubator/shindig/trunk/pom.xml
>
> Modified:
> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js?rev=805532&r1=805531&r2=805532&view=diff
>
> ==============================================================================
> ---
> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
> (original)
> +++
> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
> Tue Aug 18 18:36:29 2009
> @@ -201,6 +201,8 @@
>     if (gadgets.views)
>       gadgets.views.getCurrentView
>           = taming.views.getCurrentView(gadgets.views.getCurrentView);
> +
> +    if (opensocial)
>       opensocial.newDataRequest = taming.newDataRequest(imports.$v,
>
> opensocial.newDataRequest);
>       if (gadgets.MiniMessage)
>
> Modified:
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java?rev=805532&r1=805531&r2=805532&view=diff
>
> ==============================================================================
> ---
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
> (original)
> +++
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
> Tue Aug 18 18:36:29 2009
> @@ -21,8 +21,10 @@
>  import org.apache.commons.lang.StringUtils;
>  import org.apache.shindig.gadgets.Gadget;
>  import org.apache.shindig.gadgets.rewrite.MutableContent;
> +
>  import org.w3c.dom.Document;
>  import org.w3c.dom.Element;
> +import org.w3c.dom.Node;
>
>  import java.io.IOException;
>  import java.io.InputStreamReader;
> @@ -32,16 +34,13 @@
>  import java.util.Map;
>  import java.util.logging.Logger;
>
> -import com.google.caja.lexer.CharProducer;
>  import com.google.caja.lexer.ExternalReference;
> -import com.google.caja.lexer.FilePosition;
>  import com.google.caja.lexer.InputSource;
>  import com.google.caja.lexer.escaping.Escaping;
>  import com.google.caja.opensocial.DefaultGadgetRewriter;
>  import com.google.caja.opensocial.GadgetRewriteException;
>  import com.google.caja.opensocial.UriCallback;
>  import com.google.caja.opensocial.UriCallbackException;
> -import com.google.caja.parser.html.Nodes;
>  import com.google.caja.reporting.BuildInfo;
>  import com.google.caja.reporting.Message;
>  import com.google.caja.reporting.MessageContext;
> @@ -49,6 +48,7 @@
>  import com.google.caja.reporting.MessageQueue;
>  import com.google.caja.reporting.SimpleMessageQueue;
>  import com.google.caja.reporting.SnippetProducer;
> +import com.google.caja.util.Pair;
>  import com.google.common.collect.Maps;
>
>  public class CajaContentRewriter implements
> org.apache.shindig.gadgets.rewrite.GadgetRewriter {
> @@ -90,36 +90,52 @@
>       DefaultGadgetRewriter rw = new DefaultGadgetRewriter(bi, mq);
>       rw.setValijaMode(true);
>       InputSource is = new InputSource(retrievedUri);
> -      String origContent = content.getContent();
> -      CharProducer input = CharProducer.Factory.create(
> -          new StringReader(origContent),
> -          FilePosition.instance(is, 5, 5, 5));
> -      StringBuilder output = new StringBuilder();
> -
>       Document doc = content.getDocument();
> +      Node root = doc.createDocumentFragment();
> +      root.appendChild(doc.getDocumentElement());
> +      boolean safe = false;
>       try {
> -        StringBuilder htmlAndJs = new StringBuilder();
> -        rw.rewriteContent(retrievedUri, input, cb, htmlAndJs);
> -        int splitPoint = htmlAndJs.indexOf("<script");
> -        String script = htmlAndJs.substring(splitPoint);
> -        String html = htmlAndJs.substring(0, splitPoint);
> -        String htmlElement =
> -          "<div id=\"cajoled-output\" class=\"g___\">" +
> -          html +
> -          "</div>";
> -        output.append(htmlElement);
> -        output.append(tameCajaClientApi());
> -        output.append(script);
> -      } catch (Exception e) {
> -        content.setContent(messagesToHtml(doc, is, origContent, mq));
> -        throwCajolingException(e, mq);
> -        return;
> +        Pair<Node, Element> htmlAndJs = rw.rewriteContent(retrievedUri,
> root,
> +            cb);
> +        Node html = htmlAndJs.a;
> +        Element script = htmlAndJs.b;
> +
> +        Element cajoledOutput = doc.createElement("div");
> +        cajoledOutput.setAttribute("id", "cajoled-output");
> +        cajoledOutput.setAttribute("classes", "g___");
> +        cajoledOutput.appendChild(doc.adoptNode(html));
> +        cajoledOutput.appendChild(tameCajaClientApi(doc));
> +        cajoledOutput.appendChild(doc.adoptNode(script));
> +
> +        createContainerFor(doc, cajoledOutput);
> +        content.documentChanged();
> +        safe = true;
> +      } catch (GadgetRewriteException e) {
> +        // There were cajoling errors
> +        // Content is only used to produce useful snippets with error
> messages
> +        createContainerFor(doc, formatErrors(doc, is,
> content.getContent(), mq));
> +        logException(e, mq);
> +      } finally {
> +        if (!safe) {
> +          // Fail safe
> +          content.setContent("");
> +        }
>       }
> -      content.setContent(output.toString());
>     }
>   }
>
> -  private String messagesToHtml(Document doc, InputSource is, CharSequence
> orig, MessageQueue mq) {
> +  private void createContainerFor(Document doc, Element el) {
> +    Element docEl = doc.createElement("html");
> +    Element head = doc.createElement("head");
> +    Element body = doc.createElement("body");
> +    doc.appendChild(docEl);
> +    docEl.appendChild(head);
> +    docEl.appendChild(body);
> +    body.appendChild(el);
> +  }
> +
> +  private Element formatErrors(Document doc, InputSource is,
> +      CharSequence orig, MessageQueue mq) {
>     MessageContext mc = new MessageContext();
>     Map<InputSource, CharSequence> originalSrc = Maps.newHashMap();
>     originalSrc.put(is, orig);
> @@ -131,10 +147,10 @@
>       // Ignore LINT messages
>       if (MessageLevel.LINT.compareTo(msg.getMessageLevel()) <= 0) {
>         String snippet = sp.getSnippet(msg);
> -
>         messageText.append(msg.getMessageLevel().name())
>                    .append(" ")
>                    .append(html(msg.format(mc)));
> +
>         if (!StringUtils.isEmpty(snippet)) {
>           messageText.append("\n").append(snippet);
>         }
> @@ -142,7 +158,7 @@
>     }
>     Element errElement = doc.createElement("pre");
>     errElement.appendChild(doc.createTextNode(messageText.toString()));
> -    return messageText.toString();
> +    return errElement;
>   }
>
>   private static String html(CharSequence s) {
> @@ -151,25 +167,22 @@
>     return sb.toString();
>   }
>
> -  private String tameCajaClientApi() {
> -    return "<script>___.enableCaja()</script>";
> +  private Element tameCajaClientApi(Document doc) {
> +    Element scriptElement = doc.createElement("script");
> +    scriptElement.setAttribute("type", "text/javascript");
> +    scriptElement.appendChild(doc.createTextNode("___.enableCaja()"));
> +    return scriptElement;
>   }
>
> -    private void throwCajolingException(Exception cause, MessageQueue mq)
> {
> +  private void logException(Exception cause, MessageQueue mq) {
>     StringBuilder errbuilder = new StringBuilder();
>     MessageContext mc = new MessageContext();
> -
>     if (cause != null) {
>       errbuilder.append(cause).append('\n');
>     }
> -
>     for (Message m : mq.getMessages()) {
>       errbuilder.append(m.format(mc)).append('\n');
>     }
> -
>     logger.info("Unable to cajole gadget: " + errbuilder);
> -
> -    // throw new GadgetException(
> -    //    GadgetException.Code.MALFORMED_FOR_SAFE_INLINING,
> errbuilder.toString());
>   }
>  }
>
> Modified: incubator/shindig/trunk/pom.xml
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/pom.xml?rev=805532&r1=805531&r2=805532&view=diff
>
> ==============================================================================
> --- incubator/shindig/trunk/pom.xml (original)
> +++ incubator/shindig/trunk/pom.xml Tue Aug 18 18:36:29 2009
> @@ -1247,7 +1247,7 @@
>       <dependency>
>         <groupId>caja</groupId>
>         <artifactId>caja</artifactId>
> -        <version>r3574</version>
> +        <version>r3638</version>
>         <scope>compile</scope>
>       </dependency>
>       <dependency>
>
>
>

Re: svn commit: r805532 - in /incubator/shindig/trunk: features/src/main/javascript/features/caja/taming.js java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java pom.xml

Posted by Jasvir Nagra <ja...@google.com>.
On Wed, Aug 19, 2009 at 9:28 PM, Jasvir Nagra <ja...@google.com> wrote:

>
>
> 2009/8/18 Kevin Brown <et...@google.com>
>
>> It looks to me that at this point the only major barriers left to caja
>> adoption are taming related. I think we need:
>> 1. Tame osapi and various other libraries not covered by the big taming
>> blob currently in the caja feature
>>
>
> The current taming was not designed well to handle the common pattern of
> callbacks.  Notice the number of cases where functions essentially get
> rewritten to unwrap the cajoled code, call the original and rewrap to return
> a value to cajoled code.  We're redoing the mechanism by which API
> developers tame their libraries - a library developer for each function will
> simply specify what the types of input parameters are and what the return
> value is - this will ease the load on the library developer.
>
>
>> 2. Refactor that big blob so that each feature does its own taming (the
>> caja feature shouldn't know anything about specific libraries)
>>
>
> All the mechanism for this is already there.  The opensocialSchema object
> literal can be broken up and moved to individual features where the
> corresponding api is defined.  Each feature can then register its taming
> with caja___.register() which are called when caja is loaded.  As a
> convention, I suggest a taming.js file in each feature which is listed last
> in the corresponding feature.xml.
>

Actually this change is still pending -
http://codereview.appspot.com/104067/show.


>
>
>> 3. Provide some clear instruction on the preferred way to deal with common
>> third party libraries. Are we going with a model where the entire DOM is
>> tamed and most stuff should just work, or will we need developers to use
>> tamed versions of each library (and somehow provide them). This is a very
>> confusing topic.
>>
>
> Do you mean libraries such as prototype, YUI and jQuery or feature
> libraries?  If its the former, this should just work modulo some caching
> that will need to be done by Shindig - rather than cajoling the library each
> time.  This is a serious performance bottleneck - addressing it makes an
> order of magnitude improvement rather than the linear improvements we get by
> addressing the reparsing issue.  In an offline discussion some months ago,
> lryan suggested he would look at what it would take to address this.  If its
> third-party feature libraries - library authors will need to provide a
> taming.js file which tames their API.  As I mentioned earier, we're making
> good progress on a simpler way for API developers to specify how their
> library ought to be tamed.
>
>
>>
>> I think if we can address those 3 issues, most gadgets should be able to
>> work with valija. I'll be happy to take on 1 and 2 if caja folks could lend
>> some insight into #3.
>>
>
> Awesome.
>
>
>>
>> On Tue, Aug 18, 2009 at 11:36 AM, <lr...@apache.org> wrote:
>>
>>> Author: lryan
>>> Date: Tue Aug 18 18:36:29 2009
>>> New Revision: 805532
>>>
>>> URL: http://svn.apache.org/viewvc?rev=805532&view=rev
>>> Log:
>>> Applied patch from Jasvir Nagra to avoid reparsing document while
>>> Cajoling. SHINDIG-1146. Includes update to Caja r3638
>>>
>>> Modified:
>>>
>>>  incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>>>
>>>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>>>    incubator/shindig/trunk/pom.xml
>>>
>>> Modified:
>>> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js?rev=805532&r1=805531&r2=805532&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>>> (original)
>>> +++
>>> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>>> Tue Aug 18 18:36:29 2009
>>> @@ -201,6 +201,8 @@
>>>     if (gadgets.views)
>>>       gadgets.views.getCurrentView
>>>           = taming.views.getCurrentView(gadgets.views.getCurrentView);
>>> +
>>> +    if (opensocial)
>>>       opensocial.newDataRequest = taming.newDataRequest(imports.$v,
>>>
>>> opensocial.newDataRequest);
>>>       if (gadgets.MiniMessage)
>>>
>>> Modified:
>>> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java?rev=805532&r1=805531&r2=805532&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>>> (original)
>>> +++
>>> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>>> Tue Aug 18 18:36:29 2009
>>> @@ -21,8 +21,10 @@
>>>  import org.apache.commons.lang.StringUtils;
>>>  import org.apache.shindig.gadgets.Gadget;
>>>  import org.apache.shindig.gadgets.rewrite.MutableContent;
>>> +
>>>  import org.w3c.dom.Document;
>>>  import org.w3c.dom.Element;
>>> +import org.w3c.dom.Node;
>>>
>>>  import java.io.IOException;
>>>  import java.io.InputStreamReader;
>>> @@ -32,16 +34,13 @@
>>>  import java.util.Map;
>>>  import java.util.logging.Logger;
>>>
>>> -import com.google.caja.lexer.CharProducer;
>>>  import com.google.caja.lexer.ExternalReference;
>>> -import com.google.caja.lexer.FilePosition;
>>>  import com.google.caja.lexer.InputSource;
>>>  import com.google.caja.lexer.escaping.Escaping;
>>>  import com.google.caja.opensocial.DefaultGadgetRewriter;
>>>  import com.google.caja.opensocial.GadgetRewriteException;
>>>  import com.google.caja.opensocial.UriCallback;
>>>  import com.google.caja.opensocial.UriCallbackException;
>>> -import com.google.caja.parser.html.Nodes;
>>>  import com.google.caja.reporting.BuildInfo;
>>>  import com.google.caja.reporting.Message;
>>>  import com.google.caja.reporting.MessageContext;
>>> @@ -49,6 +48,7 @@
>>>  import com.google.caja.reporting.MessageQueue;
>>>  import com.google.caja.reporting.SimpleMessageQueue;
>>>  import com.google.caja.reporting.SnippetProducer;
>>> +import com.google.caja.util.Pair;
>>>  import com.google.common.collect.Maps;
>>>
>>>  public class CajaContentRewriter implements
>>> org.apache.shindig.gadgets.rewrite.GadgetRewriter {
>>> @@ -90,36 +90,52 @@
>>>       DefaultGadgetRewriter rw = new DefaultGadgetRewriter(bi, mq);
>>>       rw.setValijaMode(true);
>>>       InputSource is = new InputSource(retrievedUri);
>>> -      String origContent = content.getContent();
>>> -      CharProducer input = CharProducer.Factory.create(
>>> -          new StringReader(origContent),
>>> -          FilePosition.instance(is, 5, 5, 5));
>>> -      StringBuilder output = new StringBuilder();
>>> -
>>>       Document doc = content.getDocument();
>>> +      Node root = doc.createDocumentFragment();
>>> +      root.appendChild(doc.getDocumentElement());
>>> +      boolean safe = false;
>>>       try {
>>> -        StringBuilder htmlAndJs = new StringBuilder();
>>> -        rw.rewriteContent(retrievedUri, input, cb, htmlAndJs);
>>> -        int splitPoint = htmlAndJs.indexOf("<script");
>>> -        String script = htmlAndJs.substring(splitPoint);
>>> -        String html = htmlAndJs.substring(0, splitPoint);
>>> -        String htmlElement =
>>> -          "<div id=\"cajoled-output\" class=\"g___\">" +
>>> -          html +
>>> -          "</div>";
>>> -        output.append(htmlElement);
>>> -        output.append(tameCajaClientApi());
>>> -        output.append(script);
>>> -      } catch (Exception e) {
>>> -        content.setContent(messagesToHtml(doc, is, origContent, mq));
>>> -        throwCajolingException(e, mq);
>>> -        return;
>>> +        Pair<Node, Element> htmlAndJs = rw.rewriteContent(retrievedUri,
>>> root,
>>> +            cb);
>>> +        Node html = htmlAndJs.a;
>>> +        Element script = htmlAndJs.b;
>>> +
>>> +        Element cajoledOutput = doc.createElement("div");
>>> +        cajoledOutput.setAttribute("id", "cajoled-output");
>>> +        cajoledOutput.setAttribute("classes", "g___");
>>> +        cajoledOutput.appendChild(doc.adoptNode(html));
>>> +        cajoledOutput.appendChild(tameCajaClientApi(doc));
>>> +        cajoledOutput.appendChild(doc.adoptNode(script));
>>> +
>>> +        createContainerFor(doc, cajoledOutput);
>>> +        content.documentChanged();
>>> +        safe = true;
>>> +      } catch (GadgetRewriteException e) {
>>> +        // There were cajoling errors
>>> +        // Content is only used to produce useful snippets with error
>>> messages
>>> +        createContainerFor(doc, formatErrors(doc, is,
>>> content.getContent(), mq));
>>> +        logException(e, mq);
>>> +      } finally {
>>> +        if (!safe) {
>>> +          // Fail safe
>>> +          content.setContent("");
>>> +        }
>>>       }
>>> -      content.setContent(output.toString());
>>>     }
>>>   }
>>>
>>> -  private String messagesToHtml(Document doc, InputSource is,
>>> CharSequence orig, MessageQueue mq) {
>>> +  private void createContainerFor(Document doc, Element el) {
>>> +    Element docEl = doc.createElement("html");
>>> +    Element head = doc.createElement("head");
>>> +    Element body = doc.createElement("body");
>>> +    doc.appendChild(docEl);
>>> +    docEl.appendChild(head);
>>> +    docEl.appendChild(body);
>>> +    body.appendChild(el);
>>> +  }
>>> +
>>> +  private Element formatErrors(Document doc, InputSource is,
>>> +      CharSequence orig, MessageQueue mq) {
>>>     MessageContext mc = new MessageContext();
>>>     Map<InputSource, CharSequence> originalSrc = Maps.newHashMap();
>>>     originalSrc.put(is, orig);
>>> @@ -131,10 +147,10 @@
>>>       // Ignore LINT messages
>>>       if (MessageLevel.LINT.compareTo(msg.getMessageLevel()) <= 0) {
>>>         String snippet = sp.getSnippet(msg);
>>> -
>>>         messageText.append(msg.getMessageLevel().name())
>>>                    .append(" ")
>>>                    .append(html(msg.format(mc)));
>>> +
>>>         if (!StringUtils.isEmpty(snippet)) {
>>>           messageText.append("\n").append(snippet);
>>>         }
>>> @@ -142,7 +158,7 @@
>>>     }
>>>     Element errElement = doc.createElement("pre");
>>>     errElement.appendChild(doc.createTextNode(messageText.toString()));
>>> -    return messageText.toString();
>>> +    return errElement;
>>>   }
>>>
>>>   private static String html(CharSequence s) {
>>> @@ -151,25 +167,22 @@
>>>     return sb.toString();
>>>   }
>>>
>>> -  private String tameCajaClientApi() {
>>> -    return "<script>___.enableCaja()</script>";
>>> +  private Element tameCajaClientApi(Document doc) {
>>> +    Element scriptElement = doc.createElement("script");
>>> +    scriptElement.setAttribute("type", "text/javascript");
>>> +    scriptElement.appendChild(doc.createTextNode("___.enableCaja()"));
>>> +    return scriptElement;
>>>   }
>>>
>>> -    private void throwCajolingException(Exception cause, MessageQueue
>>> mq) {
>>> +  private void logException(Exception cause, MessageQueue mq) {
>>>     StringBuilder errbuilder = new StringBuilder();
>>>     MessageContext mc = new MessageContext();
>>> -
>>>     if (cause != null) {
>>>       errbuilder.append(cause).append('\n');
>>>     }
>>> -
>>>     for (Message m : mq.getMessages()) {
>>>       errbuilder.append(m.format(mc)).append('\n');
>>>     }
>>> -
>>>     logger.info("Unable to cajole gadget: " + errbuilder);
>>> -
>>> -    // throw new GadgetException(
>>> -    //    GadgetException.Code.MALFORMED_FOR_SAFE_INLINING,
>>> errbuilder.toString());
>>>   }
>>>  }
>>>
>>> Modified: incubator/shindig/trunk/pom.xml
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/shindig/trunk/pom.xml?rev=805532&r1=805531&r2=805532&view=diff
>>>
>>> ==============================================================================
>>> --- incubator/shindig/trunk/pom.xml (original)
>>> +++ incubator/shindig/trunk/pom.xml Tue Aug 18 18:36:29 2009
>>> @@ -1247,7 +1247,7 @@
>>>       <dependency>
>>>         <groupId>caja</groupId>
>>>         <artifactId>caja</artifactId>
>>> -        <version>r3574</version>
>>> +        <version>r3638</version>
>>>         <scope>compile</scope>
>>>       </dependency>
>>>       <dependency>
>>>
>>>
>>>
>>
>

Re: svn commit: r805532 - in /incubator/shindig/trunk: features/src/main/javascript/features/caja/taming.js java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java pom.xml

Posted by Jasvir Nagra <ja...@google.com>.
2009/8/18 Kevin Brown <et...@google.com>

> It looks to me that at this point the only major barriers left to caja
> adoption are taming related. I think we need:
> 1. Tame osapi and various other libraries not covered by the big taming
> blob currently in the caja feature
>

The current taming was not designed well to handle the common pattern of
callbacks.  Notice the number of cases where functions essentially get
rewritten to unwrap the cajoled code, call the original and rewrap to return
a value to cajoled code.  We're redoing the mechanism by which API
developers tame their libraries - a library developer for each function will
simply specify what the types of input parameters are and what the return
value is - this will ease the load on the library developer.


> 2. Refactor that big blob so that each feature does its own taming (the
> caja feature shouldn't know anything about specific libraries)
>

All the mechanism for this is already there.  The opensocialSchema object
literal can be broken up and moved to individual features where the
corresponding api is defined.  Each feature can then register its taming
with caja___.register() which are called when caja is loaded.  As a
convention, I suggest a taming.js file in each feature which is listed last
in the corresponding feature.xml.


> 3. Provide some clear instruction on the preferred way to deal with common
> third party libraries. Are we going with a model where the entire DOM is
> tamed and most stuff should just work, or will we need developers to use
> tamed versions of each library (and somehow provide them). This is a very
> confusing topic.
>

Do you mean libraries such as prototype, YUI and jQuery or feature
libraries?  If its the former, this should just work modulo some caching
that will need to be done by Shindig - rather than cajoling the library each
time.  This is a serious performance bottleneck - addressing it makes an
order of magnitude improvement rather than the linear improvements we get by
addressing the reparsing issue.  In an offline discussion some months ago,
lryan suggested he would look at what it would take to address this.  If its
third-party feature libraries - library authors will need to provide a
taming.js file which tames their API.  As I mentioned earier, we're making
good progress on a simpler way for API developers to specify how their
library ought to be tamed.


>
> I think if we can address those 3 issues, most gadgets should be able to
> work with valija. I'll be happy to take on 1 and 2 if caja folks could lend
> some insight into #3.
>

Awesome.


>
> On Tue, Aug 18, 2009 at 11:36 AM, <lr...@apache.org> wrote:
>
>> Author: lryan
>> Date: Tue Aug 18 18:36:29 2009
>> New Revision: 805532
>>
>> URL: http://svn.apache.org/viewvc?rev=805532&view=rev
>> Log:
>> Applied patch from Jasvir Nagra to avoid reparsing document while
>> Cajoling. SHINDIG-1146. Includes update to Caja r3638
>>
>> Modified:
>>
>>  incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>>
>>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>>    incubator/shindig/trunk/pom.xml
>>
>> Modified:
>> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>> URL:
>> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js?rev=805532&r1=805531&r2=805532&view=diff
>>
>> ==============================================================================
>> ---
>> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>> (original)
>> +++
>> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js
>> Tue Aug 18 18:36:29 2009
>> @@ -201,6 +201,8 @@
>>     if (gadgets.views)
>>       gadgets.views.getCurrentView
>>           = taming.views.getCurrentView(gadgets.views.getCurrentView);
>> +
>> +    if (opensocial)
>>       opensocial.newDataRequest = taming.newDataRequest(imports.$v,
>>
>> opensocial.newDataRequest);
>>       if (gadgets.MiniMessage)
>>
>> Modified:
>> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java?rev=805532&r1=805531&r2=805532&view=diff
>>
>> ==============================================================================
>> ---
>> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>> (original)
>> +++
>> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
>> Tue Aug 18 18:36:29 2009
>> @@ -21,8 +21,10 @@
>>  import org.apache.commons.lang.StringUtils;
>>  import org.apache.shindig.gadgets.Gadget;
>>  import org.apache.shindig.gadgets.rewrite.MutableContent;
>> +
>>  import org.w3c.dom.Document;
>>  import org.w3c.dom.Element;
>> +import org.w3c.dom.Node;
>>
>>  import java.io.IOException;
>>  import java.io.InputStreamReader;
>> @@ -32,16 +34,13 @@
>>  import java.util.Map;
>>  import java.util.logging.Logger;
>>
>> -import com.google.caja.lexer.CharProducer;
>>  import com.google.caja.lexer.ExternalReference;
>> -import com.google.caja.lexer.FilePosition;
>>  import com.google.caja.lexer.InputSource;
>>  import com.google.caja.lexer.escaping.Escaping;
>>  import com.google.caja.opensocial.DefaultGadgetRewriter;
>>  import com.google.caja.opensocial.GadgetRewriteException;
>>  import com.google.caja.opensocial.UriCallback;
>>  import com.google.caja.opensocial.UriCallbackException;
>> -import com.google.caja.parser.html.Nodes;
>>  import com.google.caja.reporting.BuildInfo;
>>  import com.google.caja.reporting.Message;
>>  import com.google.caja.reporting.MessageContext;
>> @@ -49,6 +48,7 @@
>>  import com.google.caja.reporting.MessageQueue;
>>  import com.google.caja.reporting.SimpleMessageQueue;
>>  import com.google.caja.reporting.SnippetProducer;
>> +import com.google.caja.util.Pair;
>>  import com.google.common.collect.Maps;
>>
>>  public class CajaContentRewriter implements
>> org.apache.shindig.gadgets.rewrite.GadgetRewriter {
>> @@ -90,36 +90,52 @@
>>       DefaultGadgetRewriter rw = new DefaultGadgetRewriter(bi, mq);
>>       rw.setValijaMode(true);
>>       InputSource is = new InputSource(retrievedUri);
>> -      String origContent = content.getContent();
>> -      CharProducer input = CharProducer.Factory.create(
>> -          new StringReader(origContent),
>> -          FilePosition.instance(is, 5, 5, 5));
>> -      StringBuilder output = new StringBuilder();
>> -
>>       Document doc = content.getDocument();
>> +      Node root = doc.createDocumentFragment();
>> +      root.appendChild(doc.getDocumentElement());
>> +      boolean safe = false;
>>       try {
>> -        StringBuilder htmlAndJs = new StringBuilder();
>> -        rw.rewriteContent(retrievedUri, input, cb, htmlAndJs);
>> -        int splitPoint = htmlAndJs.indexOf("<script");
>> -        String script = htmlAndJs.substring(splitPoint);
>> -        String html = htmlAndJs.substring(0, splitPoint);
>> -        String htmlElement =
>> -          "<div id=\"cajoled-output\" class=\"g___\">" +
>> -          html +
>> -          "</div>";
>> -        output.append(htmlElement);
>> -        output.append(tameCajaClientApi());
>> -        output.append(script);
>> -      } catch (Exception e) {
>> -        content.setContent(messagesToHtml(doc, is, origContent, mq));
>> -        throwCajolingException(e, mq);
>> -        return;
>> +        Pair<Node, Element> htmlAndJs = rw.rewriteContent(retrievedUri,
>> root,
>> +            cb);
>> +        Node html = htmlAndJs.a;
>> +        Element script = htmlAndJs.b;
>> +
>> +        Element cajoledOutput = doc.createElement("div");
>> +        cajoledOutput.setAttribute("id", "cajoled-output");
>> +        cajoledOutput.setAttribute("classes", "g___");
>> +        cajoledOutput.appendChild(doc.adoptNode(html));
>> +        cajoledOutput.appendChild(tameCajaClientApi(doc));
>> +        cajoledOutput.appendChild(doc.adoptNode(script));
>> +
>> +        createContainerFor(doc, cajoledOutput);
>> +        content.documentChanged();
>> +        safe = true;
>> +      } catch (GadgetRewriteException e) {
>> +        // There were cajoling errors
>> +        // Content is only used to produce useful snippets with error
>> messages
>> +        createContainerFor(doc, formatErrors(doc, is,
>> content.getContent(), mq));
>> +        logException(e, mq);
>> +      } finally {
>> +        if (!safe) {
>> +          // Fail safe
>> +          content.setContent("");
>> +        }
>>       }
>> -      content.setContent(output.toString());
>>     }
>>   }
>>
>> -  private String messagesToHtml(Document doc, InputSource is,
>> CharSequence orig, MessageQueue mq) {
>> +  private void createContainerFor(Document doc, Element el) {
>> +    Element docEl = doc.createElement("html");
>> +    Element head = doc.createElement("head");
>> +    Element body = doc.createElement("body");
>> +    doc.appendChild(docEl);
>> +    docEl.appendChild(head);
>> +    docEl.appendChild(body);
>> +    body.appendChild(el);
>> +  }
>> +
>> +  private Element formatErrors(Document doc, InputSource is,
>> +      CharSequence orig, MessageQueue mq) {
>>     MessageContext mc = new MessageContext();
>>     Map<InputSource, CharSequence> originalSrc = Maps.newHashMap();
>>     originalSrc.put(is, orig);
>> @@ -131,10 +147,10 @@
>>       // Ignore LINT messages
>>       if (MessageLevel.LINT.compareTo(msg.getMessageLevel()) <= 0) {
>>         String snippet = sp.getSnippet(msg);
>> -
>>         messageText.append(msg.getMessageLevel().name())
>>                    .append(" ")
>>                    .append(html(msg.format(mc)));
>> +
>>         if (!StringUtils.isEmpty(snippet)) {
>>           messageText.append("\n").append(snippet);
>>         }
>> @@ -142,7 +158,7 @@
>>     }
>>     Element errElement = doc.createElement("pre");
>>     errElement.appendChild(doc.createTextNode(messageText.toString()));
>> -    return messageText.toString();
>> +    return errElement;
>>   }
>>
>>   private static String html(CharSequence s) {
>> @@ -151,25 +167,22 @@
>>     return sb.toString();
>>   }
>>
>> -  private String tameCajaClientApi() {
>> -    return "<script>___.enableCaja()</script>";
>> +  private Element tameCajaClientApi(Document doc) {
>> +    Element scriptElement = doc.createElement("script");
>> +    scriptElement.setAttribute("type", "text/javascript");
>> +    scriptElement.appendChild(doc.createTextNode("___.enableCaja()"));
>> +    return scriptElement;
>>   }
>>
>> -    private void throwCajolingException(Exception cause, MessageQueue mq)
>> {
>> +  private void logException(Exception cause, MessageQueue mq) {
>>     StringBuilder errbuilder = new StringBuilder();
>>     MessageContext mc = new MessageContext();
>> -
>>     if (cause != null) {
>>       errbuilder.append(cause).append('\n');
>>     }
>> -
>>     for (Message m : mq.getMessages()) {
>>       errbuilder.append(m.format(mc)).append('\n');
>>     }
>> -
>>     logger.info("Unable to cajole gadget: " + errbuilder);
>> -
>> -    // throw new GadgetException(
>> -    //    GadgetException.Code.MALFORMED_FOR_SAFE_INLINING,
>> errbuilder.toString());
>>   }
>>  }
>>
>> Modified: incubator/shindig/trunk/pom.xml
>> URL:
>> http://svn.apache.org/viewvc/incubator/shindig/trunk/pom.xml?rev=805532&r1=805531&r2=805532&view=diff
>>
>> ==============================================================================
>> --- incubator/shindig/trunk/pom.xml (original)
>> +++ incubator/shindig/trunk/pom.xml Tue Aug 18 18:36:29 2009
>> @@ -1247,7 +1247,7 @@
>>       <dependency>
>>         <groupId>caja</groupId>
>>         <artifactId>caja</artifactId>
>> -        <version>r3574</version>
>> +        <version>r3638</version>
>>         <scope>compile</scope>
>>       </dependency>
>>       <dependency>
>>
>>
>>
>