Detach previous UI with the same window.name (#10338, #12255)

Change-Id: I15234985f1591d6af383c6e014679762619d5000

8 files changed, 281 insertions, 47 deletions

diff --git a/WebContent/VAADIN/vaadinBootstrap.js b/WebContent/VAADIN/vaadinBootstrap.js
index b2995dd0bd..bab759b812 100644
--- a/WebContent/VAADIN/vaadinBootstrap.js
+++ b/WebContent/VAADIN/vaadinBootstrap.js
@@ -120,6 +120,8 @@
 					url += '&theme=' + encodeURIComponent(theme);
 				}
 				
+				url += "&v-appId=" + appId;
+				
 				var extraParams = getConfig('extraParams')
 				if (extraParams !== undefined) {
 					url += extraParams;
diff --git a/server/src/com/vaadin/server/Page.java b/server/src/com/vaadin/server/Page.java
index 4d7d2c10c5..4c19d28b9c 100644
--- a/server/src/com/vaadin/server/Page.java
+++ b/server/src/com/vaadin/server/Page.java
@@ -458,6 +458,8 @@ public class Page implements Serializable {
 
     private final PageState state;
 
+    private String windowName;
+
     public Page(UI uI, PageState state) {
         this.uI = uI;
         this.state = state;
@@ -592,6 +594,7 @@ public class Page implements Serializable {
         String location = request.getParameter("v-loc");
         String clientWidth = request.getParameter("v-cw");
         String clientHeight = request.getParameter("v-ch");
+        windowName = request.getParameter("v-wn");
 
         if (location != null) {
             try {
@@ -617,6 +620,17 @@ public class Page implements Serializable {
     }
 
     /**
+     * Gets the window.name value of the browser window of this page.
+     * 
+     * @since 7.2
+     * 
+     * @return the window name, <code>null</code> if the name is not known
+     */
+    public String getWindowName() {
+        return windowName;
+    }
+
+    /**
      * Updates the internal state with the given values. Does not resize the
      * Page or browser window.
      * 
diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java
index 8f27241384..8f15dacc98 100644
--- a/server/src/com/vaadin/server/VaadinSession.java
+++ b/server/src/com/vaadin/server/VaadinSession.java
@@ -40,7 +40,6 @@ import javax.servlet.http.HttpSession;
 import javax.servlet.http.HttpSessionBindingEvent;
 import javax.servlet.http.HttpSessionBindingListener;
 
-import com.vaadin.annotations.PreserveOnRefresh;
 import com.vaadin.data.util.converter.Converter;
 import com.vaadin.data.util.converter.ConverterFactory;
 import com.vaadin.data.util.converter.DefaultConverterFactory;
@@ -170,7 +169,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
     private int nextUIId = 0;
     private Map<Integer, UI> uIs = new HashMap<Integer, UI>();
 
-    private final Map<String, Integer> retainOnRefreshUIs = new HashMap<String, Integer>();
+    private final Map<String, Integer> embedIdMap = new HashMap<String, Integer>();
 
     private final EventRouter eventRouter = new EventRouter();
 
@@ -793,10 +792,13 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      */
     public void removeUI(UI ui) {
         assert hasLock();
-        int id = ui.getUIId();
+        Integer id = Integer.valueOf(ui.getUIId());
         ui.setSession(null);
         uIs.remove(id);
-        retainOnRefreshUIs.values().remove(id);
+        String embedId = ui.getEmbedId();
+        if (embedId != null && id.equals(embedIdMap.get(embedId))) {
+            embedIdMap.remove(embedId);
+        }
     }
 
     /**
@@ -1050,20 +1052,6 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
     }
 
     /**
-     * Gets the mapping from <code>window.name</code> to UI id for UIs that are
-     * should be retained on refresh.
-     * 
-     * @see VaadinService#preserveUIOnRefresh(VaadinRequest, UI, UIProvider)
-     * @see PreserveOnRefresh
-     * 
-     * @return the mapping between window names and UI ids for this session.
-     */
-    public Map<String, Integer> getPreserveOnRefreshUIs() {
-        assert hasLock();
-        return retainOnRefreshUIs;
-    }
-
-    /**
      * Adds an initialized UI to this session.
      * 
      * @param ui
@@ -1080,7 +1068,21 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
                     "The UI belongs to a different session");
         }
 
-        uIs.put(Integer.valueOf(ui.getUIId()), ui);
+        Integer uiId = Integer.valueOf(ui.getUIId());
+        uIs.put(uiId, ui);
+
+        String embedId = ui.getEmbedId();
+        if (embedId != null) {
+            Integer previousUiId = embedIdMap.put(embedId, uiId);
+            if (previousUiId != null) {
+                UI previousUi = uIs.get(previousUiId);
+                assert previousUi != null
+                        && embedId.equals(previousUi.getEmbedId()) : "UI id map and embed id map not in sync";
+
+                // Will fire cleanup events at the end of the request handling.
+                previousUi.close();
+            }
+        }
     }
 
     /**
@@ -1285,4 +1287,24 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
         return csrfToken;
     }
 
+    /**
+     * Finds the UI with the corresponding embed id.
+     * 
+     * @since 7.2
+     * @param embedId
+     *            the embed id
+     * @return the UI with the corresponding embed id, or <code>null</code> if
+     *         no UI is found
+     * 
+     * @see UI#getEmbedId()
+     */
+    public UI getUIByEmbedId(String embedId) {
+        Integer uiId = embedIdMap.get(embedId);
+        if (uiId == null) {
+            return null;
+        } else {
+            return getUIById(uiId.intValue());
+        }
+    }
+
 }
diff --git a/server/src/com/vaadin/server/communication/UIInitHandler.java b/server/src/com/vaadin/server/communication/UIInitHandler.java
index e16e4f6cdd..b2c17d00c1 100644
--- a/server/src/com/vaadin/server/communication/UIInitHandler.java
+++ b/server/src/com/vaadin/server/communication/UIInitHandler.java
@@ -20,7 +20,6 @@ import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.StringWriter;
 import java.util.List;
-import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -164,31 +163,29 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler {
             return null;
         }
 
-        // Check for an existing UI based on window.name
+        // Check for an existing UI based on embed id
 
-        // Special parameter sent by vaadinBootstrap.js
-        String windowName = request.getParameter("v-wn");
-
-        Map<String, Integer> retainOnRefreshUIs = session
-                .getPreserveOnRefreshUIs();
-        if (windowName != null && !retainOnRefreshUIs.isEmpty()) {
-            // Check for a known UI
+        String embedId = getEmbedId(request);
 
-            Integer retainedUIId = retainOnRefreshUIs.get(windowName);
-
-            if (retainedUIId != null) {
-                UI retainedUI = session.getUIById(retainedUIId.intValue());
+        UI retainedUI = session.getUIByEmbedId(embedId);
+        if (retainedUI != null) {
+            if (vaadinService.preserveUIOnRefresh(provider, new UICreateEvent(
+                    request, uiClass))) {
                 if (uiClass.isInstance(retainedUI)) {
                     reinitUI(retainedUI, request);
                     return retainedUI;
                 } else {
                     getLogger().info(
-                            "Not using retained UI in " + windowName
-                                    + " because retained UI was of type "
+                            "Not using the preserved UI " + embedId
+                                    + " because it is of type "
                                     + retainedUI.getClass() + " but " + uiClass
                                     + " is expected for the request.");
                 }
             }
+            /*
+             * Previous UI without preserve on refresh will be closed when the
+             * new UI gets added to the session.
+             */
         }
 
         // No existing UI found - go on by creating and initializing one
@@ -221,26 +218,45 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler {
         // Set thread local here so it is available in init
         UI.setCurrent(ui);
 
-        ui.doInit(request, uiId.intValue());
+        ui.doInit(request, uiId.intValue(), embedId);
 
         session.addUI(ui);
 
-        // Remember if it should be remembered
-        if (vaadinService.preserveUIOnRefresh(provider, event)) {
-            // Remember this UI
-            if (windowName == null) {
-                getLogger().warning(
-                        "There is no window.name available for UI " + uiClass
-                                + " that should be preserved.");
-            } else {
-                session.getPreserveOnRefreshUIs().put(windowName, uiId);
-            }
+        // Warn if the window can't be preserved
+        if (embedId == null
+                && vaadinService.preserveUIOnRefresh(provider, event)) {
+            getLogger().warning(
+                    "There is no embed id available for UI " + uiClass
+                            + " that should be preserved.");
         }
 
         return ui;
     }
 
     /**
+     * Constructs an embed id based on information in the request.
+     * 
+     * @since 7.2
+     * 
+     * @param request
+     *            the request to get embed information from
+     * @return the embed id, or <code>null</code> if id is not available.
+     * 
+     * @see UI#getEmbedId()
+     */
+    protected String getEmbedId(VaadinRequest request) {
+        // Parameters sent by vaadinBootstrap.js
+        String windowName = request.getParameter("v-wn");
+        String appId = request.getParameter("v-appId");
+
+        if (windowName != null && appId != null) {
+            return windowName + '.' + appId;
+        } else {
+            return null;
+        }
+    }
+
+    /**
      * Updates a UI that has already been initialized but is now loaded again,
      * e.g. because of {@link PreserveOnRefresh}.
      * 
diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java
index d4756e9ec1..8beebb0f1e 100644
--- a/server/src/com/vaadin/ui/UI.java
+++ b/server/src/com/vaadin/ui/UI.java
@@ -547,6 +547,8 @@ public abstract class UI extends AbstractSingleComponentContainer implements
     private LocaleService localeService = new LocaleService(this,
             getState(false).localeServiceState);
 
+    private String embedId;
+
     /**
      * This method is used by Component.Focusable objects to request focus to
      * themselves. Focus renders must be handled at window level (instead of
@@ -594,12 +596,19 @@ public abstract class UI extends AbstractSingleComponentContainer implements
      *            the initialization request
      * @param uiId
      *            the id of the new ui
+     * @param embedId
+     *            the embed id of this UI, or <code>null</code> if no id is
+     *            known
+     * 
+     * @see #getUIId()
+     * @see #getEmbedId()
      */
-    public void doInit(VaadinRequest request, int uiId) {
+    public void doInit(VaadinRequest request, int uiId, String embedId) {
         if (this.uiId != -1) {
             throw new IllegalStateException("UI id has already been defined");
         }
         this.uiId = uiId;
+        this.embedId = embedId;
 
         // Actual theme - used for finding CustomLayout templates
         theme = request.getParameter("theme");
@@ -1498,4 +1507,18 @@ public abstract class UI extends AbstractSingleComponentContainer implements
     private static Logger getLogger() {
         return Logger.getLogger(UI.class.getName());
     }
+
+    /**
+     * Gets a string the uniquely distinguishes this UI instance based on where
+     * it is embedded. The embed identifier is based on the
+     * <code>window.name</code> DOM attribute of the browser window where the UI
+     * is displayed and the id of the div element where the UI is embedded.
+     * 
+     * @since 7.2
+     * @return the embed id for this UI, or <code>null</code> if no id known
+     */
+    public String getEmbedId() {
+        return embedId;
+    }
+
 }
diff --git a/server/tests/src/com/vaadin/server/VaadinSessionTest.java b/server/tests/src/com/vaadin/server/VaadinSessionTest.java
index 68f198410c..51ae2a2d13 100644
--- a/server/tests/src/com/vaadin/server/VaadinSessionTest.java
+++ b/server/tests/src/com/vaadin/server/VaadinSessionTest.java
@@ -100,7 +100,7 @@ public class VaadinSessionTest {
             }
         };
 
-        ui.doInit(vaadinRequest, session.getNextUIid());
+        ui.doInit(vaadinRequest, session.getNextUIid(), null);
 
         ui.setSession(session);
         session.addUI(ui);
diff --git a/uitest/src/com/vaadin/tests/application/DetachOldUIOnReload.html b/uitest/src/com/vaadin/tests/application/DetachOldUIOnReload.html
new file mode 100644
index 0000000000..038283324d
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/application/DetachOldUIOnReload.html
@@ -0,0 +1,77 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="" />
+<title>New Test</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">New Test</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.application.DetachOldUIOnReload?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td>
+	<td>This is UI 0</td>
+</tr>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.application.DetachOldUIOnReload</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::PID_SLog_row_0</td>
+	<td>1. UI 0 has been detached</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td>
+	<td>This is UI 1</td>
+</tr>
+<tr>
+	<td>clickAndWait</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[1]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::PID_SLog_row_0</td>
+	<td>2. UI 1 has been detached</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td>
+	<td>This is UI 2</td>
+</tr>
+<tr>
+	<td>open</td>
+	<td>/</td>
+	<td></td>
+</tr>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.application.DetachOldUIOnReload</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::PID_SLog_row_0</td>
+	<td>3. UI 2 has been detached</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestsapplicationDetachOldUIOnReload::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td>
+	<td>This is UI 3</td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/uitest/src/com/vaadin/tests/application/DetachOldUIOnReload.java b/uitest/src/com/vaadin/tests/application/DetachOldUIOnReload.java
new file mode 100644
index 0000000000..7104146203
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/application/DetachOldUIOnReload.java
@@ -0,0 +1,80 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.vaadin.tests.application;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Label;
+
+public class DetachOldUIOnReload extends AbstractTestUIWithLog {
+    private static final String PERSISTENT_MESSAGES_ATTRIBUTE = DetachOldUIOnReload.class
+            .getName() + ".sessionMessages";
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        for (String message : getSessionMessages(false)) {
+            log(message);
+        }
+
+        addComponent(new Label("This is UI " + getUIId()));
+        addComponent(new Button("Reload page", new Button.ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                getPage().reload();
+            }
+        }));
+    }
+
+    private List<String> getSessionMessages(boolean storeIfNeeded) {
+        List<String> messages = (List<String>) getSession().getAttribute(
+                PERSISTENT_MESSAGES_ATTRIBUTE);
+        if (messages == null) {
+            messages = new ArrayList<String>();
+            if (storeIfNeeded) {
+                getSession().setAttribute(PERSISTENT_MESSAGES_ATTRIBUTE,
+                        messages);
+            }
+        }
+        return messages;
+    }
+
+    private void logToSession(String message) {
+        getSessionMessages(true).add(message);
+    }
+
+    @Override
+    public void detach() {
+        super.detach();
+        logToSession("UI " + getUIId() + " has been detached");
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Tests that the previous UI gets cleaned immediately when refreshing.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(10338);
+    }
+
+}