Fix duplicate event listeners in TemplateRenderer (#8428)

in case where the element is attached several times during the same roundtrip.
Fix memory leak by removing detach listener when called.

Fixes vaadin/vaadin-grid-flow#837

flow-data/src/main/java/com/vaadin/flow/data/renderer/RendererUtil.java

@@ -16,6 +16,7 @@
 package com.vaadin.flow.data.renderer;
 
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
@@ -30,6 +31,7 @@
 import com.vaadin.flow.internal.StateNode;
 import com.vaadin.flow.internal.StateTree;
 import com.vaadin.flow.server.Command;
+import com.vaadin.flow.shared.Registration;
 
 /**
  * Contains helper methods to register events triggered by rendered templates.
@@ -82,11 +84,9 @@ public static <T> void registerEventHandlers(Renderer<T> renderer,
              * the column element.
              */
             runOnAttach(templateDataHost.getNode(),
-                    () -> getUI(templateDataHost).getInternals().getStateTree()
-                            .beforeClientResponse(templateDataHost.getNode(),
-                                    context -> processTemplateRendererEventHandlers(
-                                            context.getUI(), templateDataHost,
-                                            eventHandlers, keyMapper)));
+                    () -> processTemplateRendererEventHandlers(
+                            getUI(templateDataHost), templateDataHost,
+                            eventHandlers, keyMapper));
 
             runOnAttach(contentTemplate.getNode(), () -> getUI(contentTemplate)
                     .getInternals().getStateTree()
@@ -120,18 +120,20 @@ private static <T> void processTemplateRendererEventHandlers(UI ui,
     private static <T> void setupTemplateRendererEventHandler(UI ui,
             Element eventOrigin, String handlerName, Consumer<T> consumer,
             Function<String, T> keyMapper) {
-
-        // vaadin.sendEventMessage is an exported function at the client
-        // side
-        ui.getPage().executeJs(String.format(
-                "$0.%s = function(e) {Vaadin.Flow.clients[$1].sendEventMessage(%d, '%s', {key: e.model ? e.model.__data.item.key : e.target.__dataHost.__data.item.key})}",
-                handlerName, eventOrigin.getNode().getId(), handlerName),
-                eventOrigin, ui.getInternals().getAppId());
+        ui.getInternals().getStateTree()
+                .beforeClientResponse(eventOrigin.getNode(), context ->
+                // sendEventMessage is an exported function at the client side
+                ui.getPage().executeJs(String.format(
+                        "$0.%s = function(e) {Vaadin.Flow.clients[$1].sendEventMessage(%d, '%s', {key: e.model ? e.model.__data.item.key : e.target.__dataHost.__data.item.key})}",
+                        handlerName, eventOrigin.getNode().getId(),
+                        handlerName), eventOrigin,
+                        ui.getInternals().getAppId()));
 
         DomListenerRegistration registration = eventOrigin.addEventListener(
                 handlerName, event -> processEventFromTemplateRenderer(event,
                         handlerName, consumer, keyMapper));
-        eventOrigin.addDetachListener(event -> registration.remove());
+
+        runOnceOnDetach(eventOrigin, registration::remove);
     }
 
     private static <T> void processEventFromTemplateRenderer(DomEvent event,
@@ -155,4 +157,14 @@ private static <T> void processEventFromTemplateRenderer(DomEvent event,
         }
     }
 
+    private static void runOnceOnDetach(Element element, Command action) {
+        AtomicReference<Registration> detachRegistration = new AtomicReference<>();
+        detachRegistration.set(element.addDetachListener(event -> {
+            action.execute();
+            if (detachRegistration.get() != null) {
+                detachRegistration.get().remove();
+            }
+        }));
+    }
+
 }

flow-data/src/test/java/com/vaadin/flow/data/renderer/RendererUtilTest.java

@@ -19,6 +19,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -28,8 +29,13 @@
 import com.vaadin.flow.component.internal.PendingJavaScriptInvocation;
 import com.vaadin.flow.component.internal.UIInternals;
 import com.vaadin.flow.component.internal.UIInternals.JavaScriptInvocation;
+import com.vaadin.flow.dom.DomEvent;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.function.ValueProvider;
+import com.vaadin.flow.internal.nodefeature.ElementListenerMap;
+
+import elemental.json.Json;
+import elemental.json.JsonObject;
 
 public class RendererUtilTest {
 
@@ -109,6 +115,38 @@ public void registerEventHandlers_setupEvenHandlersOnAttach() {
         assertJSExecutions(ui, internals, contentTemplate, templateDataHost);
     }
 
+    @Test
+    public void registerEventHandlers_attachMultipleTimes_singleEventListenerRegistered() {
+        UI ui = new TestUI();
+        TestUIInternals internals = (TestUIInternals) ui.getInternals();
+
+        Renderer<String> renderer = new Renderer<>();
+
+        AtomicInteger eventCounter = new AtomicInteger(0);
+
+        renderer.setEventHandler("foo", value -> {
+            eventCounter.getAndIncrement();
+        });
+
+        Element contentTemplate = new Element(Tag.DIV);
+        Element templateDataHost = new Element(Tag.SPAN);
+
+        RendererUtil.registerEventHandlers(renderer, contentTemplate,
+                templateDataHost, ValueProvider.identity());
+
+        attachElements(ui, contentTemplate, templateDataHost);
+        attachElements(ui, contentTemplate, templateDataHost);
+
+        internals.getStateTree().runExecutionsBeforeClientResponse();
+
+        JsonObject eventData = Json.createObject();
+        eventData.put("key", "");
+        templateDataHost.getNode().getFeature(ElementListenerMap.class)
+                .fireEvent(new DomEvent(templateDataHost, "foo", eventData));
+
+        Assert.assertEquals(1, eventCounter.get());
+    }
+
     private void attachElements(UI ui, Element contentTemplate,
             Element templateDataHost) {
         ui.getElement().appendChild(contentTemplate);