Re: [UndoManager] Re-introduce DOMTransaction interface?

On 07/05/2012 08:00 AM, Adam Barth wrote:
> On Wed, Jul 4, 2012 at 5:25 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) )
>
> I haven't tested, by my reading of the MutationObserver implementation
> in WebKit is that it leaks.  Specifically:
>
> MutationObserver --retains--> MutationCallback --retains-->
> some_callback_function_object --retains--> MutationObserver
>
> I don't see any code that breaks this cycle.


Ok. In Gecko cycle collector breaks the cycle. But very much an implementation detail.


>
> DOM events
Probably EventListeners, not Events.

> have a bunch of delicate code to avoid break these
> reference cycles and avoid leaks.  We can re-invent that wheel here,
Or use some generic approach to fix such leaks.

> but it's going to be buggy and leaky.
In certain kinds of implementations.

>
> I appreciatie that these jQuery-style APIs are fashionable at the
> moment, but API fashions come and go.  If we use this approach, we'll
> need to maintain this buggy, leaky code forever.
Implementation detail. Very much so :)

Do JS callbacks cause implementation problems in Presto or Trident?




-Olli



> Instead, we can save
> ourselves a lot of pain by just using events, like the rest of the web
> platform.
>
> Adam
>
>
>>>          Since I want to make the API consistent with the rest of the
>>> platform and the implementation maintainable in WebKit, I propose the
>>> following
>>>          changes:
>>>
>>>             * Re-introduce DOMTransaction interface so that scripts can
>>> instantiate new DOMTransaction().
>>>             * Introduce AutomaticDOMTransaction that inherits from
>>> DOMTransaction and has a constructor that takes two arguments: a function
>>> and an
>>>          optional label
>>>
>>>
>>>          After this change, authors can write:
>>>          scope.undoManager.transact(new AutomaticDOMTransaction{__function
>>> () {
>>>
>>>                scope.appendChild("foo");
>>>          }, 'append "foo"'));
>>>
>>>
>>>      Looks somewhat odd. DOMTransaction would be just a container for a
>>> callback?
>>>
>>>
>>> Right. If we wanted, we can make DOMTransaction an event target and
>>> implement execute, undo, & redo as event listeners to further simplify the
>>> matter.
>>
>>
>> That could make the code more consistent with rest of the platform, but the
>> API would become harder to use.
>>
>>>
>>> - Ryosuke
>>>
>>

Received on Thursday, 5 July 2012 08:38:00 UTC