Implemented changes to CSS injection based on API review #5500

Change-Id: I2bed5f5a5c3cfc6b97e94cbd218bb06f446c7325

4 files changed, 71 insertions, 125 deletions

diff --git a/client/src/com/vaadin/client/ui/ui/UIConnector.java b/client/src/com/vaadin/client/ui/ui/UIConnector.java
index f4524882fa..69296b537c 100644
--- a/client/src/com/vaadin/client/ui/ui/UIConnector.java
+++ b/client/src/com/vaadin/client/ui/ui/UIConnector.java
@@ -17,7 +17,6 @@ package com.vaadin.client.ui.ui;
 
 import java.util.ArrayList;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 
 import com.google.gwt.core.client.Scheduler;
@@ -26,7 +25,6 @@ import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.HeadElement;
 import com.google.gwt.dom.client.LinkElement;
 import com.google.gwt.dom.client.NativeEvent;
-import com.google.gwt.dom.client.NodeList;
 import com.google.gwt.dom.client.Style;
 import com.google.gwt.dom.client.Style.Position;
 import com.google.gwt.dom.client.StyleInjector;
@@ -62,7 +60,7 @@ import com.vaadin.client.ui.VNotification;
 import com.vaadin.client.ui.VUI;
 import com.vaadin.client.ui.layout.MayScrollChildren;
 import com.vaadin.client.ui.window.WindowConnector;
-import com.vaadin.server.Page.StyleSheet;
+import com.vaadin.server.Page.Styles;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.ui.ComponentStateUtil;
 import com.vaadin.shared.ui.Connect;
@@ -340,7 +338,7 @@ public class UIConnector extends AbstractSingleComponentContainerConnector
     }
 
     /**
-     * Reads CSS strings and resources injected by {@link StyleSheet#inject}
+     * Reads CSS strings and resources injected by {@link Styles#inject}
      * from the UIDL stream.
      * 
      * @param uidl
@@ -354,8 +352,6 @@ public class UIConnector extends AbstractSingleComponentContainerConnector
         /*
          * Search the UIDL stream for CSS resources and strings to be injected.
          */
-        final List<String> resourcesToInject = new LinkedList<String>();
-        final StringBuilder cssToInject = new StringBuilder();
         for (Iterator<?> it = uidl.getChildIterator(); it.hasNext();) {
             UIDL cssInjectionsUidl = (UIDL) it.next();
 
@@ -364,62 +360,22 @@ public class UIConnector extends AbstractSingleComponentContainerConnector
                 String url = getWidget().connection
                         .translateVaadinUri(cssInjectionsUidl
                                 .getStringAttribute("url"));
-
-                // Check if url already has been injected
-                boolean injected = false;
-                NodeList<com.google.gwt.dom.client.Element> links = head
-                        .getElementsByTagName(LinkElement.TAG);
-                for (int i = 0; i < links.getLength(); i++) {
-                    LinkElement link = LinkElement.as(links.getItem(i));
-                    if (link.getHref().equals(url)) {
-                        injected = true;
-                        break;
-                    }
-                }
-
-                if (!injected) {
-                    // Ensure duplicates do not get injected
-                    resourcesToInject.add(url);
-                }
+                LinkElement link = LinkElement.as(DOM
+                        .createElement(LinkElement.TAG));
+                link.setRel("stylesheet");
+                link.setHref(url);
+                link.setType("text/css");
+                head.appendChild(link);
 
                 // Check if we have CSS string to inject
             } else if (cssInjectionsUidl.getTag().equals("css-string")) {
                 for (Iterator<?> it2 = cssInjectionsUidl.getChildIterator(); it2
                         .hasNext();) {
-                    cssToInject.append((String) it2.next());
+                    StyleInjector.injectAtEnd((String) it2.next());
+                    StyleInjector.flush();
                 }
             }
         }
