Re: [UndoManager] Re-introduce DOMTransaction interface?

On Wed, Jul 4, 2012 at 8:05 PM, Ojan Vafai <ojan@chromium.org> wrote:

> On Wed, Jul 4, 2012 at 5:39 PM, Ryosuke Niwa <rniwa@webkit.org> wrote:
>
>> On Jul 4, 2012 5:26 PM, "Olli Pettay" <Olli.Pettay@helsinki.fi> wrote:
>> >
>> > On 07/05/2012 03:11 AM, Ryosuke Niwa wrote:
>> >
>> >> On Wed, Jul 4, 2012 at 5:00 PM, Olli Pettay <Olli.Pettay@helsinki.fi<mailto:
>> Olli.Pettay@helsinki.fi>> wrote:
>> >>
>> >>     On 07/05/2012 01:38 AM, Ryosuke Niwa wrote:
>> >>
>> >>         Hi all,
>> >>
>> >>         Sukolsak has been implementing the Undo Manager API in WebKit
>> but the fact undoManager.transact() takes a pure JS object with callback
>> >>         functions is
>> >>         making it very challenging.  The problem is that this object
>> needs to be kept alive by either JS reference or DOM but doesn't have a
>> backing C++
>> >>         object.  Also, as far as we've looked, there are no other
>> specification that uses the same mechanism.
>> >>
>> >>
>> >>     I don't understand what is difficult.
>> >>     How is that any different to
>> >>     target.addEventListener("foo", { handleEvent: function() {}})
>> >>
>> >>
>> >> It will be very similar to that except this object is going to have 3
>> callbacks instead of one.
>> >>
>> >> The problem is that the event listener is a very special object in
>> WebKit for which we have a lot of custom binding code. We don't want to
>> implement a
>> >> similar behavior for the DOM transaction because it's very error prone.
>> >
>> >
>> > So, it is very much implementation detail.
>> > (And I still don't understand how a callback can be so hard in this
>> case. There are plenty of different kinds of callback objects.
>> >  new MutationObserver(some_callback_function_object) )
>>
>> Yes. It's an implementation feedback. The mutation observer callback is
>> implemented as a special event handler in WebKit.
>>
> How does this make implementation easier? I'm pretty sure Adam's concern
> about increasing the likelihood of memory leaks due to the function objects
> that are held on indefinitely by the undomanager. That concern is not
> implementation specific and is not addressed by this change. The only thing
> that would address Adam's concern that I can think of would be something
> that didn't involve registering functions at all. I have trouble thinking
> of a useful undomanager API that addresses his memory leak concern though.
>

That is not the problem I'm trying to address.

- Ryosuke