[webkit-reviews] review denied: [Bug 14997] Support for server-sent DOM events : [Attachment 34139] updated patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 7 13:50:27 PDT 2009
Sam Weinig <sam at webkit.org> has denied Adam Bergkvist
<adam.bergkvist at ericsson.com>'s request for review:
Bug 14997: Support for server-sent DOM events
https://bugs.webkit.org/show_bug.cgi?id=14997
Attachment 34139: updated patch
https://bugs.webkit.org/attachment.cgi?id=34139&action=review
------- Additional Comments from Sam Weinig <sam at webkit.org>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 46804)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,12 @@
> +2009-08-05 Adam Bergkvist <adam.bergkvist at ericsson.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [GTK] Added EventSource to the build (default on).
> + https://bugs.webkit.org/show_bug.cgi?id=14997
> +
> + * configure.ac:
> +
> 2009-08-05 Jan Michael Alonzo <jmalonzo at webkit.org>
>
> Reviewed by Xan Lopez.
> Index: configure.ac
> ===================================================================
> --- configure.ac (revision 46804)
> +++ configure.ac (working copy)
> @@ -348,6 +348,14 @@ AC_ARG_ENABLE(database,
> [],[enable_database="yes"])
> AC_MSG_RESULT([$enable_database])
>
> +# check whether to build with server-sent events support
> +AC_MSG_CHECKING([whether to enable HTML5 server-sent events support])
> +AC_ARG_ENABLE(eventsource,
> + AC_HELP_STRING([--enable-eventsource],
> + [enable HTML5 server-sent events support
[default=yes]]),
> + [],[enable_eventsource="yes"])
> +AC_MSG_RESULT([$enable_eventsource])
> +
> # check whether to build with icon database support
> AC_MSG_CHECKING([whether to enable icon database support])
> AC_ARG_ENABLE(icon_database,
> @@ -702,6 +710,7 @@ AM_CONDITIONAL([ENABLE_JAVASCRIPT_DEBUGG
> AM_CONDITIONAL([ENABLE_OFFLINE_WEB_APPLICATIONS],[test
"$enable_offline_web_applications" = "yes"])
> AM_CONDITIONAL([ENABLE_DOM_STORAGE],[test "$enable_dom_storage" = "yes"])
> AM_CONDITIONAL([ENABLE_DATABASE],[test "$enable_database" = "yes"])
> +AM_CONDITIONAL([ENABLE_EVENTSOURCE],[test "$enable_eventsource" = "yes"])
> AM_CONDITIONAL([ENABLE_ICONDATABASE],[test "$enable_icon_database" = "yes"])
> AM_CONDITIONAL([ENABLE_XPATH],[test "$enable_xpath" = "yes"])
> AM_CONDITIONAL([ENABLE_XSLT],[test "$enable_xslt" = "yes"])
> @@ -758,6 +767,7 @@ Features:
> HTML5 client-side session and persistent storage support :
$enable_dom_storage
> HTML5 client-side database storage support : $enable_database
> HTML5 ruby support : $enable_ruby
> + HTML5 server-sent events support :
$enable_eventsource
> HTML5 video element support : $enable_video
> Icon database support :
$enable_icon_database
> SharedWorkers support :
$enable_shared_workers
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 46804)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,87 @@
> +2009-08-05 Adam Bergkvist <adam.bergkvist at ericsson.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Added implementation of the EventSource object that enables
> + server-sent events from HTML5.
> + http://dev.w3.org/html5/eventsource/
> + https://bugs.webkit.org/show_bug.cgi?id=14997
> +
> + Tests: fast/eventsource/eventsource-attribute-listeners.html
> + fast/eventsource/eventsource-constructor.html
> + http/tests/eventsource/eventsource-bad-mime-type.html
> + http/tests/eventsource/eventsource-parse-event-stream.html
> + http/tests/eventsource/eventsource-reconnect.html
> + http/tests/eventsource/eventsource-status-code-states.html
> + http/tests/eventsource/workers/eventsource-simple.html
> +
> + * GNUmakefile.am:
> + * WebCore.pro:
> + * bindings/js/JSDOMWindowCustom.cpp:
> + (WebCore::JSDOMWindow::eventSource):
> + * bindings/js/JSEventSourceConstructor.cpp: Added.
> + (WebCore::):
> + (WebCore::JSEventSourceConstructor::JSEventSourceConstructor):
> + (WebCore::JSEventSourceConstructor::scriptExecutionContext):
> + (WebCore::constructEventSource):
> + (WebCore::JSEventSourceConstructor::getConstructData):
> + (WebCore::JSEventSourceConstructor::mark):
> + * bindings/js/JSEventSourceConstructor.h: Added.
> + (WebCore::JSEventSourceConstructor::classInfo):
> + * bindings/js/JSEventSourceCustom.cpp: Added.
> + (WebCore::JSEventSource::mark):
> + (WebCore::JSEventSource::addEventListener):
> + (WebCore::JSEventSource::removeEventListener):
> + * bindings/js/JSEventTarget.cpp:
> + (WebCore::toJS):
> + (WebCore::toEventTarget):
> + * bindings/js/JSWorkerContextCustom.cpp:
> + (WebCore::JSWorkerContext::eventSource):
> + * dom/EventNames.h:
> + * dom/EventTarget.cpp:
> + (WebCore::EventTarget::toEventSource):
> + * dom/EventTarget.h:
> + * page/DOMWindow.idl:
> + * page/EventSource.cpp: Added.
> + (WebCore::EventSource::EventSource):
> + (WebCore::EventSource::~EventSource):
> + (WebCore::EventSource::connect):
> + (WebCore::EventSource::endRequest):
> + (WebCore::EventSource::scheduleReconnect):
> + (WebCore::EventSource::reconnectTimerFired):
> + (WebCore::EventSource::url):
> + (WebCore::EventSource::readyState):
> + (WebCore::EventSource::close):
> + (WebCore::EventSource::scriptExecutionContext):
> + (WebCore::EventSource::addEventListener):
> + (WebCore::EventSource::removeEventListener):
> + (WebCore::EventSource::dispatchEvent):
> + (WebCore::EventSource::didReceiveResponse):
> + (WebCore::EventSource::didReceiveData):
> + (WebCore::EventSource::didFinishLoading):
> + (WebCore::EventSource::didFail):
> + (WebCore::EventSource::didFailRedirectCheck):
> + (WebCore::EventSource::parseEventStream):
> + (WebCore::EventSource::parseEventStreamLine):
> + (WebCore::EventSource::dispatchGenericEvent):
> + (WebCore::EventSource::dispatchMessageEvent):
> + (WebCore::EventSource::stop):
> + * page/EventSource.h: Added.
> + (WebCore::EventSource::create):
> + (WebCore::EventSource::):
> + (WebCore::EventSource::setOnopen):
> + (WebCore::EventSource::onopen):
> + (WebCore::EventSource::setOnmessage):
> + (WebCore::EventSource::onmessage):
> + (WebCore::EventSource::setOnerror):
> + (WebCore::EventSource::onerror):
> + (WebCore::EventSource::toEventSource):
> + (WebCore::EventSource::eventListeners):
> + (WebCore::EventSource::refEventTarget):
> + (WebCore::EventSource::derefEventTarget):
> + * page/EventSource.idl: Added.
> + * workers/WorkerContext.idl:
> +
> 2009-08-05 Andras Becsi <becsi.andras at stud.u-szeged.hu>
>
> Reviewed by Simon Hausmann.
> Index: WebCore/GNUmakefile.am
> ===================================================================
> --- WebCore/GNUmakefile.am (revision 46804)
> +++ WebCore/GNUmakefile.am (working copy)
> @@ -237,6 +237,7 @@ IDL_BINDINGS += \
> WebCore/page/Coordinates.idl \
> WebCore/page/DOMSelection.idl \
> WebCore/page/DOMWindow.idl \
> + WebCore/page/EventSource.idl \
> WebCore/page/Geolocation.idl \
> WebCore/page/Geoposition.idl \
> WebCore/page/History.idl \
> @@ -339,6 +340,9 @@ webcore_sources += \
> WebCore/bindings/js/JSEventCustom.cpp \
> WebCore/bindings/js/JSEventListener.cpp \
> WebCore/bindings/js/JSEventListener.h \
> + WebCore/bindings/js/JSEventSourceConstructor.cpp \
> + WebCore/bindings/js/JSEventSourceConstructor.h \
> + WebCore/bindings/js/JSEventSourceCustom.cpp \
> WebCore/bindings/js/JSEventTarget.cpp \
> WebCore/bindings/js/JSEventTarget.h \
> WebCore/bindings/js/JSGeolocationCustom.cpp \
> @@ -1263,6 +1267,8 @@ webcore_sources += \
> WebCore/page/EditorClient.h \
> WebCore/page/EventHandler.cpp \
> WebCore/page/EventHandler.h \
> + WebCore/page/EventSource.cpp \
> + WebCore/page/EventSource.h \
> WebCore/page/FocusController.cpp \
> WebCore/page/FocusController.h \
> WebCore/page/FocusDirection.h \
> @@ -2114,6 +2120,20 @@ webcore_cppflags += -DENABLE_DATABASE=0
> endif # END ENABLE_DATABASE
>
> # ----
> +# HTML5 server-sent events
> +# ----
> +if !ENABLE_EVENTSOURCE
> +global_cppflags += -DENABLE_EVENTSOURCE=0
> +endif
> +
> +if ENABLE_EVENTSOURCE
> +FEATURE_DEFINES_JAVASCRIPT += ENABLE_EVENTSOURCE=1
> +
> +webcore_cppflags += \
> + -DENABLE_EVENTSOURCE=1
> +endif # ENABLE_EVENTSOURCE
> +
> +# ----
> # HTML5 client-side session and persistent storage
> # ----
> if ENABLE_DOM_STORAGE
> Index: WebCore/WebCore.pro
> ===================================================================
> --- WebCore/WebCore.pro (revision 46804)
> +++ WebCore/WebCore.pro (working copy)
> @@ -106,6 +106,7 @@ contains(DEFINES, ENABLE_SINGLE_THREADED
>
> !contains(DEFINES, ENABLE_JAVASCRIPT_DEBUGGER=.): DEFINES +=
ENABLE_JAVASCRIPT_DEBUGGER=1
> !contains(DEFINES, ENABLE_DATABASE=.): DEFINES += ENABLE_DATABASE=1
> +!contains(DEFINES, ENABLE_EVENTSOURCE=.): DEFINES += ENABLE_EVENTSOURCE=1
> !contains(DEFINES, ENABLE_OFFLINE_WEB_APPLICATIONS=.): DEFINES +=
ENABLE_OFFLINE_WEB_APPLICATIONS=1
> !contains(DEFINES, ENABLE_DOM_STORAGE=.): DEFINES += ENABLE_DOM_STORAGE=1
> !contains(DEFINES, ENABLE_ICONDATABASE=.): DEFINES += ENABLE_ICONDATABASE=1
> @@ -425,6 +426,7 @@ IDL_BINDINGS += \
> page/Coordinates.idl \
> page/DOMSelection.idl \
> page/DOMWindow.idl \
> + page/EventSource.idl \
> page/Geolocation.idl \
> page/Geoposition.idl \
> page/History.idl \
> @@ -486,6 +488,8 @@ SOURCES += \
> bindings/js/JSDOMWindowShell.cpp \
> bindings/js/JSElementCustom.cpp \
> bindings/js/JSEventCustom.cpp \
> + bindings/js/JSEventSourceConstructor.cpp \
> + bindings/js/JSEventSourceCustom.cpp \
> bindings/js/JSEventTarget.cpp \
> bindings/js/JSGeolocationCustom.cpp \
> bindings/js/JSHTMLAllCollection.cpp \
> @@ -916,6 +920,7 @@ SOURCES += \
> page/NavigatorBase.cpp \
> page/DragController.cpp \
> page/EventHandler.cpp \
> + page/EventSource.cpp \
> page/FocusController.cpp \
> page/Frame.cpp \
> page/FrameTree.cpp \
> @@ -1162,6 +1167,7 @@ HEADERS += \
> bindings/js/JSDOMWindowCustom.h \
> bindings/js/JSDOMWindowShell.h \
> bindings/js/JSEventListener.h \
> + bindings/js/JSEventSourceConstructor.h \
> bindings/js/JSEventTarget.h \
> bindings/js/JSHistoryCustom.h \
> bindings/js/JSHTMLAllCollection.h \
> @@ -1586,6 +1592,7 @@ HEADERS += \
> page/DOMWindow.h \
> page/DragController.h \
> page/EventHandler.h \
> + page/EventSource.h \
> page/FocusController.h \
> page/Frame.h \
> page/FrameTree.h \
> @@ -2264,6 +2271,10 @@ contains(DEFINES, ENABLE_DATAGRID=1) {
> FEATURE_DEFINES_JAVASCRIPT += ENABLE_DATAGRID=1
> }
>
> +contains(DEFINES, ENABLE_EVENTSOURCE=1) {
> + FEATURE_DEFINES_JAVASCRIPT += ENABLE_EVENTSOURCE=1
> +}
> +
> contains(DEFINES, ENABLE_SQLITE=1) {
> !system-sqlite:exists( $${SQLITE3SRCDIR}/sqlite3.c ) {
> # Build sqlite3 into WebCore from source
> Index: WebCore/bindings/js/JSDOMWindowCustom.cpp
> ===================================================================
> --- WebCore/bindings/js/JSDOMWindowCustom.cpp (revision 46804)
> +++ WebCore/bindings/js/JSDOMWindowCustom.cpp (working copy)
> @@ -38,6 +38,7 @@
> #include "JSDOMWindowShell.h"
> #include "JSEvent.h"
> #include "JSEventListener.h"
> +#include "JSEventSourceConstructor.h"
> #include "JSHTMLCollection.h"
> #include "JSHistory.h"
> #include "JSImageConstructor.h"
> @@ -424,6 +425,13 @@ JSValue JSDOMWindow::event(ExecState* ex
> return toJS(exec, event);
> }
>
> +#if ENABLE(EVENTSOURCE)
> +JSValue JSDOMWindow::eventSource(ExecState* exec) const
> +{
> + return getDOMConstructor<JSEventSourceConstructor>(exec, this);
> +}
> +#endif
> +
> JSValue JSDOMWindow::image(ExecState* exec) const
> {
> return getDOMConstructor<JSImageConstructor>(exec, this);
> Index: WebCore/bindings/js/JSEventSourceConstructor.cpp
> ===================================================================
> --- WebCore/bindings/js/JSEventSourceConstructor.cpp (revision 0)
> +++ WebCore/bindings/js/JSEventSourceConstructor.cpp (revision 0)
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (C) 2009 Ericsson AB
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + * 3. Neither the name of Ericsson nor the names of its contributors
> + * may be used to endorse or promote products derived from this
> + * software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +
> +#if ENABLE(EVENTSOURCE)
> +
> +#include "JSEventSourceConstructor.h"
> +
> +#include "EventSource.h"
> +#include "ExceptionCode.h"
> +#include "JSEventSource.h"
> +#include "ScriptExecutionContext.h"
> +
> +using namespace JSC;
> +
> +namespace WebCore {
> +
> +ASSERT_CLASS_FITS_IN_CELL(JSEventSourceConstructor);
> +
> +const ClassInfo JSEventSourceConstructor::s_info = {
"EventSourceContructor", 0, 0, 0 };
> +
> +JSEventSourceConstructor::JSEventSourceConstructor(ExecState* exec,
JSDOMGlobalObject* globalObject)
> + :
DOMObject(JSEventSourceConstructor::createStructure(exec->lexicalGlobalObject()
->objectPrototype()))
This should inherit from DOMConstructorObject
> + , m_globalObject(globalObject)
> +{
> + putDirect(exec->propertyNames().prototype,
JSEventSourcePrototype::self(exec, exec->lexicalGlobalObject()), None);
This should use the passed in global object instead of the lexical global
object.
> + putDirect(exec->propertyNames().length, jsNumber(exec, 1),
ReadOnly|DontDelete|DontEnum);
> +}
> +
> +ScriptExecutionContext* JSEventSourceConstructor::scriptExecutionContext()
const
> +{
> + return m_globalObject->scriptExecutionContext();
> +}
> +
> +static JSObject* constructEventSource(ExecState* exec, JSObject*
constructor, const ArgList& args)
> +{
> + if (args.size() == 0)
> + return throwError(exec, SyntaxError, "Not enough arguments");
> +
> + UString url = args.at(0).toString(exec);
> + if (exec->hadException())
> + return 0;
> +
> + ScriptExecutionContext* context =
static_cast<JSEventSourceConstructor*>(constructor)->scriptExecutionContext();
> + if (!context)
> + return throwError(exec, ReferenceError, "EventSource constructor
associated document is unavailable");
> +
> + ExceptionCode ec = 0;
> + RefPtr<EventSource> eventSource = EventSource::create(url, context, ec);
> + setDOMException(exec, ec);
> +
> + return asObject(toJS(exec, eventSource.release()));
> +}
> +
> +ConstructType JSEventSourceConstructor::getConstructData(ConstructData&
constructData)
> +{
> + constructData.native.function = constructEventSource;
> + return ConstructTypeHost;
> +}
> +
> +void JSEventSourceConstructor::mark()
> +{
> + DOMObject::mark();
> + if (!m_globalObject->marked())
> + m_globalObject->mark();
> +}
This method will not be necessary if you inherit from DOMConstructorObject.
> +#if ENABLE(EVENTSOURCE)
> + if (EventSource* eventSource = target->toEventSource())
> + return getCachedDOMObjectWrapper(exec->globalData(), eventSource);
This should use toJS(exec, globalObject, eventSource);
In addition, the new files need to be added to all the project files (xcode,
vcproj, etc.) and DerivedSources.make needs to be updated.
r-
More information about the webkit-reviews
mailing list