Title: [142154] trunk/Source/WebCore
- Revision
- 142154
- Author
- [email protected]
- Date
- 2013-02-07 10:57:09 -0800 (Thu, 07 Feb 2013)
Log Message
[V8] StringCache::m_stringCache should be HashMap<StringImpl*, Persistent<String>>
https://bugs.webkit.org/show_bug.cgi?id=109123
Reviewed by Adam Barth.
Currently StringCache::m_stringCache is implemented as
HashMap<StringImpl*, v8::String*>. Given that v8::String*
can change when a GC is triggered, it is dangerous to store a raw pointer.
We should use HashMap<StringImpl*, v8::Persistent<v8::String>> instead.
This is a possible fix for an IndexedDB crash (https://bugs.webkit.org/show_bug.cgi?id=105363),
although I'm not sure if this patch fixes the crash. (I couldn't reproduce the crash.)
No tests. This change highly depends on GC behavior and thus it is
difficult to make a reliable test case.
* bindings/v8/V8ValueCache.cpp:
(WebCore::makeExternalString):
* bindings/v8/V8ValueCache.h:
(StringCache):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (142153 => 142154)
--- trunk/Source/WebCore/ChangeLog 2013-02-07 18:49:45 UTC (rev 142153)
+++ trunk/Source/WebCore/ChangeLog 2013-02-07 18:57:09 UTC (rev 142154)
@@ -1,3 +1,26 @@
+2013-02-07 Kentaro Hara <[email protected]>
+
+ [V8] StringCache::m_stringCache should be HashMap<StringImpl*, Persistent<String>>
+ https://bugs.webkit.org/show_bug.cgi?id=109123
+
+ Reviewed by Adam Barth.
+
+ Currently StringCache::m_stringCache is implemented as
+ HashMap<StringImpl*, v8::String*>. Given that v8::String*
+ can change when a GC is triggered, it is dangerous to store a raw pointer.
+ We should use HashMap<StringImpl*, v8::Persistent<v8::String>> instead.
+
+ This is a possible fix for an IndexedDB crash (https://bugs.webkit.org/show_bug.cgi?id=105363),
+ although I'm not sure if this patch fixes the crash. (I couldn't reproduce the crash.)
+
+ No tests. This change highly depends on GC behavior and thus it is
+ difficult to make a reliable test case.
+
+ * bindings/v8/V8ValueCache.cpp:
+ (WebCore::makeExternalString):
+ * bindings/v8/V8ValueCache.h:
+ (StringCache):
+
2013-01-27 Robert Hogan <[email protected]>
CSS 2.1 failure: floats-149 fails
Modified: trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp (142153 => 142154)
--- trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp 2013-02-07 18:49:45 UTC (rev 142153)
+++ trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp 2013-02-07 18:57:09 UTC (rev 142154)
@@ -85,16 +85,13 @@
if (!stringImpl->length())
return v8::String::Empty(isolate);
- v8::String* cachedV8String = m_stringCache.get(stringImpl);
- if (cachedV8String) {
- v8::Persistent<v8::String> handle(cachedV8String);
- if (handle.IsWeak()) {
- m_lastStringImpl = stringImpl;
- m_lastV8String = handle;
- if (handleType == ReturnUnsafeHandle)
- return handle;
- return v8::Local<v8::String>::New(handle);
- }
+ v8::Persistent<v8::String> cachedV8String = m_stringCache.get(stringImpl);
+ if (cachedV8String.IsWeak()) {
+ m_lastStringImpl = stringImpl;
+ m_lastV8String = cachedV8String;
+ if (handleType == ReturnUnsafeHandle)
+ return cachedV8String;
+ return v8::Local<v8::String>::New(cachedV8String);
}
v8::Local<v8::String> newString = makeExternalString(String(stringImpl));
@@ -108,7 +105,7 @@
stringImpl->ref();
wrapper.MarkIndependent();
wrapper.MakeWeak(isolate, stringImpl, cachedStringCallback);
- m_stringCache.set(stringImpl, *wrapper);
+ m_stringCache.set(stringImpl, wrapper);
m_lastStringImpl = stringImpl;
m_lastV8String = wrapper;
Modified: trunk/Source/WebCore/bindings/v8/V8ValueCache.h (142153 => 142154)
--- trunk/Source/WebCore/bindings/v8/V8ValueCache.h 2013-02-07 18:49:45 UTC (rev 142153)
+++ trunk/Source/WebCore/bindings/v8/V8ValueCache.h 2013-02-07 18:57:09 UTC (rev 142154)
@@ -71,7 +71,7 @@
private:
v8::Handle<v8::String> v8ExternalStringSlow(StringImpl*, ReturnHandleType, v8::Isolate*);
- HashMap<StringImpl*, v8::String*> m_stringCache;
+ HashMap<StringImpl*, v8::Persistent<v8::String> > m_stringCache;
v8::Persistent<v8::String> m_lastV8String;
// Note: RefPtr is a must as we cache by StringImpl* equality, not identity
// hence lastStringImpl might be not a key of the cache (in sense of identity)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
