SimpleEventBus : why such a non-predictive behavior implementation ?
123 views
Skip to first unread message
david....@gmail.com
Jul 25, 2011, 10:50:48 AM7/25/11
to Google Web Toolkit
Hi,
I'm facing a problem when using the simple implementation of the
EventBus.
// register widget B to be notified when WidgetA trigger an event
register handler of WidgetB on WidgetA.EventType
// dispatch (firingDepth is incremented)
WidgetA.fireEvent
// my widget is really called !!! that's good news
WidgetB.onWidgetAEvent
// on the WidgetA.EventType reception is decided to create a widget C
create widgetC
// this widget register himself to be notified on widgetB events ->
here the widget is registered in the deferredDeltas map (enqueueing)
register handler of widgetC on WidgetB.EventType
// widget B fired an event which is never trapped by the widget C
because the handler of Widget C is in the deferredDeltas AND
firingDepth is not 0
WidgetB.fireEvent
I'm doubtful because I was expected Widget C to receive event from
Widget B. A work around is to encapsulate the WidgetB.fireEvent into a
DeferredCommand but it's not really a solution.
An other solution is to reconsider the usage of firingDepth, I don't
really know why it is used (actually I know, but avoid concurrency
modification that way leads to the upper that strange behavior).
In simple implementation (thread safe) will be to get rid of the usage
of this firingDepth/deferredDeltas and that getHandlerList return a
copy of the handlers to go through and that's it:
We insert directly the handler when they request for registration and
we fire on the current map of handlers
Below is a possible implementation of what I think is a better
implementation (behaviorally speaking) :
package com.google.web.bindery.event.shared;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import com.google.web.bindery.event.shared.Event.Type;
public class MySimpleEventBus extends EventBus {
private final boolean isReverseOrder;
/**
* Map of event type to map of event source to list of their
handlers.
*/
private final Map<Event.Type<?>, Map<Object, List<?>>> map =
new HashMap<Event.Type<?>, Map<Object, List<?>>>();
public MySimpleEventBus() {
this(false);
}
/**
* Allows creation of an instance that fires its handlers in the
reverse of
* the order in which they were added, although filtered handlers
all fire
* before unfiltered handlers.
* <p>
*
* @deprecated This is a legacy feature, required by GWT's old
HandlerManager.
* Reverse order is not honored for handlers tied to a
specific
* event source (via {@link #addHandlerToSource}.
*/
@Deprecated
protected MySimpleEventBus(boolean fireInReverseOrder) {
isReverseOrder = fireInReverseOrder;
}
@Override
public <H> HandlerRegistration addHandler(Type<H> type, H handler)
{
return doAdd(type, null, handler);
}
@Override
public <H> HandlerRegistration addHandlerToSource(final
Event.Type<H> type, final Object source,
final H handler) {
if (source == null) {
throw new NullPointerException("Cannot add a handler with a
null source");
}
return doAdd(type, source, handler);
}
@Override
public void fireEvent(Event<?> event) {
doFire(event, null);
}
@Override
public void fireEventFromSource(Event<?> event, Object source) {
if (source == null) {
throw new NullPointerException("Cannot fire from a null
source");
}
doFire(event, source);
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected <H> void doRemove(Event.Type<H> type, Object source, H
handler) {
doRemoveNow(type, source, handler);
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected <H> H getHandler(Event.Type<H> type, int index) {
assert index < getHandlerCount(type) : "handlers for " +
type.getClass() + " have size: "
+ getHandlerCount(type) + " so do not have a handler at
index: " + index;
List<H> l = getHandlerList(type, null);
return l.get(index);
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected int getHandlerCount(Event.Type<?> eventKey) {
return getHandlerList(eventKey, null).size();
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected boolean isEventHandled(Event.Type<?> eventKey) {
return map.containsKey(eventKey);
}
private <H> HandlerRegistration doAdd(final Event.Type<H> type,
final Object source,
final H handler) {
if (type == null) {
throw new NullPointerException("Cannot add a handler with a
null type");
}
if (handler == null) {
throw new NullPointerException("Cannot add a null handler");
}
doAddNow(type, source, handler);
return new HandlerRegistration() {
public void removeHandler() {
doRemove(type, source, handler);
}
};
}
private <H> void doAddNow(Event.Type<H> type, Object source, H
handler) {
List<H> l = ensureHandlerList(type, source);
l.add(handler);
}
private <H> void doFire(Event<H> event, Object source) {
if (event == null) {
throw new NullPointerException("Cannot fire null event");
}
try {
if (source != null) {
event.setSource(source);
}
List<H> handlers = getDispatchList(event.getAssociatedType(),
source);
Set<Throwable> causes = null;
ListIterator<H> it =
isReverseOrder ? handlers.listIterator(handlers.size()) :
handlers.listIterator();
while (isReverseOrder ? it.hasPrevious() : it.hasNext()) {
H handler = isReverseOrder ? it.previous() : it.next();
try {
event.dispatch(handler);
} catch (Throwable e) {
if (causes == null) {
causes = new HashSet<Throwable>();
}
causes.add(e);
}
}
if (causes != null) {
throw new UmbrellaException(causes);
}
} finally {
// no more to do
}
}
private <H> void doRemoveNow(Event.Type<H> type, Object source, H
handler) {
List<H> l = getHandlerList(type, source);
boolean removed = l.remove(handler);
assert removed : "redundant remove call";
if (removed && l.isEmpty()) {
prune(type, source);
}
}
private <H> List<H> ensureHandlerList(Event.Type<H> type, Object
source) {
Map<Object, List<?>> sourceMap = map.get(type);
if (sourceMap == null) {
sourceMap = new HashMap<Object, List<?>>();
map.put(type, sourceMap);
}
// safe, we control the puts.
@SuppressWarnings("unchecked")
List<H> handlers = (List<H>) sourceMap.get(source);
if (handlers == null) {
handlers = new ArrayList<H>();
sourceMap.put(source, handlers);
}
return handlers;
}
/**
* Return a copy of handlers to dispatch event to
*/
private <H> List<H> getDispatchList(Event.Type<H> type, Object
source) {
List<H> directHandlers = getHandlerList(type, source);
if (source == null) {
return new ArrayList<H>(directHandlers);
}
List<H> globalHandlers = getHandlerList(type, null);
List<H> rtn = new ArrayList<H>(directHandlers);
rtn.addAll(globalHandlers);
return rtn;
}
private <H> List<H> getHandlerList(Event.Type<H> type, Object
source) {
Map<Object, List<?>> sourceMap = map.get(type);
if (sourceMap == null) {
return Collections.emptyList();
}
// safe, we control the puts.
@SuppressWarnings("unchecked")
List<H> handlers = (List<H>) sourceMap.get(source);
if (handlers == null) {
return Collections.emptyList();
}
return handlers;
}
private void prune(Event.Type<?> type, Object source) {
Map<Object, List<?>> sourceMap = map.get(type);
List<?> pruned = sourceMap.remove(source);
assert pruned != null : "Can't prune what wasn't there";
assert pruned.isEmpty() : "Pruned unempty list!";
if (sourceMap.isEmpty()) {
map.remove(type);
}
}
}
I'm facing a problem when using the simple implementation of the
EventBus.
// register widget B to be notified when WidgetA trigger an event
register handler of WidgetB on WidgetA.EventType
// dispatch (firingDepth is incremented)
WidgetA.fireEvent
// my widget is really called !!! that's good news
WidgetB.onWidgetAEvent
// on the WidgetA.EventType reception is decided to create a widget C
create widgetC
// this widget register himself to be notified on widgetB events ->
here the widget is registered in the deferredDeltas map (enqueueing)
register handler of widgetC on WidgetB.EventType
// widget B fired an event which is never trapped by the widget C
because the handler of Widget C is in the deferredDeltas AND
firingDepth is not 0
WidgetB.fireEvent
I'm doubtful because I was expected Widget C to receive event from
Widget B. A work around is to encapsulate the WidgetB.fireEvent into a
DeferredCommand but it's not really a solution.
An other solution is to reconsider the usage of firingDepth, I don't
really know why it is used (actually I know, but avoid concurrency
modification that way leads to the upper that strange behavior).
In simple implementation (thread safe) will be to get rid of the usage
of this firingDepth/deferredDeltas and that getHandlerList return a
copy of the handlers to go through and that's it:
We insert directly the handler when they request for registration and
we fire on the current map of handlers
Below is a possible implementation of what I think is a better
implementation (behaviorally speaking) :
package com.google.web.bindery.event.shared;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import com.google.web.bindery.event.shared.Event.Type;
public class MySimpleEventBus extends EventBus {
private final boolean isReverseOrder;
/**
* Map of event type to map of event source to list of their
handlers.
*/
private final Map<Event.Type<?>, Map<Object, List<?>>> map =
new HashMap<Event.Type<?>, Map<Object, List<?>>>();
public MySimpleEventBus() {
this(false);
}
/**
* Allows creation of an instance that fires its handlers in the
reverse of
* the order in which they were added, although filtered handlers
all fire
* before unfiltered handlers.
* <p>
*
* @deprecated This is a legacy feature, required by GWT's old
HandlerManager.
* Reverse order is not honored for handlers tied to a
specific
* event source (via {@link #addHandlerToSource}.
*/
@Deprecated
protected MySimpleEventBus(boolean fireInReverseOrder) {
isReverseOrder = fireInReverseOrder;
}
@Override
public <H> HandlerRegistration addHandler(Type<H> type, H handler)
{
return doAdd(type, null, handler);
}
@Override
public <H> HandlerRegistration addHandlerToSource(final
Event.Type<H> type, final Object source,
final H handler) {
if (source == null) {
throw new NullPointerException("Cannot add a handler with a
null source");
}
return doAdd(type, source, handler);
}
@Override
public void fireEvent(Event<?> event) {
doFire(event, null);
}
@Override
public void fireEventFromSource(Event<?> event, Object source) {
if (source == null) {
throw new NullPointerException("Cannot fire from a null
source");
}
doFire(event, source);
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected <H> void doRemove(Event.Type<H> type, Object source, H
handler) {
doRemoveNow(type, source, handler);
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected <H> H getHandler(Event.Type<H> type, int index) {
assert index < getHandlerCount(type) : "handlers for " +
type.getClass() + " have size: "
+ getHandlerCount(type) + " so do not have a handler at
index: " + index;
List<H> l = getHandlerList(type, null);
return l.get(index);
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected int getHandlerCount(Event.Type<?> eventKey) {
return getHandlerList(eventKey, null).size();
}
/**
* @deprecated required by legacy features in GWT's old
HandlerManager
*/
@Deprecated
protected boolean isEventHandled(Event.Type<?> eventKey) {
return map.containsKey(eventKey);
}
private <H> HandlerRegistration doAdd(final Event.Type<H> type,
final Object source,
final H handler) {
if (type == null) {
throw new NullPointerException("Cannot add a handler with a
null type");
}
if (handler == null) {
throw new NullPointerException("Cannot add a null handler");
}
doAddNow(type, source, handler);
return new HandlerRegistration() {
public void removeHandler() {
doRemove(type, source, handler);
}
};
}
private <H> void doAddNow(Event.Type<H> type, Object source, H
handler) {
List<H> l = ensureHandlerList(type, source);
l.add(handler);
}
private <H> void doFire(Event<H> event, Object source) {
if (event == null) {
throw new NullPointerException("Cannot fire null event");
}
try {
if (source != null) {
event.setSource(source);
}
List<H> handlers = getDispatchList(event.getAssociatedType(),
source);
Set<Throwable> causes = null;
ListIterator<H> it =
isReverseOrder ? handlers.listIterator(handlers.size()) :
handlers.listIterator();
while (isReverseOrder ? it.hasPrevious() : it.hasNext()) {
H handler = isReverseOrder ? it.previous() : it.next();
try {
event.dispatch(handler);
} catch (Throwable e) {
if (causes == null) {
causes = new HashSet<Throwable>();
}
causes.add(e);
}
}
if (causes != null) {
throw new UmbrellaException(causes);
}
} finally {
// no more to do
}
}
private <H> void doRemoveNow(Event.Type<H> type, Object source, H
handler) {
List<H> l = getHandlerList(type, source);
boolean removed = l.remove(handler);
assert removed : "redundant remove call";
if (removed && l.isEmpty()) {
prune(type, source);
}
}
private <H> List<H> ensureHandlerList(Event.Type<H> type, Object
source) {
Map<Object, List<?>> sourceMap = map.get(type);
if (sourceMap == null) {
sourceMap = new HashMap<Object, List<?>>();
map.put(type, sourceMap);
}
// safe, we control the puts.
@SuppressWarnings("unchecked")
List<H> handlers = (List<H>) sourceMap.get(source);
if (handlers == null) {
handlers = new ArrayList<H>();
sourceMap.put(source, handlers);
}
return handlers;
}
/**
* Return a copy of handlers to dispatch event to
*/
private <H> List<H> getDispatchList(Event.Type<H> type, Object
source) {
List<H> directHandlers = getHandlerList(type, source);
if (source == null) {
return new ArrayList<H>(directHandlers);
}
List<H> globalHandlers = getHandlerList(type, null);
List<H> rtn = new ArrayList<H>(directHandlers);
rtn.addAll(globalHandlers);
return rtn;
}
private <H> List<H> getHandlerList(Event.Type<H> type, Object
source) {
Map<Object, List<?>> sourceMap = map.get(type);
if (sourceMap == null) {
return Collections.emptyList();
}
// safe, we control the puts.
@SuppressWarnings("unchecked")
List<H> handlers = (List<H>) sourceMap.get(source);
if (handlers == null) {
return Collections.emptyList();
}
return handlers;
}
private void prune(Event.Type<?> type, Object source) {
Map<Object, List<?>> sourceMap = map.get(type);
List<?> pruned = sourceMap.remove(source);
assert pruned != null : "Can't prune what wasn't there";
assert pruned.isEmpty() : "Pruned unempty list!";
if (sourceMap.isEmpty()) {
map.remove(type);
}
}
}
david....@gmail.com
Jul 26, 2011, 4:11:30 PM7/26/11
to Google Web Toolkit
Gal Dolber
Jul 26, 2011, 4:43:03 PM7/26/11
to google-we...@googlegroups.com
I am, but your post is too long to read :)
--
Guit: Elegant, beautiful, modular and *production ready* gwt applications.
http://code.google.com/p/guit/
For what I understood, I had a similar problem, and I solved it encapsulating the "create widgetC" in a DeferredCommand (for your example).
Its really hard for me to understand the modification you purpose, maybe someone from google could see it.
But I think that if you found a better way of doing something you should send a patch and make us all happy
http://code.google.com/webtoolkit/makinggwtbetter.html (also you will get more help in gwt-code-reviews.appspot.com than here)
Regards!
// on the WidgetA.EventType reception is decided to create a widget C
create widgetC
create widgetC
--
You received this message because you are subscribed to the Google Groups "Google Web Toolkit" group.
To post to this group, send email to google-we...@googlegroups.com.
To unsubscribe from this group, send email to google-web-tool...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-web-toolkit?hl=en.
--
Guit: Elegant, beautiful, modular and *production ready* gwt applications.
http://code.google.com/p/guit/
david....@gmail.com
Jul 27, 2011, 9:58:03 AM7/27/11
to Google Web Toolkit
thanks, committed patch here : http://gwt-code-reviews.appspot.com/1500804/
On 26 juil, 22:43, Gal Dolber <gal.dol...@gmail.com> wrote:
> I am, but your post is too long to read :)
>
> For what I understood, I had a similar problem, and I solved it
> encapsulating the "create widgetC" in a DeferredCommand (for your example).
>
> Its really hard for me to understand the modification you purpose, maybe
> someone from google could see it.
> But I think that if you found a better way of doing something you should
> send a patch and make us all happyhttp://code.google.com/webtoolkit/makinggwtbetter.html(also you will get
> ...
>
> plus de détails »
On 26 juil, 22:43, Gal Dolber <gal.dol...@gmail.com> wrote:
> I am, but your post is too long to read :)
>
> For what I understood, I had a similar problem, and I solved it
> encapsulating the "create widgetC" in a DeferredCommand (for your example).
>
> Its really hard for me to understand the modification you purpose, maybe
> someone from google could see it.
> But I think that if you found a better way of doing something you should
> more help in gwt-code-reviews.appspot.com than here)
>
> Regards!
>
> // on the WidgetA.EventType reception is decided to create a widget C
> create widgetC
>
> On Tue, Jul 26, 2011 at 5:11 PM, david.herv...@gmail.com <
>
> Regards!
>
> // on the WidgetA.EventType reception is decided to create a widget C
> create widgetC
>
>
> plus de détails »
Sixin Zhang
Jul 27, 2011, 11:28:39 AM7/27/11
to Google Web Toolkit
i feel alike, all new registrations are delayed until the end of
firing (depth=0) .. so nothing will happen if firing those new events
before the end ..
depth behaves like a lock for concurrency, but not really necessary
for single-thread JavaScript ..
On Jul 27, 9:58 am, "david.herv...@gmail.com"
> ...
>
> read more »
firing (depth=0) .. so nothing will happen if firing those new events
before the end ..
depth behaves like a lock for concurrency, but not really necessary
for single-thread JavaScript ..
On Jul 27, 9:58 am, "david.herv...@gmail.com"
<david.herv...@gmail.com> wrote:
> thanks, committed patch here :http://gwt-code-reviews.appspot.com/1500804/
>
> On 26 juil, 22:43, Gal Dolber <gal.dol...@gmail.com> wrote:
>
>
>
>
>
>
>
> > I am, but your post is too long to read :)
>
> > For what I understood, I had a similar problem, and I solved it
> > encapsulating the "create widgetC" in a DeferredCommand (for your example).
>
> > Its really hard for me to understand the modification you purpose, maybe
> > someone from google could see it.
> > But I think that if you found a better way of doing something you should
> > send a patch and make us all happyhttp://code.google.com/webtoolkit/makinggwtbetter.html(alsoyou will get
> thanks, committed patch here :http://gwt-code-reviews.appspot.com/1500804/
>
> On 26 juil, 22:43, Gal Dolber <gal.dol...@gmail.com> wrote:
>
>
>
>
>
>
>
> > I am, but your post is too long to read :)
>
> > For what I understood, I had a similar problem, and I solved it
> > encapsulating the "create widgetC" in a DeferredCommand (for your example).
>
> > Its really hard for me to understand the modification you purpose, maybe
> > someone from google could see it.
> > But I think that if you found a better way of doing something you should
>
> read more »
david....@gmail.com
Jul 27, 2011, 11:48:03 AM7/27/11
to Google Web Toolkit
Even if multithreaded my implementation is (normally) done.
I really don't know why they have used such a mecanism...
There may be a use case I'm missing
On 27 juil, 17:28, Sixin Zhang <sixin.zh...@gmail.com> wrote:
> i feel alike, all new registrations are delayed until the end of
> firing (depth=0) .. so nothing will happen if firing those new events
> before the end ..
> depth behaves like a lock for concurrency, but not really necessary
> for single-thread JavaScript ..
>
> On Jul 27, 9:58 am, "david.herv...@gmail.com"<david.herv...@gmail.com> wrote:
> > thanks, committed patch here :http://gwt-code-reviews.appspot.com/1500804/
>
> > On 26 juil, 22:43, Gal Dolber <gal.dol...@gmail.com> wrote:
>
> > > I am, but your post is too long to read :)
>
> > > For what I understood, I had a similar problem, and I solved it
> > > encapsulating the "create widgetC" in a DeferredCommand (for your example).
>
> > > Its really hard for me to understand the modification you purpose, maybe
> > > someone from google could see it.
> > > But I think that if you found a better way of doing something you should
> > > send a patch and make us all happyhttp://code.google.com/webtoolkit/makinggwtbetter.html(alsoyouwill get
I really don't know why they have used such a mecanism...
There may be a use case I'm missing
On 27 juil, 17:28, Sixin Zhang <sixin.zh...@gmail.com> wrote:
> i feel alike, all new registrations are delayed until the end of
> firing (depth=0) .. so nothing will happen if firing those new events
> before the end ..
> depth behaves like a lock for concurrency, but not really necessary
> for single-thread JavaScript ..
>
> On Jul 27, 9:58 am, "david.herv...@gmail.com"<david.herv...@gmail.com> wrote:
> > thanks, committed patch here :http://gwt-code-reviews.appspot.com/1500804/
>
> > On 26 juil, 22:43, Gal Dolber <gal.dol...@gmail.com> wrote:
>
> > > I am, but your post is too long to read :)
>
> > > For what I understood, I had a similar problem, and I solved it
> > > encapsulating the "create widgetC" in a DeferredCommand (for your example).
>
> > > Its really hard for me to understand the modification you purpose, maybe
> > > someone from google could see it.
> > > But I think that if you found a better way of doing something you should
Hilco Wijbenga
Aug 5, 2011, 5:33:42 PM8/5/11
to google-we...@googlegroups.com
On 27 July 2011 08:48, david....@gmail.com <david....@gmail.com> wrote:
> Even if multithreaded my implementation is (normally) done.
> I really don't know why they have used such a mecanism...
> There may be a use case I'm missing
>
> On 27 juil, 17:28, Sixin Zhang <sixin.zh...@gmail.com> wrote:
>> i feel alike, all new registrations are delayed until the end of
>> firing (depth=0) .. so nothing will happen if firing those new events
>> before the end ..
>> depth behaves like a lock for concurrency, but not really necessary
>> for single-thread JavaScript ..
> Even if multithreaded my implementation is (normally) done.
> I really don't know why they have used such a mecanism...
> There may be a use case I'm missing
>
> On 27 juil, 17:28, Sixin Zhang <sixin.zh...@gmail.com> wrote:
>> i feel alike, all new registrations are delayed until the end of
>> firing (depth=0) .. so nothing will happen if firing those new events
>> before the end ..
>> depth behaves like a lock for concurrency, but not really necessary
>> for single-thread JavaScript ..
I'd like to add my "+1" ... I just finished a major debugging session.
Everything was working correctly but the event handlers were not
receiving the event they had signed up for. Because of firingDepth
(which was >0 because of a PlaceChange event). It's really insidious
because all code is correct and working in the right order and context
... but the event handlers don't respond to events.
I would think that either both handler registration *and* event firing
is deferred or neither. The current situation seems a strange
combination where event handler registration is deferred but event
firing is not.
I'm wondering now whether I should implement my own EventBus (using
David's patch e.g.) or if I am simply using events incorrectly?
Reply all
Reply to author
Forward
0 new messages