myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andy Schwartz <andy.g.schwa...@gmail.com>
Subject Fwd: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
Date Wed, 13 Jan 2010 15:42:29 GMT
MyFaces Gang -

I haven't had a chance to check to see whether this same issue exists
in MyFaces 2.0.  Thought I would pass this along in case anyone has
thoughts on the existing Facelets behavior (ie. can anyone think of a
reason why this behavior might be intentional?) and proposed fix.

Thanks,
Andy



---------- Forwarded message ----------
From: Andy Schwartz <andy.g.schwartz@gmail.com>
Date: Wed, Jan 13, 2010 at 10:25 AM
Subject: FaceletViewHandlingStrategy: cloneWithWriter() results in
duplicate ResponseWriters
To: dev@javaserverfaces.dev.java.net


Gang -

While looking into Trinidad/JSF 2 Ajax integration, I came across the
following very old comment in Trinidad:

 // Facelets - as of version 1.1.11 - does something
 // very strange with its ResponseWriter stack.
 // It starts with an ordinary stack (which will have
 // a PPRResponseWriter around an HtmlResponseWriter).
 // Then it treats that *as an ordinary Writer*, and
 // wraps it in a "StateWriter" class, which merely
 // is used to switch between passing output through
 // and buffering it.  Then it takes that StateWriter
 // and uses it to cloneWithWriter() a new ResponseWriter
 // stack!  As a result, we have the following stack
 // outermost to innermost:
 //   PPRResponseWriter
 //   HtmlResponseWriter
 //   StateWriter
 //   PPRResponseWriter
 //   HtmlResponseWriter
 //   ServletResponse's Writer
 // In the end, we have to get that "inner" PPRResponseWriter
 // to just cut it out and pass everything through.  Hence,
 // this hack!

The behavior described above still exists in Mojarra 2.0, in
FaceletViewHandlingStrategy.renderView():

           stateWriter = new WriteBehindStateWriter(origWriter,
                                                    ctx,
                                                    responseBufferSize);

           ResponseWriter writer = origWriter.cloneWithWriter(stateWriter);
           ctx.setResponseWriter(writer);

My guess is that the intention was not to duplicate the entire
response writer stack ("origWriter" above), but to insert the
WriteBehindStateWriter into the stack around the original Writer.
This is easily accomplished.  Instead of re-purposing the current
ResponseWriter as the Writer to use in the clone, we can simply grab
the Writer itself from the ExternalContext:

           ExternalContext extContext = ctx.getExternalContext();
           Writer outputWriter = extContext.getResponseOutputWriter()
           stateWriter = new WriteBehindStateWriter(outputWriter,
                                                    ctx,
                                                    responseBufferSize);

           ResponseWriter writer = origWriter.cloneWithWriter(stateWriter);

Note that FaceletViewHandlingStrategy uses this technique elsewhere -
in createResponseWriter() - which makes me think that the renderView()
approach was a simple oversight.  On the other hand, if this was as
simple as I think, seems like this would have been addressed sooner
(like when the Trinidad issue was originally uncovered in 2006), so I
would appreciate any thoughts on whether my analysis is off.

BTW, while this behavior impacts Trinidad in a noticeable way (and
thus forced the hack), it also has a more subtle impact on Mojarra
itself.  As far as I can tell, Mojarra's own HtmlResponseWriter ends
up in the response writer stack twice.  That is, before the clone, the
response writer stack for Mojarra looks like:

 HtmlResponseWriter
   PECoyoteResponse$PECoyoteWriter

After the clone, the stack is:

 HtmlResponseWriter
   WriteBehindStateWriter
     HtmlResponseWriter
       PECoyoteResponse$PECoyoteWriter

With my proposed fix above, the stack ends up as:

 HtmlResponseWriter
   WriteBehindStateWriter
     PECoyoteResponse$PECoyoteWriter

Pretty sure this was the original intention.  It certainly seems
cleaner to avoid duplicating the HtmlResponseWriter.

Thoughts?

Andy

Mime
View raw message