I've chatted with Mads today to make sure FunctionTemplate would work for
me.
It actually lets me do things I wanted to do with ObjectTemplate but didn't
see
an easy way of doing it, like specifying the prototype methods.
I now:
1. Create FunctionTemplate
2. Add internal field to its InstanceTemplate
3. Add all methods to its PrototypeTemplate
4. Create each break iterator as persistent
5. Check the type on upack
6. Delete the pointer and Dispose of the handle on delete callback
http://codereview.chromium.org/6598014/diff/4003/src/extensions/experimental/break-iterator.cc
File src/extensions/experimental/break-iterator.cc (right):
http://codereview.chromium.org/6598014/diff/4003/src/extensions/experimental/break-iterator.cc#newcode42
src/extensions/experimental/break-iterator.cc:42: return
static_cast<icu::BreakIterator*>(obj->GetPointerFromInternalField(0));
On 2011/02/28 13:03:32, Mads Ager wrote:
You need to be more careful here. The users of UnpackBreakIterator are
functions. These functions can be moved to other objects and therefore
the
holder does not need to be a break iterator object at all.
Instead of using an ObjectTemplate for you break iterator wrappers I
think you
should use a FunctionTemplate instead. Then here you can check that
the argument
is an instance of that function template. If it is, it will have be
internal
field and the value in it will be a BreakIterator pointer.
Have a look at FunctionTemplate. You add the methods on the
instance_template as
you do now. Then you can use templ->HasInstance(obj).
Done.
http://codereview.chromium.org/6598014/diff/4003/src/extensions/experimental/break-iterator.cc#newcode49
src/extensions/experimental/break-iterator.cc:49: delete
UnpackBreakIterator(v8::Persistent<v8::Object>::Cast(object));
On 2011/02/28 13:03:32, Mads Ager wrote:
You need to do object.Dispose() as well to delete the persistent
handle as well.
This callback needs to be used on all break iterator instances instead
of on the
template (see below).
Done.
http://codereview.chromium.org/6598014/diff/4003/src/extensions/experimental/break-iterator.cc#newcode150
src/extensions/experimental/break-iterator.cc:150: v8::HandleScope
handle_scope;
On 2011/02/28 13:03:32, Mads Ager wrote:
Is there a reason not have have this handle scope earlier in this
method? You
are also allocating strings above for instance. The handles for those
might as
well be local as well.
Done.
http://codereview.chromium.org/6598014/diff/4003/src/extensions/experimental/break-iterator.cc#newcode167
src/extensions/experimental/break-iterator.cc:167: // Make template weak
so we can delete iterator once GC kicks in.
Yip, it felt weird when I did it. Never occurred to me I should make
actual Object persistent.
Done.
On 2011/02/28 13:03:32, Mads Ager wrote:
This is not the thing that you want to make weak. The
break_iterator_template
does not correspond to a concrete break iterator?
http://codereview.chromium.org/6598014/diff/4003/src/extensions/experimental/break-iterator.cc#newcode174
src/extensions/experimental/break-iterator.cc:174:
wrapper->SetPointerInInternalField(0, break_iterator);
On 2011/02/28 13:03:32, Mads Ager wrote:
This is where you set up the correspondence. Here you need to decide
on the
memory management story that you want. I think your BreakIterator
instance needs
to have a reference (persistent handle) to the wrapper so it can
return it again
for the next access to it. Also, that will allow you to tie the
life-time of the
wrapper to the lifetime of the breakiterator C++ instance.
Is the breakiterator C++ instance used by anything else than
JavaScript? If not,
then each break iterator C++ instance should have a persistent handle
to the JS
wrapper. This persistent handle should be weak and in the callback you
should
call dispose on the persistent handle *and* delete the break iterator
object.
Done.
http://codereview.chromium.org/6598014/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
