Title: [145626] branches/safari-536.30-branch

Diff

Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (145625 => 145626)


--- branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-03-13 00:14:07 UTC (rev 145626)
@@ -1,5 +1,25 @@
 2013-03-12  Lucas Forschler  <[email protected]>
 
+        Merge r135478
+
+    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.
+
+2013-03-12  Lucas Forschler  <[email protected]>
+
         Merge r135454
 
     2012-11-21  Daniel Bates  <[email protected]>

Copied: branches/safari-536.30-branch/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt (from rev 135478, trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt) (0 => 145626)


--- branches/safari-536.30-branch/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/fast/dom/Geolocation/watchPosition-unique-expected.txt	2013-03-13 00:14:07 UTC (rev 145626)
@@ -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
+

Copied: branches/safari-536.30-branch/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html (from rev 135478, trunk/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html) (0 => 145626)


--- branches/safari-536.30-branch/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/fast/dom/Geolocation/watchPosition-unique.html	2013-03-13 00:14:07 UTC (rev 145626)
@@ -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>

Copied: branches/safari-536.30-branch/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt (from rev 135478, trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt) (0 => 145626)


--- branches/safari-536.30-branch/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique-expected.txt	2013-03-13 00:14:07 UTC (rev 145626)
@@ -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
+

Copied: branches/safari-536.30-branch/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html (from rev 135478, trunk/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html) (0 => 145626)


--- branches/safari-536.30-branch/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/fast/dom/Window/setTimeout-setInterval-unique.html	2013-03-13 00:14:07 UTC (rev 145626)
@@ -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: branches/safari-536.30-branch/Source/WebCore/ChangeLog (145625 => 145626)


--- branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-03-13 00:14:07 UTC (rev 145626)
@@ -1,5 +1,43 @@
 2013-03-12  Lucas Forschler  <[email protected]>
 
+        Merge r135478
+
+    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.
+
+2013-03-12  Lucas Forschler  <[email protected]>
+
         Merge r135303
 
     2012-11-20  Abhishek Arya  <[email protected]>

Modified: branches/safari-536.30-branch/Source/WebCore/Modules/geolocation/Geolocation.cpp (145625 => 145626)


--- branches/safari-536.30-branch/Source/WebCore/Modules/geolocation/Geolocation.cpp	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/Source/WebCore/Modules/geolocation/Geolocation.cpp	2013-03-13 00:14:07 UTC (rev 145626)
@@ -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 = startRequest(successCallback, errorCallback, options);
     ASSERT(notifier);
 
-    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;
 }
 
 PassRefPtr<Geolocation::GeoNotifier> Geolocation::startRequest(PassRefPtr<PositionCallback> successCallback, PassRefPtr<PositionErrorCallback> errorCallback, PassRefPtr<PositionOptions> options)
@@ -423,14 +418,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: branches/safari-536.30-branch/Source/WebCore/Modules/geolocation/Geolocation.h (145625 => 145626)


--- branches/safari-536.30-branch/Source/WebCore/Modules/geolocation/Geolocation.h	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/Source/WebCore/Modules/geolocation/Geolocation.h	2013-03-13 00:14:07 UTC (rev 145626)
@@ -59,7 +59,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: branches/safari-536.30-branch/Source/WebCore/dom/ScriptExecutionContext.cpp (145625 => 145626)


--- branches/safari-536.30-branch/Source/WebCore/dom/ScriptExecutionContext.cpp	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/Source/WebCore/dom/ScriptExecutionContext.cpp	2013-03-13 00:14:07 UTC (rev 145626)
@@ -90,6 +90,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))
@@ -341,22 +342,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: branches/safari-536.30-branch/Source/WebCore/dom/ScriptExecutionContext.h (145625 => 145626)


--- branches/safari-536.30-branch/Source/WebCore/dom/ScriptExecutionContext.h	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/Source/WebCore/dom/ScriptExecutionContext.h	2013-03-13 00:14:07 UTC (rev 145626)
@@ -139,10 +139,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
@@ -191,30 +194,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: branches/safari-536.30-branch/Source/WebCore/page/DOMTimer.cpp (145625 => 145626)


--- branches/safari-536.30-branch/Source/WebCore/page/DOMTimer.cpp	2013-03-13 00:09:55 UTC (rev 145625)
+++ branches/safari-536.30-branch/Source/WebCore/page/DOMTimer.cpp	2013-03-13 00:14:07 UTC (rev 145626)
@@ -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,14 +54,15 @@
 
 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)
     , m_shouldForwardUserGesture(shouldForwardUserGesture(interval, m_nestingLevel))
 {
-    scriptExecutionContext()->addTimeout(m_timeoutId, this);
 
+    context->addTimeout(m_timeoutId, this);
+
     double intervalMilliseconds = intervalClampedToMinimum(interval, context->minimumTimerInterval());
     if (singleShot)
         startOneShot(intervalMilliseconds);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to