AG-13363 Long-Tap iOS Safari

[olegat]

https://ag-grid.atlassian.net/browse/AG-13363

Depends on:

• AG-13915 QA Feedback: zoom.dragAction: 'drag' #3439
• AG-13967 Touch docs placeholder #3441

[alantreadway]

In the cases where longTapInterrupted is set to true, could you cancel the setTimeout() via clearTimeout() so that we don't have dangling JS variable contexts hanging around?

It would also be good if we could clear these up on chart.destroy() too, otherwise the callback could happen after chart destruction, with inevitable errors - also references in the callback context can prevent garbage collection?

[olegat]

In the cases where longTapInterrupted is set to true, could you cancel the setTimeout() via clearTimeout() so that we don't have dangling JS variable contexts hanging around?

Can you clarify how we would end up with a dangling reference? I don't mind using clearTimeout(), but I cannot see how that improves memory safety. Without a clearTimeout(), the setTimeout() will eventually call then remove the function, which would in turn make its variable context eligible for garbage collection.

It would also be good if we could clear these up on chart.destroy() too, otherwise the callback could happen after chart destruction, with inevitable errors

The callbacks will not cause errors because we always remove the listeners on chart.destroy() (see WidgetListenerInternal.destroy()). So these callbacks will, at worst, loop through an undefined or empty array of listeners.

Calling chart.destroy() whilst a finger is being dragged on that chart can indeed delay garbage collection until after the finger is lifted. Removing the listeners on chart.destroy() isn't necessary, but it would stop this delay. However, the challenge is that these are global callbacks shared amongst all chart instances, we must be cautious to ensure that destroying one chart doesn't interrupt another chart that's currently being dragged.

We could perhaps refactor the startMouseDrag and startOneFingerTouch to be temporary member objects of WidgetListenerInternal to manage the memory better. But the way I see it there's very little gain in doing that: there's only one mouse & touch device in the window and therefore there's only ever one drag motion in progress. So we can safely manage the memory regardless of whether or not the chart gets destroyed.

also references in the callback context can prevent garbage collection?

Are you referring to that.globalTouchDragCallbacks? If yes, that variable is used correctly AFAIK. It sets to undefined either in the touchend handler (called on 'touchstart', 'touchend' or 'touchcancel') or in the WidgetListenerInternal.destroy() method (called by chart.destroy()).