- Revision
- 115534
- Author
- [email protected]
- Date
- 2012-04-27 20:43:29 -0700 (Fri, 27 Apr 2012)
Log Message
Only allow non-null pointers in the WeakSet
https://bugs.webkit.org/show_bug.cgi?id=85119
Reviewed by Darin Adler.
../_javascript_Core:
This is a step toward more efficient finalization.
No clients put non-pointers (JSValues) into Weak<T> and PassWeak<T>.
Some clients put null pointers into Weak<T> and PassWeak<T>, but this is
more efficient and straight-forward to model with a null in the Weak<T>
or PassWeak<T> instead of allocating a WeakImpl just to hold null.
* heap/PassWeak.h:
(JSC): Removed the Unknown (JSValue) type of weak pointer because it's
unused now.
(PassWeak): Don't provide a default initializer for our JSCell* argument.
This feature was only used in one place, and it was a bug.
(JSC::::get): Don't check for a null stored inside our WeakImpl: that's
not allowed anymore.
(JSC::PassWeak::PassWeak): Handle null as a null WeakImpl instead of
allocating a WeakImpl and storing null into it.
* heap/Weak.h:
(Weak):
(JSC::::Weak): Same changes as in PassWeak<T>.
* heap/WeakBlock.cpp:
(JSC::WeakBlock::visitLiveWeakImpls):
(JSC::WeakBlock::visitDeadWeakImpls): Only non-null cells are valid in
the WeakSet now, so no need to check for non-cells and null cell pointers.
* heap/WeakImpl.h:
(JSC::WeakImpl::WeakImpl): Only non-null cells are valid in the WeakSet
now, so ASSERT that.
../WebCore:
* bridge/jsc/BridgeJSC.cpp:
(JSC::Bindings::Instance::Instance): Don't allocate a WeakImpl just to
store null. This was needless, and is now a compile error. Instead,
rely on the default constructor, which will produce a cheap null.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (115533 => 115534)
--- trunk/Source/_javascript_Core/ChangeLog 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-04-28 03:43:29 UTC (rev 115534)
@@ -1,3 +1,44 @@
+2012-04-27 Geoffrey Garen <[email protected]>
+
+ Only allow non-null pointers in the WeakSet
+ https://bugs.webkit.org/show_bug.cgi?id=85119
+
+ Reviewed by Darin Adler.
+
+ This is a step toward more efficient finalization.
+
+ No clients put non-pointers (JSValues) into Weak<T> and PassWeak<T>.
+
+ Some clients put null pointers into Weak<T> and PassWeak<T>, but this is
+ more efficient and straight-forward to model with a null in the Weak<T>
+ or PassWeak<T> instead of allocating a WeakImpl just to hold null.
+
+ * heap/PassWeak.h:
+ (JSC): Removed the Unknown (JSValue) type of weak pointer because it's
+ unused now.
+
+ (PassWeak): Don't provide a default initializer for our JSCell* argument.
+ This feature was only used in one place, and it was a bug.
+
+ (JSC::::get): Don't check for a null stored inside our WeakImpl: that's
+ not allowed anymore.
+
+ (JSC::PassWeak::PassWeak): Handle null as a null WeakImpl instead of
+ allocating a WeakImpl and storing null into it.
+
+ * heap/Weak.h:
+ (Weak):
+ (JSC::::Weak): Same changes as in PassWeak<T>.
+
+ * heap/WeakBlock.cpp:
+ (JSC::WeakBlock::visitLiveWeakImpls):
+ (JSC::WeakBlock::visitDeadWeakImpls): Only non-null cells are valid in
+ the WeakSet now, so no need to check for non-cells and null cell pointers.
+
+ * heap/WeakImpl.h:
+ (JSC::WeakImpl::WeakImpl): Only non-null cells are valid in the WeakSet
+ now, so ASSERT that.
+
2012-04-27 Gavin Barraclough <[email protected]>
<rdar://problem/7909395> Math in _javascript_ is inaccurate on iOS
Modified: trunk/Source/_javascript_Core/heap/PassWeak.h (115533 => 115534)
--- trunk/Source/_javascript_Core/heap/PassWeak.h 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/_javascript_Core/heap/PassWeak.h 2012-04-28 03:43:29 UTC (rev 115534)
@@ -50,15 +50,6 @@
#endif
};
-template<typename Base> class WeakImplAccessor<Base, Unknown> {
-public:
- typedef JSValue GetType;
-
- const JSValue* operator->() const;
- const JSValue& operator*() const;
- GetType get() const;
-};
-
template<typename T> class PassWeak : public WeakImplAccessor<PassWeak<T>, T> {
public:
friend class WeakImplAccessor<PassWeak<T>, T>;
@@ -66,7 +57,7 @@
PassWeak();
PassWeak(std::nullptr_t);
- PassWeak(JSGlobalData&, GetType = GetType(), WeakHandleOwner* = 0, void* context = 0);
+ PassWeak(JSGlobalData&, GetType, WeakHandleOwner* = 0, void* context = 0);
// It somewhat breaks the type system to allow transfer of ownership out of
// a const PassWeak. However, it makes it much easier to work with PassWeak
@@ -105,7 +96,7 @@
template<typename Base, typename T> inline typename WeakImplAccessor<Base, T>::GetType WeakImplAccessor<Base, T>::get() const
{
- if (!static_cast<const Base*>(this)->m_impl || static_cast<const Base*>(this)->m_impl->state() != WeakImpl::Live || !static_cast<const Base*>(this)->m_impl->jsValue())
+ if (!static_cast<const Base*>(this)->m_impl || static_cast<const Base*>(this)->m_impl->state() != WeakImpl::Live)
return GetType();
return jsCast<T*>(static_cast<const Base*>(this)->m_impl->jsValue().asCell());
}
@@ -117,25 +108,6 @@
}
#endif
-template<typename Base> inline const JSValue* WeakImplAccessor<Base, Unknown>::operator->() const
-{
- ASSERT(static_cast<const Base*>(this)->m_impl && static_cast<const Base*>(this)->m_impl->state() == WeakImpl::Live);
- return &static_cast<const Base*>(this)->m_impl->jsValue();
-}
-
-template<typename Base> inline const JSValue& WeakImplAccessor<Base, Unknown>::operator*() const
-{
- ASSERT(static_cast<const Base*>(this)->m_impl && static_cast<const Base*>(this)->m_impl->state() == WeakImpl::Live);
- return static_cast<const Base*>(this)->m_impl->jsValue();
-}
-
-template<typename Base> inline typename WeakImplAccessor<Base, Unknown>::GetType WeakImplAccessor<Base, Unknown>::get() const
-{
- if (!static_cast<const Base*>(this)->m_impl || static_cast<const Base*>(this)->m_impl->state() != WeakImpl::Live)
- return GetType();
- return static_cast<const Base*>(this)->m_impl->jsValue();
-}
-
template<typename T> inline PassWeak<T>::PassWeak()
: m_impl(0)
{
@@ -147,7 +119,7 @@
}
template<typename T> inline PassWeak<T>::PassWeak(JSGlobalData& globalData, typename PassWeak<T>::GetType getType, WeakHandleOwner* weakOwner, void* context)
- : m_impl(globalData.heap.weakSet()->allocate(getType, weakOwner, context))
+ : m_impl(getType ? globalData.heap.weakSet()->allocate(getType, weakOwner, context) : 0)
{
}
Modified: trunk/Source/_javascript_Core/heap/Weak.h (115533 => 115534)
--- trunk/Source/_javascript_Core/heap/Weak.h 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/_javascript_Core/heap/Weak.h 2012-04-28 03:43:29 UTC (rev 115534)
@@ -40,7 +40,7 @@
Weak();
Weak(std::nullptr_t);
- Weak(JSGlobalData&, GetType = GetType(), WeakHandleOwner* = 0, void* context = 0);
+ Weak(JSGlobalData&, GetType, WeakHandleOwner* = 0, void* context = 0);
enum HashTableDeletedValueTag { HashTableDeletedValue };
bool isHashTableDeletedValue() const;
@@ -79,7 +79,7 @@
}
template<typename T> inline Weak<T>::Weak(JSGlobalData& globalData, typename Weak<T>::GetType getType, WeakHandleOwner* weakOwner, void* context)
- : m_impl(globalData.heap.weakSet()->allocate(getType, weakOwner, context))
+ : m_impl(getType ? globalData.heap.weakSet()->allocate(getType, weakOwner, context) : 0)
{
}
Modified: trunk/Source/_javascript_Core/heap/WeakBlock.cpp (115533 => 115534)
--- trunk/Source/_javascript_Core/heap/WeakBlock.cpp 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/_javascript_Core/heap/WeakBlock.cpp 2012-04-28 03:43:29 UTC (rev 115534)
@@ -104,13 +104,9 @@
continue;
const JSValue& jsValue = weakImpl->jsValue();
- if (!jsValue || !jsValue.isCell())
+ if (Heap::isMarked(jsValue.asCell()))
continue;
- JSCell* jsCell = jsValue.asCell();
- if (Heap::isMarked(jsCell))
- continue;
-
WeakHandleOwner* weakHandleOwner = weakImpl->weakHandleOwner();
if (!weakHandleOwner)
continue;
@@ -133,14 +129,9 @@
if (weakImpl->state() > WeakImpl::Dead)
continue;
- const JSValue& jsValue = weakImpl->jsValue();
- if (!jsValue || !jsValue.isCell())
+ if (Heap::isMarked(weakImpl->jsValue().asCell()))
continue;
- JSCell* jsCell = jsValue.asCell();
- if (Heap::isMarked(jsCell))
- continue;
-
weakImpl->setState(WeakImpl::Dead);
}
}
Modified: trunk/Source/_javascript_Core/heap/WeakImpl.h (115533 => 115534)
--- trunk/Source/_javascript_Core/heap/WeakImpl.h 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/_javascript_Core/heap/WeakImpl.h 2012-04-28 03:43:29 UTC (rev 115534)
@@ -76,6 +76,7 @@
, m_context(context)
{
ASSERT(state() == Live);
+ ASSERT(m_jsValue && m_jsValue.isCell());
}
inline WeakImpl::State WeakImpl::state()
Modified: trunk/Source/WebCore/ChangeLog (115533 => 115534)
--- trunk/Source/WebCore/ChangeLog 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/WebCore/ChangeLog 2012-04-28 03:43:29 UTC (rev 115534)
@@ -1,3 +1,15 @@
+2012-04-27 Geoffrey Garen <[email protected]>
+
+ Only allow non-null pointers in the WeakSet
+ https://bugs.webkit.org/show_bug.cgi?id=85119
+
+ Reviewed by Darin Adler.
+
+ * bridge/jsc/BridgeJSC.cpp:
+ (JSC::Bindings::Instance::Instance): Don't allocate a WeakImpl just to
+ store null. This was needless, and is now a compile error. Instead,
+ rely on the default constructor, which will produce a cheap null.
+
2012-04-27 Kentaro Hara <[email protected]>
"Not enough arguments" error should be TypeError
Modified: trunk/Source/WebCore/bridge/jsc/BridgeJSC.cpp (115533 => 115534)
--- trunk/Source/WebCore/bridge/jsc/BridgeJSC.cpp 2012-04-28 03:30:28 UTC (rev 115533)
+++ trunk/Source/WebCore/bridge/jsc/BridgeJSC.cpp 2012-04-28 03:43:29 UTC (rev 115534)
@@ -54,7 +54,6 @@
Instance::Instance(PassRefPtr<RootObject> rootObject)
: m_rootObject(rootObject)
- , m_runtimeObject(*WebCore::JSDOMWindowBase::commonJSGlobalData())
{
ASSERT(m_rootObject);
}
