Reviewers: Mads Ager,
Message:
Tested on Mac, Win and Linux Chromium, and Mac WebKit tree, and all is good.
Description:
Fix memory corruption with AdoptText method.
Icu setText method keeps pointer to text, it doesn't copy it so we have to
keep
text around for the lifetime of the break iterator object,
or next setText operation.
Please review this at http://codereview.chromium.org/6609038/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/extensions/experimental/break-iterator.h
M src/extensions/experimental/break-iterator.cc
Index: src/extensions/experimental/break-iterator.cc
===================================================================
--- src/extensions/experimental/break-iterator.cc (revision 7051)
+++ src/extensions/experimental/break-iterator.cc (working copy)
@@ -46,6 +46,23 @@
return NULL;
}
+UnicodeString* BreakIterator::ResetAdoptedText(
+ v8::Handle<v8::Object> obj, v8::Handle<v8::Value> value) {
+ // Get the previous value from the internal field.
+ UnicodeString* text = static_cast<UnicodeString*>(
+ obj->GetPointerFromInternalField(1));
+ delete text;
+
+ // Assign new value to the internal pointer.
+ v8::String::Value text_value(value);
+ text = new UnicodeString(
+ reinterpret_cast<const UChar*>(*text_value), text_value.length());
+ obj->SetPointerInInternalField(1, text);
+
+ // Return new unicode string pointer.
+ return text;
+}
+
void BreakIterator::DeleteBreakIterator(v8::Persistent<v8::Value> object,
void* param) {
v8::Persistent<v8::Object> persistent_object =
@@ -57,6 +74,9 @@
// pointing to a break iterator.
delete UnpackBreakIterator(persistent_object);
+ delete static_cast<UnicodeString*>(
+ persistent_object->GetPointerFromInternalField(1));
+
// Then dispose of the persistent handle to JS object.
persistent_object.Dispose();
}
@@ -81,12 +101,8 @@
return ThrowUnexpectedObjectError();
}
- v8::String::Value text_value(args[0]);
- UnicodeString text(
- reinterpret_cast<const UChar*>(*text_value), text_value.length());
+ break_iterator->setText(*ResetAdoptedText(args.Holder(), args[0]));
- break_iterator->setText(text);
-
return v8::Undefined();
}
@@ -192,8 +208,10 @@
// Define internal field count on instance template.
v8::Local<v8::ObjectTemplate> object_template =
raw_template->InstanceTemplate();
- object_template->SetInternalFieldCount(1);
+ // Set aside internal fields for icu break iterator and adopted text.
+ object_template->SetInternalFieldCount(2);
+
// Define all of the prototype methods on prototype template.
v8::Local<v8::ObjectTemplate> proto =
raw_template->PrototypeTemplate();
proto->Set(v8::String::New("adoptText"),
@@ -219,6 +237,8 @@
// Set break iterator as internal field of the resulting JS object.
wrapper->SetPointerInInternalField(0, break_iterator);
+ // Make sure that the pointer to adopted text is NULL.
+ wrapper->SetPointerInInternalField(1, NULL);
// Make object handle weak so we can delete iterator once GC kicks in.
wrapper.MakeWeak(NULL, DeleteBreakIterator);
Index: src/extensions/experimental/break-iterator.h
===================================================================
--- src/extensions/experimental/break-iterator.h (revision 7051)
+++ src/extensions/experimental/break-iterator.h (working copy)
@@ -34,6 +34,7 @@
namespace U_ICU_NAMESPACE {
class BreakIterator;
+class UnicodeString;
}
namespace v8 {
@@ -48,6 +49,11 @@
// Unpacks break iterator object from corresponding JavaScript object.
static icu::BreakIterator* UnpackBreakIterator(v8::Handle<v8::Object>
obj);
+ // Deletes the old value and sets the adopted text in
+ // corresponding JavaScript object.
+ static UnicodeString* ResetAdoptedText(v8::Handle<v8::Object> obj,
+ v8::Handle<v8::Value> text_value);
+
// Release memory we allocated for the BreakIterator once the JS object
that
// holds the pointer gets garbage collected.
static void DeleteBreakIterator(v8::Persistent<v8::Value> object,
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
