[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198

msaboff at apple.com msaboff at apple.com
Mon Feb 21 00:11:35 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 53783a0b6f33f728beba0db5db03d91b7ec23e13
Author: msaboff at apple.com <msaboff at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Jan 28 20:21:07 2011 +0000

    2011-01-28  Michael Saboff  <msaboff at apple.com>
    
            Potentially Unsafe HashSet of RuntimeObject* in RootObject definition
            https://bugs.webkit.org/show_bug.cgi?id=53271
    
            Reapplying this this change.  No change from prior patch in
            JavaScriptCore.
    
            Added new isValid() methods to check if a contained object in
            a WeakGCMap is valid when using an unchecked iterator.
    
            * runtime/WeakGCMap.h:
            (JSC::WeakGCMap::isValid):
    2011-01-28  Michael Saboff  <msaboff at apple.com>
    
            Reviewed by Geoffrey Garen.
    
            Potentially Unsafe HashSet of RuntimeObject* in RootObject definition
            https://bugs.webkit.org/show_bug.cgi?id=53271
    
            Reapplying this patch with the change that the second ASSERT in
            RootObject::removeRuntimeObject was changed to use
            .uncheckedGet() instead of the failing .get().  The object in question
            could be in the process of being GC'ed.  The get() call will not return
            such an object while the uncheckedGet() call will return the (unsafe)
            object.  This is the behavior we want.
    
            Precautionary change.
            Changed RootObject to use WeakGCMap instead of HashSet.
            Found will looking for another issue, but can't produce a test case
            that is problematic.  THerefore there aren't any new tests.
    
            * bridge/runtime_root.cpp:
            (JSC::Bindings::RootObject::invalidate):
            (JSC::Bindings::RootObject::addRuntimeObject):
            (JSC::Bindings::RootObject::removeRuntimeObject):
            * bridge/runtime_root.h:
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76969 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index bde0be9..2ff2338 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,17 @@
+2011-01-28  Michael Saboff  <msaboff at apple.com>
+
+        Potentially Unsafe HashSet of RuntimeObject* in RootObject definition
+        https://bugs.webkit.org/show_bug.cgi?id=53271
+
+        Reapplying this this change.  No change from prior patch in
+        JavaScriptCore.
+
+        Added new isValid() methods to check if a contained object in
+        a WeakGCMap is valid when using an unchecked iterator.
+
+        * runtime/WeakGCMap.h:
+        (JSC::WeakGCMap::isValid):
+
 2011-01-27  Adam Roben  <aroben at apple.com>
 
         Extract code to convert a WTF absolute time to a Win32 wait interval into a separate
diff --git a/Source/JavaScriptCore/runtime/WeakGCMap.h b/Source/JavaScriptCore/runtime/WeakGCMap.h
index 316794f..c063dd2 100644
--- a/Source/JavaScriptCore/runtime/WeakGCMap.h
+++ b/Source/JavaScriptCore/runtime/WeakGCMap.h
@@ -69,6 +69,9 @@ public:
     const_iterator uncheckedBegin() const { return m_map.begin(); }
     const_iterator uncheckedEnd() const { return m_map.end(); }
 
+    bool isValid(iterator it) const { return Heap::isCellMarked(it->second); }
+    bool isValid(const_iterator it) const { return Heap::isCellMarked(it->second); }
+
 private:
     HashMap<KeyType, MappedType> m_map;
 };
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index f0bcb98..6a60150 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2011-01-28  Michael Saboff  <msaboff at apple.com>
+
+        Reviewed by Geoffrey Garen.
+
+        Potentially Unsafe HashSet of RuntimeObject* in RootObject definition
+        https://bugs.webkit.org/show_bug.cgi?id=53271
+
+        Reapplying this patch with the change that the second ASSERT in 
+        RootObject::removeRuntimeObject was changed to use
+        .uncheckedGet() instead of the failing .get().  The object in question
+        could be in the process of being GC'ed.  The get() call will not return
+        such an object while the uncheckedGet() call will return the (unsafe) 
+        object.  This is the behavior we want.
+
+        Precautionary change.
+        Changed RootObject to use WeakGCMap instead of HashSet.
+        Found will looking for another issue, but can't produce a test case
+        that is problematic.  THerefore there aren't any new tests.
+
+        * bridge/runtime_root.cpp:
+        (JSC::Bindings::RootObject::invalidate):
+        (JSC::Bindings::RootObject::addRuntimeObject):
+        (JSC::Bindings::RootObject::removeRuntimeObject):
+        * bridge/runtime_root.h:
+
 2011-01-28  Adam Roben  <aroben at apple.com>
 
         Notify CACFLayerTreeHost when the context is flushed
diff --git a/Source/WebCore/bridge/runtime_root.cpp b/Source/WebCore/bridge/runtime_root.cpp
index 796354f..3c8b313 100644
--- a/Source/WebCore/bridge/runtime_root.cpp
+++ b/Source/WebCore/bridge/runtime_root.cpp
@@ -101,13 +101,15 @@ void RootObject::invalidate()
         return;
 
     {
-        HashSet<RuntimeObject*>::iterator end = m_runtimeObjects.end();
-        for (HashSet<RuntimeObject*>::iterator it = m_runtimeObjects.begin(); it != end; ++it)
-            (*it)->invalidate();
-        
+        WeakGCMap<RuntimeObject*, RuntimeObject*>::iterator end = m_runtimeObjects.uncheckedEnd();
+        for (WeakGCMap<RuntimeObject*, RuntimeObject*>::iterator it = m_runtimeObjects.uncheckedBegin(); it != end; ++it) {
+            if (m_runtimeObjects.isValid(it))
+                it->second->invalidate();
+        }
+
         m_runtimeObjects.clear();
     }
-    
+
     m_isValid = false;
 
     m_nativeHandle = 0;
@@ -176,17 +178,17 @@ void RootObject::updateGlobalObject(JSGlobalObject* globalObject)
 void RootObject::addRuntimeObject(RuntimeObject* object)
 {
     ASSERT(m_isValid);
-    ASSERT(!m_runtimeObjects.contains(object));
-    
-    m_runtimeObjects.add(object);
-}        
-    
+    ASSERT(!m_runtimeObjects.get(object));
+
+    m_runtimeObjects.set(object, object);
+}
+
 void RootObject::removeRuntimeObject(RuntimeObject* object)
 {
     ASSERT(m_isValid);
-    ASSERT(m_runtimeObjects.contains(object));
-    
-    m_runtimeObjects.remove(object);
+    ASSERT(m_runtimeObjects.uncheckedGet(object));
+
+    m_runtimeObjects.take(object);
 }
 
 } } // namespace JSC::Bindings
diff --git a/Source/WebCore/bridge/runtime_root.h b/Source/WebCore/bridge/runtime_root.h
index babd7ad..8290e7c 100644
--- a/Source/WebCore/bridge/runtime_root.h
+++ b/Source/WebCore/bridge/runtime_root.h
@@ -31,8 +31,8 @@
 #endif
 #include <runtime/Protect.h>
 
+#include <runtime/WeakGCMap.h>
 #include <wtf/Forward.h>
-#include <wtf/HashSet.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
@@ -89,7 +89,7 @@ private:
     ProtectedPtr<JSGlobalObject> m_globalObject;
 
     ProtectCountSet m_protectCountSet;
-    HashSet<RuntimeObject*> m_runtimeObjects;    
+    WeakGCMap<RuntimeObject*, RuntimeObject*> m_runtimeObjects; // Really need a WeakGCSet, but this will do.
 
     HashSet<InvalidationCallback*> m_invalidationCallbacks;
 };

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list