Title: [176824] trunk/Source/_javascript_Core
- Revision
- 176824
- Author
- [email protected]
- Date
- 2014-12-04 16:55:30 -0800 (Thu, 04 Dec 2014)
Log Message
REGRESSION(r173188): Text inserted when trying to delete a word from the Twitter message box.
<https://webkit.org/b/139076>
Reviewed by Geoffrey Garen.
The StringImpl* -> Weak<JSString> cache used by the DOM bindings
had a bug where the key could become a stale pointer if the cached
JSString had its internal StringImpl atomicized.
If a new StringImpl was then later constructed at the exact same
address as the stale key, before the Weak<JSString> got booted out
of the string cache, we'd now have a situation where asking the
string cache for that key would return the old JSString.
Solve this by not allowing JSString::toExistingAtomicString() to
change the JSString's internal StringImpl unless it's resolving a
rope string. (The StringImpl nullity determines rope state.)
This means that calling toExistingAtomicString() may now have to
query the AtomicString table on each call rather than just once.
All clients of this API would be forced to do this regardless,
since they return value will be used to key into containers with
AtomicStringImpl* keys.
No test because this relies on malloc putting two StringImpls
at the same address at different points in time and we have no
mechanism to reliably test that.
* runtime/JSString.h:
(JSC::JSString::toExistingAtomicString):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (176823 => 176824)
--- trunk/Source/_javascript_Core/ChangeLog 2014-12-04 23:55:34 UTC (rev 176823)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-12-05 00:55:30 UTC (rev 176824)
@@ -1,3 +1,36 @@
+2014-12-04 Andreas Kling <[email protected]>
+
+ REGRESSION(r173188): Text inserted when trying to delete a word from the Twitter message box.
+ <https://webkit.org/b/139076>
+
+ Reviewed by Geoffrey Garen.
+
+ The StringImpl* -> Weak<JSString> cache used by the DOM bindings
+ had a bug where the key could become a stale pointer if the cached
+ JSString had its internal StringImpl atomicized.
+
+ If a new StringImpl was then later constructed at the exact same
+ address as the stale key, before the Weak<JSString> got booted out
+ of the string cache, we'd now have a situation where asking the
+ string cache for that key would return the old JSString.
+
+ Solve this by not allowing JSString::toExistingAtomicString() to
+ change the JSString's internal StringImpl unless it's resolving a
+ rope string. (The StringImpl nullity determines rope state.)
+
+ This means that calling toExistingAtomicString() may now have to
+ query the AtomicString table on each call rather than just once.
+ All clients of this API would be forced to do this regardless,
+ since they return value will be used to key into containers with
+ AtomicStringImpl* keys.
+
+ No test because this relies on malloc putting two StringImpls
+ at the same address at different points in time and we have no
+ mechanism to reliably test that.
+
+ * runtime/JSString.h:
+ (JSC::JSString::toExistingAtomicString):
+
2014-12-04 Geoffrey Garen <[email protected]>
Marked some final things final.
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (176823 => 176824)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2014-12-04 23:55:34 UTC (rev 176823)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2014-12-05 00:55:30 UTC (rev 176824)
@@ -482,12 +482,7 @@
return static_cast<const JSRopeString*>(this)->resolveRopeToExistingAtomicString(exec);
if (m_value.impl()->isAtomic())
return static_cast<AtomicStringImpl*>(m_value.impl());
- if (AtomicStringImpl* existingAtomicString = AtomicString::find(m_value.impl())) {
- m_value = *existingAtomicString;
- setIs8Bit(m_value.impl()->is8Bit());
- return existingAtomicString;
- }
- return nullptr;
+ return AtomicString::find(m_value.impl());
}
inline const String& JSString::value(ExecState* exec) const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
