From: Artur <artur@vaadin.com>
Date: Mon, 23 Jan 2017 15:05:01 +0000 (+0200)
Subject: Remove custom preloading support and load scripts using async=false (#8291)
X-Git-Tag: 8.0.0.beta2~51
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3a3e482606db2ae250e3fb4a02695dbacd0aed10;p=vaadin-framework.git

Remove custom preloading support and load scripts using async=false (#8291)

When using async='false' for scripts created by scripts
the execution order is guaranteed to be the same as the order the
script tags are created

Fixes #5339, #3631
---

diff --git a/client/src/main/java/com/vaadin/client/DependencyLoader.java b/client/src/main/java/com/vaadin/client/DependencyLoader.java
index c644cd87db..d217c824b2 100644
--- a/client/src/main/java/com/vaadin/client/DependencyLoader.java
+++ b/client/src/main/java/com/vaadin/client/DependencyLoader.java
@@ -113,12 +113,6 @@ public class DependencyLoader {
         ResourceLoadListener resourceLoadListener = new ResourceLoadListener() {
             @Override
             public void onLoad(ResourceLoadEvent event) {
-                if (dependencies.length() != 0) {
-                    String url = translateVaadinUri(dependencies.shift());
-                    ApplicationConfiguration.startDependencyLoading();
-                    // Load next in chain (hopefully already preloaded)
-                    event.getResourceLoader().loadScript(url, this);
-                }
                 // Call start for next before calling end for current
                 ApplicationConfiguration.endDependencyLoading();
             }
@@ -133,23 +127,10 @@ public class DependencyLoader {
         };
 
         ResourceLoader loader = ResourceLoader.get();
-
-        // Start chain by loading first
-        String url = translateVaadinUri(dependencies.shift());
-        ApplicationConfiguration.startDependencyLoading();
-        loader.loadScript(url, resourceLoadListener);
-
-        if (ResourceLoader.supportsInOrderScriptExecution()) {
-            for (int i = 0; i < dependencies.length(); i++) {
-                String preloadUrl = translateVaadinUri(dependencies.get(i));
-                loader.loadScript(preloadUrl, null);
-            }
-        } else {
-            // Preload all remaining
-            for (int i = 0; i < dependencies.length(); i++) {
-                String preloadUrl = translateVaadinUri(dependencies.get(i));
-                loader.preloadResource(preloadUrl, null);
-            }
+        for (int i = 0; i < dependencies.length(); i++) {
+            ApplicationConfiguration.startDependencyLoading();
+            String preloadUrl = translateVaadinUri(dependencies.get(i));
+            loader.loadScript(preloadUrl, resourceLoadListener);
         }
     }
 
diff --git a/client/src/main/java/com/vaadin/client/ResourceLoader.java b/client/src/main/java/com/vaadin/client/ResourceLoader.java
index 37f6f51a4c..a20daced44 100644
--- a/client/src/main/java/com/vaadin/client/ResourceLoader.java
+++ b/client/src/main/java/com/vaadin/client/ResourceLoader.java
@@ -21,6 +21,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.logging.Logger;
 
 import com.google.gwt.core.client.Duration;
 import com.google.gwt.core.client.GWT;
@@ -30,7 +31,6 @@ import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.LinkElement;
 import com.google.gwt.dom.client.NodeList;
-import com.google.gwt.dom.client.ObjectElement;
 import com.google.gwt.dom.client.ScriptElement;
 import com.google.gwt.user.client.Timer;
 
@@ -38,10 +38,6 @@ import com.google.gwt.user.client.Timer;
  * ResourceLoader lets you dynamically include external scripts and styles on
  * the page and lets you know when the resource has been loaded.
  *
- * You can also preload resources, allowing them to get cached by the browser
- * without being evaluated. This enables downloading multiple resources at once
- * while still controlling in which order e.g. scripts are executed.
- *
  * @author Vaadin Ltd
  * @since 7.0.0
  */
@@ -52,7 +48,6 @@ public class ResourceLoader {
     public static class ResourceLoadEvent {
         private final ResourceLoader loader;
         private final String resourceUrl;
-        private final boolean preload;
 
         /**
          * Creates a new event.
@@ -61,19 +56,14 @@ public class ResourceLoader {
          *            the resource loader that has loaded the resource
          * @param resourceUrl
          *            the url of the loaded resource
-         * @param preload
-         *            true if the resource has only been preloaded, false if
-         *            it's fully loaded
          */
-        public ResourceLoadEvent(ResourceLoader loader, String resourceUrl,
-                boolean preload) {
+        public ResourceLoadEvent(ResourceLoader loader, String resourceUrl) {
             this.loader = loader;
             this.resourceUrl = resourceUrl;
-            this.preload = preload;
         }
 
         /**
-         * Gets the resource loader that has fired this event
+         * Gets the resource loader that has fired this event.
          *
          * @return the resource loader
          */
@@ -90,22 +80,10 @@ public class ResourceLoader {
             return resourceUrl;
         }
 
-        /**
-         * Returns true if the resource has been preloaded, false if it's fully
-         * loaded
-         *
-         * @see ResourceLoader#preloadResource(String, ResourceLoadListener)
-         *
-         * @return true if the resource has been preloaded, false if it's fully
-         *         loaded
-         */
-        public boolean isPreload() {
-            return preload;
-        }
     }
 
     /**
-     * Event listener that gets notified when a resource has been loaded
+     * Event listener that gets notified when a resource has been loaded.
      */
     public interface ResourceLoadListener {
         /**
@@ -143,10 +121,8 @@ public class ResourceLoader {
     private ApplicationConnection connection;
 
     private final Set<String> loadedResources = new HashSet<>();
-    private final Set<String> preloadedResources = new HashSet<>();
 
     private final Map<String, Collection<ResourceLoadListener>> loadListeners = new HashMap<>();
-    private final Map<String, Collection<ResourceLoadListener>> preloadListeners = new HashMap<>();
 
     private final Element head;
 
@@ -196,7 +172,6 @@ public class ResourceLoader {
      * doesn't cause the script to be loaded again, but the listener will still
      * be notified when appropriate.
      *
-     *
      * @param scriptUrl
      *            the url of the script to load
      * @param resourceLoadListener
@@ -204,29 +179,8 @@ public class ResourceLoader {
      */
     public void loadScript(final String scriptUrl,
             final ResourceLoadListener resourceLoadListener) {
-        loadScript(scriptUrl, resourceLoadListener,
-                !supportsInOrderScriptExecution());
-    }
-
-    /**
-     * Load a script and notify a listener when the script is loaded. Calling
-     * this method when the script is currently loading or already loaded
-     * doesn't cause the script to be loaded again, but the listener will still
-     * be notified when appropriate.
-     *
-     *
-     * @param scriptUrl
-     *            url of script to load
-     * @param resourceLoadListener
-     *            listener to notify when script is loaded
-     * @param async
-     *            What mode the script.async attribute should be set to
-     * @since 7.2.4
-     */
-    public void loadScript(final String scriptUrl,
-            final ResourceLoadListener resourceLoadListener, boolean async) {
         final String url = WidgetUtil.getAbsoluteUrl(scriptUrl);
-        ResourceLoadEvent event = new ResourceLoadEvent(this, url, false);
+        ResourceLoadEvent event = new ResourceLoadEvent(this, url);
         if (loadedResources.contains(url)) {
             if (resourceLoadListener != null) {
                 resourceLoadListener.onLoad(event);
@@ -234,31 +188,16 @@ public class ResourceLoader {
             return;
         }
 
-        if (preloadListeners.containsKey(url)) {
-            // Preload going on, continue when preloaded
-            preloadResource(url, new ResourceLoadListener() {
-                @Override
-                public void onLoad(ResourceLoadEvent event) {
-                    loadScript(url, resourceLoadListener);
-                }
-
-                @Override
-                public void onError(ResourceLoadEvent event) {
-                    // Preload failed -> signal error to own listener
-                    if (resourceLoadListener != null) {
-                        resourceLoadListener.onError(event);
-                    }
-                }
-            });
-            return;
-        }
-
         if (addListener(url, resourceLoadListener, loadListeners)) {
+            getLogger().info("Loading script from " + url);
             ScriptElement scriptTag = Document.get().createScriptElement();
             scriptTag.setSrc(url);
             scriptTag.setType("text/javascript");
 
-            scriptTag.setPropertyBoolean("async", async);
+            // async=false causes script injected scripts to be executed in the
+            // injection order. See e.g.
+            // https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script
+            scriptTag.setPropertyBoolean("async", false);
 
             addOnloadHandler(scriptTag, new ResourceLoadListener() {
                 @Override
@@ -275,105 +214,6 @@ public class ResourceLoader {
         }
     }
 
-    /**
-     * The current browser supports script.async='false' for maintaining
-     * execution order for dynamically-added scripts.
-     *
-     * @return Browser supports script.async='false'
-     * @since 7.2.4
-     */
-    public static boolean supportsInOrderScriptExecution() {
-        return BrowserInfo.get().isIE11() || BrowserInfo.get().isEdge();
-    }
-
-    /**
-     * Download a resource and notify a listener when the resource is loaded
-     * without attempting to interpret the resource. When a resource has been
-     * preloaded, it will be present in the browser's cache (provided the HTTP
-     * headers allow caching), making a subsequent load operation complete
-     * without having to wait for the resource to be downloaded again.
-     *
-     * Calling this method when the resource is currently loading, currently
-     * preloading, already preloaded or already loaded doesn't cause the
-     * resource to be preloaded again, but the listener will still be notified
-     * when appropriate.
-     *
-     * @param url
-     *            the url of the resource to preload
-     * @param resourceLoadListener
-     *            the listener that will get notified when the resource is
-     *            preloaded
-     */
-    public void preloadResource(String url,
-            ResourceLoadListener resourceLoadListener) {
-        url = WidgetUtil.getAbsoluteUrl(url);
-        ResourceLoadEvent event = new ResourceLoadEvent(this, url, true);
-        if (loadedResources.contains(url) || preloadedResources.contains(url)) {
-            // Already loaded or preloaded -> just fire listener
-            if (resourceLoadListener != null) {
-                resourceLoadListener.onLoad(event);
-            }
-            return;
-        }
-
-        if (addListener(url, resourceLoadListener, preloadListeners)
-                && !loadListeners.containsKey(url)) {
-            // Inject loader element if this is the first time this is preloaded
-            // AND the resources isn't already being loaded in the normal way
-
-            final Element element = getPreloadElement(url);
-            addOnloadHandler(element, new ResourceLoadListener() {
-                @Override
-                public void onLoad(ResourceLoadEvent event) {
-                    fireLoad(event);
-                    Document.get().getBody().removeChild(element);
-                }
-
-                @Override
-                public void onError(ResourceLoadEvent event) {
-                    fireError(event);
-                    Document.get().getBody().removeChild(element);
-                }
-            }, event);
-
-            Document.get().getBody().appendChild(element);
-        }
-    }
-
-    private static Element getPreloadElement(String url) {
-        /*-
-         * TODO
-         * In Chrome, FF:
-         * <object> does not fire event if resource is 404 -> eternal spinner.
-         * <img> always fires onerror -> no way to know if it loaded -> eternal spinner
-         * <script type="text/javascript> fires, but also executes -> not preloading
-         * <script type="text/cache"> does not fire events
-         *  XHR not tested - should work, probably causes other issues
-         -*/
-        if (BrowserInfo.get().isIE()) {
-            // If ie11+ for some reason gets a preload request
-            if (BrowserInfo.get().getBrowserMajorVersion() >= 11) {
-                throw new RuntimeException(
-                        "Browser doesn't support preloading with text/cache");
-            }
-            ScriptElement element = Document.get().createScriptElement();
-            element.setSrc(url);
-            element.setType("text/cache");
-            return element;
-        } else {
-            ObjectElement element = Document.get().createObjectElement();
-            element.setData(url);
-            if (BrowserInfo.get().isChrome()) {
-                element.setType("text/cache");
-            } else {
-                element.setType("text/plain");
-            }
-            element.setHeight("0px");
-            element.setWidth("0px");
-            return element;
-        }
-    }
-
     /**
      * Adds an onload listener to the given element, which should be a link or a
      * script tag. The listener is called whenever loading is complete or an
@@ -424,7 +264,7 @@ public class ResourceLoader {
     public void loadStylesheet(final String stylesheetUrl,
             final ResourceLoadListener resourceLoadListener) {
         final String url = WidgetUtil.getAbsoluteUrl(stylesheetUrl);
-        final ResourceLoadEvent event = new ResourceLoadEvent(this, url, false);
+        final ResourceLoadEvent event = new ResourceLoadEvent(this, url);
         if (loadedResources.contains(url)) {
             if (resourceLoadListener != null) {
                 resourceLoadListener.onLoad(event);
@@ -432,26 +272,8 @@ public class ResourceLoader {
             return;
         }
 
-        if (preloadListeners.containsKey(url)) {
-            // Preload going on, continue when preloaded
-            preloadResource(url, new ResourceLoadListener() {
-                @Override
-                public void onLoad(ResourceLoadEvent event) {
-                    loadStylesheet(url, resourceLoadListener);
-                }
-
-                @Override
-                public void onError(ResourceLoadEvent event) {
-                    // Preload failed -> signal error to own listener
-                    if (resourceLoadListener != null) {
-                        resourceLoadListener.onError(event);
-                    }
-                }
-            });
-            return;
-        }
-
         if (addListener(url, resourceLoadListener, loadListeners)) {
+            getLogger().info("Loading style sheet from " + url);
             LinkElement linkElement = Document.get().createLinkElement();
             linkElement.setRel("stylesheet");
             linkElement.setType("text/css");
@@ -533,12 +355,12 @@ public class ResourceLoader {
                     if (rules === undefined) {
                         rules = sheet.rules;
                     }
-
+    
                     if (rules === null) {
                         // Style sheet loaded, but can't access length because of XSS -> assume there's something there
                         return 1;
                     }
-
+    
                     // Return length so we can distinguish 0 (probably 404 error) from normal case.
                     return rules.length;
                 } catch (err) {
@@ -568,14 +390,8 @@ public class ResourceLoader {
     private void fireError(ResourceLoadEvent event) {
         String resource = event.getResourceUrl();
 
-        Collection<ResourceLoadListener> listeners;
-        if (event.isPreload()) {
-            // Also fire error for load listeners
-            fireError(new ResourceLoadEvent(this, resource, false));
-            listeners = preloadListeners.remove(resource);
-        } else {
-            listeners = loadListeners.remove(resource);
-        }
+        Collection<ResourceLoadListener> listeners = loadListeners
+                .remove(resource);
         if (listeners != null && !listeners.isEmpty()) {
             for (ResourceLoadListener listener : listeners) {
                 if (listener != null) {
@@ -587,19 +403,9 @@ public class ResourceLoader {
 
     private void fireLoad(ResourceLoadEvent event) {
         String resource = event.getResourceUrl();
-        Collection<ResourceLoadListener> listeners;
-        if (event.isPreload()) {
-            preloadedResources.add(resource);
-            listeners = preloadListeners.remove(resource);
-        } else {
-            if (preloadListeners.containsKey(resource)) {
-                // Also fire preload events for potential listeners
-                fireLoad(new ResourceLoadEvent(this, resource, true));
-            }
-            preloadedResources.remove(resource);
-            loadedResources.add(resource);
-            listeners = loadListeners.remove(resource);
-        }
+        Collection<ResourceLoadListener> listeners = loadListeners
+                .remove(resource);
+        loadedResources.add(resource);
         if (listeners != null && !listeners.isEmpty()) {
             for (ResourceLoadListener listener : listeners) {
                 if (listener != null) {
@@ -609,4 +415,7 @@ public class ResourceLoader {
         }
     }
 
+    private static Logger getLogger() {
+        return Logger.getLogger(ResourceLoader.class.getName());
+    }
 }