[Webkit-unassigned] [Bug 38171] WebSocket needs to suspend/resume as Active DOM object.

[bugzilla-daemon at webkit.org]

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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54390|review?                     |review-
               Flag|                            |




--- Comment #3 from Alexey Proskuryakov <ap at webkit.org>  2010-04-27 10:10:42 PST ---
(From update of attachment 54390)
+    , m_suspend(false)

Probably should be m_suspended.

+bool WebSocket::canSuspend() const
+{
+    return true;
+}

There are currently three uses for suspend/resume that I know of:

1. Suspend behind modal dialogs and alerts, so that program state wouldn't
change during an alert() call. WebSocket definitely needs that.

2. Suspend when a page goes into back/forward cache. One can't practically
suspend a socket for that long, so open WebSocket objects should prevent a page
from going to b/f cache. Fo this, canSuspend should return false - see how it's
done for XMLHttpRequest.

3. Suspend during a pause in debugger. I don't know much about how that works.

So in short, canSuspend() should return !m_channel.

+void WebSocket::suspend()
+{
+    m_suspend = true;
+}

I don't think it's sufficient. We need to make sure that readyState and other
properties don't change during an alert or modalDialog() calls.

-    dispatchEvent(Event::create(eventNames().openEvent, false, false));
+    enqueueEvent(Event::create(eventNames().openEvent, false, false));

The enqueueEvent() name makes it sound like some kind of deferred dispatch,
like "post a task to fire a simple event" in HTML5 parlance. But that's not how
it generally behaves.

+        Deque<RefPtr<Event> > m_pendingEvents;

Deque is not a great container to use here. We don't do random append/remove in
normal case - we fill the whole container first, and then empty it. See similar
patterns in Document and HTMLMediaElement.

Of these comments, the most difficult to address is of course insufficiency of
a boolean flag. I don't know how to best fix that, perhaps one needs to rely of
platform specific code in SocketStreamHandle, similarly to what we do for
XMLHttpRequest.

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