[Webkit-unassigned] [Bug 14997] Support for server-sent DOM events

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=14997


adam.bergkvist at ericsson.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30050|0                           |1
        is obsolete|                            |
  Attachment #30778|                            |review?
               Flag|                            |




------- Comment #21 from adam.bergkvist at ericsson.com  2009-05-29 10:16 PDT -------
Created an attachment (id=30778)
 --> (https://bugs.webkit.org/attachment.cgi?id=30778&action=view)
updated patch

(In reply to comment #20)

Thanks for your feedback.

> 1) I think it would be ok to enable this by default for testing, unless there
> is some critical problem with it.
Done. Note that I have only updated the GTK build system since it is the
platform I am testing on.

> 2) It would be nice if constructors (real ones like this, not fake ones like
> Element) could be autogenerated. I guess that is not a new issue with your
> patch, so you don't have to fix this right away, just noticing.
I am not sure how to automatically generate constructors that take arguments.
This could perhaps be fixed later.

> 3) might be good to file a follow-up bug on this: // FIXME: should support
> cross-origin requests
Sure, will do that as soon as this patch is landed.

> 4) EventSource::parseEventStream looks a little large for a single function. Is
> there a way to break it up into smaller functions for understandability?
Line parsing in EventSource::parseEventStream has been extracted to a separate
function (EventSource::parseEventStreamLine).

> 5) A couple of times the code removes UChars from the beginning of the
> m_receiveBuf vector:  m_receiveBuf.remove(0, removeLen);
> 
> Removing from the start of a Vector is very inefficient. It copies almost all
> the contents of the vector every time. I would consider using a different data
> structure here. One possibility is to keep each incoming chunk of text as a
> separate Vector or String, and just track where you are in the current segment,
> and drop it once the whole thing has been processed. Another possibility is to
> use a Deque, which allows for relatively efficient prepend or remove from
> start.
Removing from the beginning of the Vector is now reduced to being done when all
lines in one chunk has been parsed.

> 6) This patch needs regression tests. I think it would be good to add http
> tests including both normal Window versions and Worker versions. Tests should
> ideally cover both normal event delivery, and handling of malformed EventSource
> streams, with particular attention to edge cases.
Added various tests.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.