-
-        /*
-         * Inject resources as deferred to ensure other Vaadin resources that
-         * are located before in the DOM get applied first so the injected ones
-         * can override them.
-         */
-        if (!resourcesToInject.isEmpty()) {
-            Scheduler.get().scheduleDeferred(new ScheduledCommand() {
-
-                @Override
-                public void execute() {
-                    for (String url : resourcesToInject) {
-                        LinkElement link = LinkElement.as(DOM
-                                .createElement(LinkElement.TAG));
-                        link.setRel("stylesheet");
-                        link.setHref(url);
-                        link.setType("text/css");
-                        head.appendChild(link);
-                    }
-                }
-            });
-        }
-
-        /*
-         * Inject the string CSS injections as a combined style tag. Not
-         * injected as deferred since StyleInjector will do it for us.
-         */
-        if (cssToInject.length() > 0) {
-            StyleInjector.injectAtEnd(cssToInject.toString());
-        }
     }
 
     public void init(String rootPanelId,
diff --git a/server/src/com/vaadin/server/Page.java b/server/src/com/vaadin/server/Page.java
index e84271e883..a7e0f7dcb3 100644
--- a/server/src/com/vaadin/server/Page.java
+++ b/server/src/com/vaadin/server/Page.java
@@ -21,9 +21,11 @@ import java.lang.reflect.Method;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.EventObject;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import com.vaadin.event.EventRouter;
 import com.vaadin.shared.ui.BorderStyle;
@@ -309,25 +311,22 @@ public class Page implements Serializable {
      * 
      * @since 7.1
      */
-    public static class StyleSheet implements Serializable {
+    public static class Styles implements Serializable {
 
-        /*
-         * Points to last injected string injection
-         */
-        private int injectedStringPointer;
+        private final Map<Integer, String> stringInjections = new HashMap<Integer, String>();
 
-        private final List<String> stringInjections = new LinkedList<String>();
+        private final Map<Integer, Resource> resourceInjections = new HashMap<Integer, Resource>();
 
-        /*
-         * Points to last injected resource injection
-         */
-        private int injectedResourcesPointer;
+        // The combined injection counter between both string and resource
+        // injections. Used as the key for the injection maps
+        private int injectionCounter = 0;
 
-        private final List<Resource> resourceInjections = new LinkedList<Resource>();
+        // Points to the next injection that has not yet been made into the Page
+        private int nextInjectionPosition = 0;
 
         private final UI ui;
 
-        private StyleSheet(UI ui) {
+        private Styles(UI ui) {
             this.ui = ui;
         }
 
@@ -337,24 +336,13 @@ public class Page implements Serializable {
          * @param css
          *            The CSS to inject
          */
-        public void inject(String css) {
+        public void add(String css) {
             if (css == null) {
                 throw new IllegalArgumentException(
                         "Cannot inject null CSS string");
             }
 
-            /*
-             * Check if last injection was the same, in that case ignore it.
-             */
-            if (!stringInjections.isEmpty() && injectedStringPointer > 0) {
-                String lastInjection = stringInjections
-                        .get(injectedStringPointer - 1);
-                if (lastInjection.equals(css.trim())) {
-                    return;
-                }
-            }
-
-            stringInjections.add(css.trim());
+            stringInjections.put(injectionCounter++, css);
             ui.markAsDirty();
         }
 
@@ -364,13 +352,13 @@ public class Page implements Serializable {
          * @param resource
          *            The resource to inject.
          */
-        public void inject(Resource resource) {
+        public void add(Resource resource) {
             if (resource == null) {
                 throw new IllegalArgumentException(
                         "Cannot inject null resource");
             }
 
-            resourceInjections.add(resource);
+            resourceInjections.put(injectionCounter++, resource);
             ui.markAsDirty();
         }
 
@@ -378,37 +366,38 @@ public class Page implements Serializable {
 
             // If full repaint repaint all injections
             if (target.isFullRepaint()) {
-                injectedStringPointer = 0;
-                injectedResourcesPointer = 0;
+                nextInjectionPosition = 0;
             }
 
-            target.startTag("css-injections");
+            if (injectionCounter > nextInjectionPosition) {
 
-            // Paint pending string injections
-            List<String> injections = stringInjections.subList(
-                    injectedStringPointer, stringInjections.size());
+                target.startTag("css-injections");
 
-            for (String css : injections) {
-                target.startTag("css-string");
-                target.addText(css);
-                target.endTag("css-string");
-            }
+                while (injectionCounter > nextInjectionPosition) {
 
-            injectedStringPointer = stringInjections.size();
+                    String stringInjection = stringInjections
+                            .get(nextInjectionPosition);
+                    if (stringInjection != null) {
+                        target.startTag("css-string");
+                        target.addAttribute("id", nextInjectionPosition);
+                        target.addText(stringInjection);
+                        target.endTag("css-string");
+                    }
 
-            // Paint pending resource injections
-            List<Resource> resInjections = resourceInjections.subList(
-                    injectedResourcesPointer, resourceInjections.size());
+                    Resource resourceInjection = resourceInjections
+                            .get(nextInjectionPosition);
+                    if (resourceInjection != null) {
+                        target.startTag("css-resource");
+                        target.addAttribute("id", nextInjectionPosition);
+                        target.addAttribute("url", resourceInjection);
+                        target.endTag("css-resource");
+                    }
 
-            for (Resource res : resInjections) {
-                target.startTag("css-resource");
-                target.addAttribute("url", res);
-                target.endTag("css-resource");
-            }
-
-            target.endTag("css-injections");
+                    nextInjectionPosition++;
+                }
 
-            injectedResourcesPointer = resourceInjections.size();
+                target.endTag("css-injections");
+            }
         }
     }
 
@@ -421,7 +410,7 @@ public class Page implements Serializable {
 
     private JavaScript javaScript;
 
-    private StyleSheet styleSheet;
+    private Styles styles;
 
     /**
      * The current browser location.
@@ -696,11 +685,12 @@ public class Page implements Serializable {
      * 
      * @since 7.1
      */
-    public StyleSheet getStyleSheet() {
-        if (styleSheet == null) {
-            styleSheet = new StyleSheet(uI);
+    public Styles getStyles() {
+
+        if (styles == null) {
+            styles = new Styles(uI);
         }
-        return styleSheet;
+        return styles;
     }
 
     public void paintContent(PaintTarget target) throws PaintException {
@@ -760,8 +750,8 @@ public class Page implements Serializable {
                     location.toString());
         }
 
-        if (styleSheet != null) {
-            styleSheet.paint(target);
+        if (styles != null) {
+            styles.paint(target);
         }
     }
 
diff --git a/uitest/src/com/vaadin/tests/minitutorials/v71beta/CSSInjectWithColorpicker.java b/uitest/src/com/vaadin/tests/minitutorials/v71beta/CSSInjectWithColorpicker.java
index 09c5f91293..e3b8f997e0 100644
--- a/uitest/src/com/vaadin/tests/minitutorials/v71beta/CSSInjectWithColorpicker.java
+++ b/uitest/src/com/vaadin/tests/minitutorials/v71beta/CSSInjectWithColorpicker.java
@@ -5,7 +5,7 @@ import java.util.Arrays;
 import com.vaadin.data.Property.ValueChangeEvent;
 import com.vaadin.data.Property.ValueChangeListener;
 import com.vaadin.server.Page;
-import com.vaadin.server.Page.StyleSheet;
+import com.vaadin.server.Page.Styles;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.shared.ui.MarginInfo;
 import com.vaadin.shared.ui.colorpicker.Color;
@@ -125,10 +125,10 @@ public class CSSInjectWithColorpicker extends UI {
                 Color color = event.getColor();
 
                 // Get the stylesheet of the page
-                StyleSheet styles = Page.getCurrent().getStyleSheet();
+                Styles styles = Page.getCurrent().getStyles();
 
                 // inject the new background color
-                styles.inject(".v-app .v-textarea.text-label { background-color:"
+                styles.add(".v-app .v-textarea.text-label { background-color:"
                         + color.getCSS() + "; }");
             }
         });
@@ -153,10 +153,10 @@ public class CSSInjectWithColorpicker extends UI {
                 Color color = event.getColor();
 
                 // Get the stylesheet of the page
-                StyleSheet styles = Page.getCurrent().getStyleSheet();
+                Styles styles = Page.getCurrent().getStyles();
 
                 // inject the new color as a style
-                styles.inject(".v-app .v-textarea.text-label { color:"
+                styles.add(".v-app .v-textarea.text-label { color:"
                         + color.getCSS() + "; }");
             }
         });
@@ -186,11 +186,11 @@ public class CSSInjectWithColorpicker extends UI {
                 String fontFamily = select.getValue().toString();
 
                 // Get the stylesheet of the page
-                StyleSheet styles = Page.getCurrent().getStyleSheet();
+                Styles styles = Page.getCurrent().getStyles();
 
                 // inject the new font size as a style. We need .v-app to
                 // override Vaadin's default styles here
-                styles.inject(".v-app .v-textarea.text-label { font-family:"
+                styles.add(".v-app .v-textarea.text-label { font-family:"
                         + fontFamily + "; }");
             }
         });
@@ -220,11 +220,11 @@ public class CSSInjectWithColorpicker extends UI {
                 Integer fontSize = (Integer) select.getValue();
 
                 // Get the stylesheet of the page
-                StyleSheet styles = Page.getCurrent().getStyleSheet();
+                Styles styles = Page.getCurrent().getStyles();
 
                 // inject the new font size as a style. We need .v-app to
                 // override Vaadin's default styles here
-                styles.inject(".v-app .v-textarea.text-label { font-size:"
+                styles.add(".v-app .v-textarea.text-label { font-size:"
                         + String.valueOf(fontSize) + "px; }");
             }
         });
diff --git a/uitest/src/com/vaadin/tests/themes/CSSInjectTest.java b/uitest/src/com/vaadin/tests/themes/CSSInjectTest.java
index bedbf47fe2..f4448bf326 100644
--- a/uitest/src/com/vaadin/tests/themes/CSSInjectTest.java
+++ b/uitest/src/com/vaadin/tests/themes/CSSInjectTest.java
@@ -5,7 +5,7 @@ import java.io.InputStream;
 import java.util.UUID;
 
 import com.vaadin.server.Page;
-import com.vaadin.server.Page.StyleSheet;
+import com.vaadin.server.Page.Styles;
 import com.vaadin.server.StreamResource;
 import com.vaadin.shared.ui.label.ContentMode;
 import com.vaadin.tests.components.TestBase;
@@ -19,10 +19,10 @@ public class CSSInjectTest extends TestBase {
     @Override
     protected void setup() {
 
-        final StyleSheet stylesheet = Page.getCurrent().getStyleSheet();
+        final Styles stylesheet = Page.getCurrent().getStyles();
 
         // Inject some resources initially
-        stylesheet.inject(new StreamResource(new StreamResource.StreamSource() {
+        stylesheet.add(new StreamResource(new StreamResource.StreamSource() {
 
             @Override
             public InputStream getStream() {
@@ -44,7 +44,7 @@ public class CSSInjectTest extends TestBase {
 
             @Override
             public void buttonClick(ClickEvent event) {
-                stylesheet.inject(cssToInject.getValue());
+                stylesheet.add(cssToInject.getValue());
                 cssToInject.setValue("");
             }
         });
@@ -58,7 +58,7 @@ public class CSSInjectTest extends TestBase {
 
                         final String css = cssToInject.getValue();
 
-                        stylesheet.inject(new StreamResource(
+                        stylesheet.add(new StreamResource(
                                 new StreamResource.StreamSource() {
 
                                     @Override