From 7e4fdac07ffb59c438a17c2c88051064aaab16b5 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 4 Jan 2015 14:54:04 +0000 Subject: Revise handler stack implementation. The old implementation: - Wasn't actually checking whether handlers had been removed before calling them. - Could end up calling the same handler twice (if a handler was removed further down the stack, and the stack elements moved due the resulting splice. Solution: - Mark elements as removed and check. Set their ids to null. - Don't splice stack. Also, optimisation: - Removing the element at the top of the stack is still O(1). - In Modes, reverse handlers before removing (so, more likely to hit the optimisation above). For the record, the stable stack length at the moment seems to be about 10-12 elements. --- lib/handler_stack.coffee | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) (limited to 'lib/handler_stack.coffee') diff --git a/lib/handler_stack.coffee b/lib/handler_stack.coffee index f05413d3..c8e9a035 100644 --- a/lib/handler_stack.coffee +++ b/lib/handler_stack.coffee @@ -14,24 +14,24 @@ class HandlerStack # processing should take place. @stopBubblingAndFalse = new Object() - genId: -> @counter = ++@counter - # Adds a handler to the stack. Returns a unique ID for that handler that can be used to remove it later. push: (handler) -> - handler.id = @genId() @stack.push handler - handler.id + handler.id = ++@counter - # Called whenever we receive a key event. Each individual handler has the option to stop the event's - # propagation by returning a falsy value. + # Called whenever we receive a key or other event. Each individual handler has the option to stop the + # event's propagation by returning a falsy value, or stop bubbling by returning @stopBubblingAndFalse or + # @stopBubblingAndTrue. bubbleEvent: (type, event) -> + # extra is passed to each handler. This allows handlers to pass information down the stack. + extra = {} for i in [(@stack.length - 1)..0] by -1 handler = @stack[i] # We need to check for existence of handler because the last function call may have caused the release # of more than one handler. - if handler && handler[type] + if handler and handler.id and handler[type] @currentId = handler.id - passThrough = handler[type].call(@, event) + passThrough = handler[type].call @, event, extra if not passThrough DomUtils.suppressEvent(event) if @isChromeEvent event return false @@ -40,11 +40,17 @@ class HandlerStack true remove: (id = @currentId) -> - for i in [(@stack.length - 1)..0] by -1 - handler = @stack[i] - if handler.id == id - @stack.splice(i, 1) - break + if 0 < @stack.length and @stack[@stack.length-1].id == id + # A common case is to remove the handler at the top of the stack. And we can this very efficiently. + # Tests suggest that this case arises more than half of the time. + @stack.pop().id = null + else + # Otherwise, we'll build a new stack. This is better than splicing the existing stack since at can't + # interfere with any concurrent bubbleEvent. + @stack = @stack.filter (handler) -> + # Mark this handler as removed (for any active bubbleEvent call). + handler.id = null if handler.id == id + handler?.id? # The handler stack handles chrome events (which may need to be suppressed) and internal (fake) events. # This checks whether that the event at hand is a chrome event. -- cgit v1.2.3