[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
ggaren at apple.com
ggaren at apple.com
Wed Apr 7 23:06:56 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit bc944558b38ae9bf57e61384b25cee81942b7f3c
Author: ggaren at apple.com <ggaren at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Oct 26 22:02:47 2009 +0000
WebCore: Fixed https://bugs.webkit.org/show_bug.cgi?id=30765
REGRESSION (r48701): Removing an event listener causes the one added just after it not to fire
Patch by Geoffrey Garen <ggaren at apple.com> on 2009-10-26
Reviewed by Dimitri Glazkov.
and related bugs.
If the event listener being removed is prior to the current firing event
iterator, we need to decrement the current firing event iterator in
addition to the endpoint. (Otherwise, shrinking the event listener vector
by one implicity moves the current firing event iterator forward by one.
It's like relativity, only without the planets.)
Also took the opportunity to change some pointers to references, since
they can't be null.
* dom/EventTarget.cpp:
(WebCore::EventTarget::removeEventListener):
(WebCore::EventTarget::removeAllEventListeners): Update iterator in addition
to end, if need be.
(WebCore::EventTarget::fireEventListeners): Updated for interface changes.
Added a comment to explain a behavior that was implicit enough to be
confusing.
* dom/EventTarget.h:
(WebCore::FiringEventIterator::FiringEventIterator):
(WebCore::EventTarget::isFiringEventListeners): Updated for interface changes.
LayoutTests: Test for https://bugs.webkit.org/show_bug.cgi?id=30765
REGRESSION (r48701): Removing an event listener causes one added after it to not fire
Patch by Dimitri Glazkov <dglazkov at chromium.org> on 2009-10-26
Reviewed by Geoffrey Garen.
* fast/events/event-listener-list-mutation-expected.txt: Added.
* fast/events/event-listener-list-mutation.html: Added.
* fast/events/script-tests/event-listener-list-mutation.js: Added.
(TestMutation.listeners):
(TestMutation.mutateList):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50100 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1d468a7..e0e2190 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2009-10-26 Dimitri Glazkov <dglazkov at chromium.org>
+
+ Reviewed by Geoffrey Garen.
+
+ Test for https://bugs.webkit.org/show_bug.cgi?id=30765
+ REGRESSION (r48701): Removing an event listener causes one added after it to not fire
+
+ * fast/events/event-listener-list-mutation-expected.txt: Added.
+ * fast/events/event-listener-list-mutation.html: Added.
+ * fast/events/script-tests/event-listener-list-mutation.js: Added.
+ (TestMutation.listeners):
+ (TestMutation.mutateList):
+
2009-10-26 Xan Lopez <xlopez at igalia.com>
Skip test fast/loader/opaque-base-url.html with a reference to the
diff --git a/LayoutTests/fast/events/event-listener-list-mutation-expected.txt b/LayoutTests/fast/events/event-listener-list-mutation-expected.txt
new file mode 100644
index 0000000..a2e4ed3
--- /dev/null
+++ b/LayoutTests/fast/events/event-listener-list-mutation-expected.txt
@@ -0,0 +1,19 @@
+Tests that event list mutation preserves the order of event firing.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+self-removal:
+PASS listener 0 removing listener 0
+PASS listener 1 removing listener 1
+PASS listener 2 removing listener 2
+successor removal:
+PASS listener 0 removing listener 1
+PASS listener 0 removing listener 2
+predecessor removal:
+PASS listener 2 removing listener 0
+PASS listener 2 removing listener 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/event-listener-list-mutation.html b/LayoutTests/fast/events/event-listener-list-mutation.html
new file mode 100644
index 0000000..895bf9c
--- /dev/null
+++ b/LayoutTests/fast/events/event-listener-list-mutation.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/event-listener-list-mutation.js"></script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/events/script-tests/event-listener-list-mutation.js b/LayoutTests/fast/events/script-tests/event-listener-list-mutation.js
new file mode 100644
index 0000000..c7b5d5b
--- /dev/null
+++ b/LayoutTests/fast/events/script-tests/event-listener-list-mutation.js
@@ -0,0 +1,50 @@
+function TestMutation(remover, removee, result)
+{
+ var report = [];
+
+ var node = document.createElement("button");
+ var eventType = "click";
+ document.body.appendChild(node);
+
+ var listeners = [
+ function() { mutateList(0); },
+ function() { mutateList(1); },
+ function() { mutateList(2); }
+ ];
+
+ listeners.forEach(function(listener) { node.addEventListener(eventType, listener, false); });
+
+ node.click();
+ document.body.removeChild(node);
+
+ var log = "listener " + remover + " removing listener " + removee;
+
+ if (report.join(" ") == result)
+ testPassed(log);
+ else
+ testFailed(log);
+
+ function mutateList(me)
+ {
+ if (remover == me)
+ node.removeEventListener(eventType, listeners[removee], false);
+ report.push(me);
+ }
+}
+
+description("Tests that event list mutation preserves the order of event firing.");
+
+debug("self-removal:");
+TestMutation(0, 0, "0 1 2");
+TestMutation(1, 1, "0 1 2");
+TestMutation(2, 2, "0 1 2");
+
+debug("successor removal:");
+TestMutation(0, 1, "0 2");
+TestMutation(0, 2, "0 1");
+
+debug("predecessor removal:");
+TestMutation(2, 0, "0 1 2");
+TestMutation(2, 1, "0 1 2");
+
+var successfullyParsed = true;
\ No newline at end of file
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 00b0eb1..b4ac79f 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2009-10-26 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by Dimitri Glazkov.
+
+ Fixed https://bugs.webkit.org/show_bug.cgi?id=30765
+ REGRESSION (r48701): Removing an event listener causes the one added just after it not to fire
+
+ and related bugs.
+
+ If the event listener being removed is prior to the current firing event
+ iterator, we need to decrement the current firing event iterator in
+ addition to the endpoint. (Otherwise, shrinking the event listener vector
+ by one implicity moves the current firing event iterator forward by one.
+ It's like relativity, only without the planets.)
+
+ Also took the opportunity to change some pointers to references, since
+ they can't be null.
+
+ * dom/EventTarget.cpp:
+ (WebCore::EventTarget::removeEventListener):
+ (WebCore::EventTarget::removeAllEventListeners): Update iterator in addition
+ to end, if need be.
+ (WebCore::EventTarget::fireEventListeners): Updated for interface changes.
+ Added a comment to explain a behavior that was implicit enough to be
+ confusing.
+
+ * dom/EventTarget.h:
+ (WebCore::FiringEventIterator::FiringEventIterator):
+ (WebCore::EventTarget::isFiringEventListeners): Updated for interface changes.
+
2009-10-26 Brian Weinstein <bweinstein at apple.com>
Reviewed by Timothy Hatcher.
diff --git a/WebCore/dom/EventTarget.cpp b/WebCore/dom/EventTarget.cpp
index 6259a0b..694e78a 100644
--- a/WebCore/dom/EventTarget.cpp
+++ b/WebCore/dom/EventTarget.cpp
@@ -192,9 +192,16 @@ bool EventTarget::removeEventListener(const AtomicString& eventType, EventListen
// Notify firing events planning to invoke the listener at 'index' that
// they have one less listener to invoke.
- for (size_t i = 0; i < d->firingEventEndIterators.size(); ++i) {
- if (eventType == *d->firingEventEndIterators[i].eventType && index < *d->firingEventEndIterators[i].value)
- --*d->firingEventEndIterators[i].value;
+ for (size_t i = 0; i < d->firingEventIterators.size(); ++i) {
+ if (eventType != d->firingEventIterators[i].eventType)
+ continue;
+
+ if (index >= d->firingEventIterators[i].end)
+ continue;
+
+ --d->firingEventIterators[i].end;
+ if (index <= d->firingEventIterators[i].iterator)
+ --d->firingEventIterators[i].iterator;
}
return true;
@@ -263,9 +270,15 @@ bool EventTarget::fireEventListeners(Event* event)
RefPtr<EventTarget> protect = this;
+ // Fire all listeners registered for this event. Don't fire listeners removed
+ // during event dispatch. Also, don't fire event listeners added during event
+ // dispatch. Conveniently, all new event listeners will be added after 'end',
+ // so iterating to 'end' naturally excludes new event listeners.
+
+ size_t i = 0;
size_t end = entry.size();
- d->firingEventEndIterators.append(FiringEventEndIterator(&event->type(), &end));
- for (size_t i = 0; i < end; ++i) {
+ d->firingEventIterators.append(FiringEventIterator(event->type(), i, end));
+ for ( ; i < end; ++i) {
RegisteredEventListener& registeredListener = entry[i];
if (event->eventPhase() == Event::CAPTURING_PHASE && !registeredListener.useCapture)
continue;
@@ -275,7 +288,7 @@ bool EventTarget::fireEventListeners(Event* event)
// event listeners, even though that violates some versions of the DOM spec.
registeredListener.listener->handleEvent(scriptExecutionContext(), event);
}
- d->firingEventEndIterators.removeLast();
+ d->firingEventIterators.removeLast();
return !event->defaultPrevented();
}
@@ -302,8 +315,10 @@ void EventTarget::removeAllEventListeners()
// Notify firing events planning to invoke the listener at 'index' that
// they have one less listener to invoke.
- for (size_t i = 0; i < d->firingEventEndIterators.size(); ++i)
- *d->firingEventEndIterators[i].value = 0;
+ for (size_t i = 0; i < d->firingEventIterators.size(); ++i) {
+ d->firingEventIterators[i].iterator = 0;
+ d->firingEventIterators[i].end = 0;
+ }
}
} // namespace WebCore
diff --git a/WebCore/dom/EventTarget.h b/WebCore/dom/EventTarget.h
index 2d612e1..9a1975c 100644
--- a/WebCore/dom/EventTarget.h
+++ b/WebCore/dom/EventTarget.h
@@ -61,24 +61,26 @@ namespace WebCore {
typedef int ExceptionCode;
- struct FiringEventEndIterator {
- FiringEventEndIterator(const AtomicString* eventType, size_t* value)
+ struct FiringEventIterator {
+ FiringEventIterator(const AtomicString& eventType, size_t& iterator, size_t& end)
: eventType(eventType)
- , value(value)
+ , iterator(iterator)
+ , end(end)
{
}
-
- const AtomicString* eventType;
- size_t* value;
+
+ const AtomicString& eventType;
+ size_t& iterator;
+ size_t& end;
};
- typedef Vector<FiringEventEndIterator, 1> FiringEventEndIteratorVector;
+ typedef Vector<FiringEventIterator, 1> FiringEventIteratorVector;
typedef Vector<RegisteredEventListener, 1> EventListenerVector;
typedef HashMap<AtomicString, EventListenerVector> EventListenerMap;
struct EventTargetData {
EventListenerMap eventListenerMap;
- FiringEventEndIteratorVector firingEventEndIterators;
+ FiringEventIteratorVector firingEventIterators;
};
class EventTarget {
@@ -209,7 +211,7 @@ namespace WebCore {
EventTargetData* d = eventTargetData();
if (!d)
return false;
- return d->firingEventEndIterators.size() != 0;
+ return d->firingEventIterators.size() != 0;
}
inline bool EventTarget::hasEventListeners()
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list