Diff
Modified: trunk/LayoutTests/ChangeLog (135477 => 135478)
--- trunk/LayoutTests/ChangeLog 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/LayoutTests/ChangeLog 2012-11-22 06:45:27 UTC (rev 135478)
@@ -1,3 +1,19 @@
+2012-11-21 Cosmin Truta <[email protected]>
+
+ Numeric identifiers of events should not be globally sequential
+ https://bugs.webkit.org/show_bug.cgi?id=102244
+
+ Reviewed by Alexey Proskuryakov.
+
+ Ensured that the results of setTimeout(), setInterval() and
+ navigator.geolocation.watchPosition() are strictly positive
+ and unique within their script execution context.
+
+ * fast/dom/Geolocation/watchPosition-unique-expected.txt: Added.
+ * fast/dom/Geolocation/watchPosition-unique.html: Added.
+ * fast/dom/Window/setTimeout-setInterval-unique-expected.txt: Added.
+ * fast/dom/Window/setTimeout-setInterval-unique.html: Added.
+
2012-11-21 Kent Tamura <[email protected]>
[Chromium] Correct svn:mime-type for platform/chromium*/**/*.png
Added: trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt (0 => 135478)
--- trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt 2012-11-22 06:45:27 UTC (rev 135478)
@@ -0,0 +1,15 @@
+Tests that navigator.geolocation.watchPosition returns unique results within its script execution context.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS watchID1 > 0 is true
+PASS watchID2 > 0 is true
+PASS watchID3 > 0 is true
+PASS watchID1 !== watchID2 is true
+PASS watchID1 !== watchID3 is true
+PASS watchID2 !== watchID3 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html (0 => 135478)
--- trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html 2012-11-22 06:45:27 UTC (rev 135478)
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+
+description("Tests that navigator.geolocation.watchPosition returns unique results within its script execution context.");
+
+if (window.testRunner)
+ testRunner.setMockGeolocationPosition(51.478, -0.166, 100.0);
+
+var watchID1 = navigator.geolocation.watchPosition(function() { });
+var watchID2 = navigator.geolocation.watchPosition(function() { });
+var watchID3 = navigator.geolocation.watchPosition(function() { });
+
+shouldBeTrue("watchID1 > 0");
+shouldBeTrue("watchID2 > 0");
+shouldBeTrue("watchID3 > 0");
+shouldBeTrue("watchID1 !== watchID2");
+shouldBeTrue("watchID1 !== watchID3");
+shouldBeTrue("watchID2 !== watchID3");
+
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt (0 => 135478)
--- trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt 2012-11-22 06:45:27 UTC (rev 135478)
@@ -0,0 +1,25 @@
+This checks that setTimeout() and setInterval() return unique results within their script execution context.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Checking the results of setTimeout:
+PASS tID1 > 0 is true
+PASS tID2 > 0 is true
+PASS tID3 > 0 is true
+PASS tID1 !== tID2 is true
+PASS tID1 !== tID3 is true
+PASS tID2 !== tID3 is true
+
+Checking the results of setInterval:
+PASS iID1 > 0 is true
+PASS iID2 > 0 is true
+PASS iID3 > 0 is true
+PASS iID1 !== iID2 is true
+PASS iID1 !== iID3 is true
+PASS iID2 !== iID3 is true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html (0 => 135478)
--- trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html 2012-11-22 06:45:27 UTC (rev 135478)
@@ -0,0 +1,54 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+
+description("This checks that setTimeout() and setInterval() return unique results within their script execution context.");
+
+function checkTimeoutIDs(tID1, tID2, tID3)
+{
+ debug("Checking the results of setTimeout:");
+ shouldBeTrue("tID1 > 0");
+ shouldBeTrue("tID2 > 0");
+ shouldBeTrue("tID3 > 0");
+ shouldBeTrue("tID1 !== tID2");
+ shouldBeTrue("tID1 !== tID3");
+ shouldBeTrue("tID2 !== tID3");
+ clearTimeout(tID1);
+ clearTimeout(tID2);
+ clearTimeout(tID3);
+ debug("");
+}
+
+function checkIntervalIDs(iID1, iID2, iID3)
+{
+ debug("Checking the results of setInterval:");
+ shouldBeTrue("iID1 > 0");
+ shouldBeTrue("iID2 > 0");
+ shouldBeTrue("iID3 > 0");
+ shouldBeTrue("iID1 !== iID2");
+ shouldBeTrue("iID1 !== iID3");
+ shouldBeTrue("iID2 !== iID3");
+ clearInterval(iID1);
+ clearInterval(iID2);
+ clearInterval(iID3);
+ debug("");
+}
+
+var tID1 = setTimeout('echo("timeout 1")', 0);
+var tID2 = setTimeout('echo("timeout 2")', 0);
+var iID1 = setInterval('echo("interval 1")', 0);
+var iID2 = setInterval('echo("interval 2")', 0);
+var tID3 = setTimeout('echo("timeout 3")', 0);
+var iID3 = setInterval('echo("interval 3")', 0);
+
+checkTimeoutIDs(tID1, tID2, tID3);
+checkIntervalIDs(iID1, iID2, iID3);
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (135477 => 135478)
--- trunk/Source/WebCore/ChangeLog 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/Source/WebCore/ChangeLog 2012-11-22 06:45:27 UTC (rev 135478)
@@ -1,3 +1,37 @@
+2012-11-21 Cosmin Truta <[email protected]>
+
+ Numeric identifiers of events should not be globally sequential
+ https://bugs.webkit.org/show_bug.cgi?id=102244
+
+ Reviewed by Alexey Proskuryakov.
+
+ The functions setTimeout, setInterval and navigator.geolocation.watchPosition
+ are currently returning values that are unique across all _javascript_ execution
+ contexts, due to their dependency on global variables.
+ Such a guarantee is unnecessarily strong. In this patch, we constrain uniqueness
+ to these functions' own script execution context only.
+
+ Tests: fast/dom/Geolocation/watchPosition-unique.html
+ fast/dom/Window/setTimeout-setInterval-unique.html
+
+ * Modules/geolocation/Geolocation.cpp:
+ (WebCore): Remove firstAvailableWatchId.
+ (WebCore::Geolocation::watchPosition): Get new watchID from script execution context.
+ (WebCore::Geolocation::clearWatch): Invalid watchID means less than or equal to 0.
+ * Modules/geolocation/Geolocation.h:
+ (Geolocation): Renamed the argument of Geolocation::clearWatch to WatchID.
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::ScriptExecutionContext::ScriptExecutionContext): Update initialization.
+ (WebCore::ScriptExecutionContext::newUniqueID): Add.
+ * dom/ScriptExecutionContext.h:
+ (ScriptExecutionContext): Add m_sequentialID.
+ (WebCore::ScriptExecutionContext::addTimeout): Inline.
+ (WebCore::ScriptExecutionContext::removeTimeout): Inline.
+ (WebCore::ScriptExecutionContext::findTimeout): Inline.
+ * page/DOMTimer.cpp:
+ (WebCore): Remove timeoutId.
+ (WebCore::DOMTimer::DOMTimer): Get new timeoutId from script execution context.
+
2012-11-21 Ryosuke Niwa <[email protected]>
CollectionType and DynamicNodeList::NodeListType should be merged
Modified: trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp (135477 => 135478)
--- trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp 2012-11-22 06:45:27 UTC (rev 135478)
@@ -50,8 +50,6 @@
static const char failedToStartServiceErrorMessage[] = "Failed to start Geolocation service";
static const char framelessDocumentErrorMessage[] = "Geolocation cannot be used in frameless documents";
-static const int firstAvailableWatchId = 1;
-
static PassRefPtr<Geoposition> createGeoposition(GeolocationPosition* position)
{
if (!position)
@@ -311,12 +309,9 @@
RefPtr<GeoNotifier> notifier = GeoNotifier::create(this, successCallback, errorCallback, options);
startRequest(notifier.get());
- static int nextAvailableWatchId = firstAvailableWatchId;
- // In case of overflow, make sure the ID remains positive, but reuse the ID values.
- if (nextAvailableWatchId < 1)
- nextAvailableWatchId = 1;
- m_watchers.set(nextAvailableWatchId, notifier.release());
- return nextAvailableWatchId++;
+ int watchID = m_scriptExecutionContext->newUniqueID();
+ m_watchers.set(watchID, notifier.release());
+ return watchID;
}
void Geolocation::startRequest(GeoNotifier *notifier)
@@ -419,14 +414,14 @@
return m_cachedPosition->timestamp() > currentTimeMillis - options->maximumAge();
}
-void Geolocation::clearWatch(int watchId)
+void Geolocation::clearWatch(int watchID)
{
- if (watchId < firstAvailableWatchId)
+ if (watchID <= 0)
return;
- if (GeoNotifier* notifier = m_watchers.find(watchId))
+ if (GeoNotifier* notifier = m_watchers.find(watchID))
m_pendingForPermissionNotifiers.remove(notifier);
- m_watchers.remove(watchId);
+ m_watchers.remove(watchID);
if (!hasListeners())
stopUpdating();
Modified: trunk/Source/WebCore/Modules/geolocation/Geolocation.h (135477 => 135478)
--- trunk/Source/WebCore/Modules/geolocation/Geolocation.h 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/Source/WebCore/Modules/geolocation/Geolocation.h 2012-11-22 06:45:27 UTC (rev 135478)
@@ -60,7 +60,7 @@
void getCurrentPosition(PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
int watchPosition(PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
- void clearWatch(int watchId);
+ void clearWatch(int watchID);
void setIsAllowed(bool);
bool isAllowed() const { return m_allowGeolocation == Yes; }
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (135477 => 135478)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2012-11-22 06:45:27 UTC (rev 135478)
@@ -99,6 +99,7 @@
ScriptExecutionContext::ScriptExecutionContext()
: m_iteratingActiveDOMObjects(false)
, m_inDestructor(false)
+ , m_sequentialID(0)
, m_inDispatchErrorEvent(false)
, m_activeDOMObjectsAreSuspended(false)
, m_reasonForSuspendingActiveDOMObjects(static_cast<ActiveDOMObject::ReasonForSuspension>(-1))
@@ -350,22 +351,16 @@
return errorEvent->defaultPrevented();
}
-void ScriptExecutionContext::addTimeout(int timeoutId, DOMTimer* timer)
+int ScriptExecutionContext::newUniqueID()
{
- ASSERT(!m_timeouts.contains(timeoutId));
- m_timeouts.set(timeoutId, timer);
+ ++m_sequentialID;
+ // FIXME: We prevent wraparound from becoming negative with a simple solution which
+ // ensures the result has a correct range, but without fully guaranteeing uniqueness.
+ if (m_sequentialID <= 0)
+ m_sequentialID = 1;
+ return m_sequentialID;
}
-void ScriptExecutionContext::removeTimeout(int timeoutId)
-{
- m_timeouts.remove(timeoutId);
-}
-
-DOMTimer* ScriptExecutionContext::findTimeout(int timeoutId)
-{
- return m_timeouts.get(timeoutId);
-}
-
#if ENABLE(BLOB)
FileThread* ScriptExecutionContext::fileThread()
{
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.h (135477 => 135478)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.h 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.h 2012-11-22 06:45:27 UTC (rev 135478)
@@ -140,10 +140,13 @@
virtual void postTask(PassOwnPtr<Task>) = 0; // Executes the task on context's thread asynchronously.
- void addTimeout(int timeoutId, DOMTimer*);
- void removeTimeout(int timeoutId);
- DOMTimer* findTimeout(int timeoutId);
+ // Creates a unique id for setTimeout, setInterval or navigator.geolocation.watchPosition.
+ int newUniqueID();
+ void addTimeout(int timeoutId, DOMTimer* timer) { ASSERT(!m_timeouts.contains(timeoutId)); m_timeouts.set(timeoutId, timer); }
+ void removeTimeout(int timeoutId) { m_timeouts.remove(timeoutId); }
+ DOMTimer* findTimeout(int timeoutId) { return m_timeouts.get(timeoutId); }
+
#if USE(JSC)
JSC::JSGlobalData* globalData();
#endif
@@ -197,30 +200,29 @@
void closeMessagePorts();
+ virtual void refScriptExecutionContext() = 0;
+ virtual void derefScriptExecutionContext() = 0;
+
HashSet<MessagePort*> m_messagePorts;
HashSet<ContextDestructionObserver*> m_destructionObservers;
HashMap<ActiveDOMObject*, void*> m_activeDOMObjects;
bool m_iteratingActiveDOMObjects;
bool m_inDestructor;
+ int m_sequentialID;
typedef HashMap<int, DOMTimer*> TimeoutMap;
TimeoutMap m_timeouts;
- virtual void refScriptExecutionContext() = 0;
- virtual void derefScriptExecutionContext() = 0;
-
bool m_inDispatchErrorEvent;
class PendingException;
OwnPtr<Vector<OwnPtr<PendingException> > > m_pendingExceptions;
-#if ENABLE(BLOB)
- OwnPtr<PublicURLManager> m_publicURLManager;
-#endif
bool m_activeDOMObjectsAreSuspended;
ActiveDOMObject::ReasonForSuspension m_reasonForSuspendingActiveDOMObjects;
bool m_activeDOMObjectsAreStopped;
#if ENABLE(BLOB)
+ OwnPtr<PublicURLManager> m_publicURLManager;
RefPtr<FileThread> m_fileThread;
#endif
};
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (135477 => 135478)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2012-11-22 06:18:35 UTC (rev 135477)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2012-11-22 06:45:27 UTC (rev 135478)
@@ -45,16 +45,6 @@
static int timerNestingLevel = 0;
-static int timeoutId()
-{
- static int lastUsedTimeoutId = 0;
- ++lastUsedTimeoutId;
- // Avoid wraparound going negative on us.
- if (lastUsedTimeoutId <= 0)
- lastUsedTimeoutId = 1;
- return lastUsedTimeoutId;
-}
-
static inline bool shouldForwardUserGesture(int interval, int nestingLevel)
{
return UserGestureIndicator::processingUserGesture()
@@ -64,15 +54,16 @@
DOMTimer::DOMTimer(ScriptExecutionContext* context, PassOwnPtr<ScheduledAction> action, int interval, bool singleShot)
: SuspendableTimer(context)
- , m_timeoutId(timeoutId())
+ , m_timeoutId(context->newUniqueID())
, m_nestingLevel(timerNestingLevel + 1)
, m_action(action)
, m_originalInterval(interval)
{
if (shouldForwardUserGesture(interval, m_nestingLevel))
m_userGestureToken = UserGestureIndicator::currentToken();
- scriptExecutionContext()->addTimeout(m_timeoutId, this);
+ context->addTimeout(m_timeoutId, this);
+
double intervalMilliseconds = intervalClampedToMinimum(interval, context->minimumTimerInterval());
if (singleShot)
startOneShot(intervalMilliseconds);
