Revision: 6403 Author: rj...@google.com Date: Fri Oct 16 15:58:05 2009 Log: Make StyleInjector use Scheduler API.
Patch by: bobv Review by: bruce http://gwt-code-reviews.appspot.com/78816 http://code.google.com/p/google-web-toolkit/source/detail?r=6403 Modified: /trunk/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java /trunk/user/src/com/google/gwt/dom/client/StyleInjector.java /trunk/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java /trunk/user/test/com/google/gwt/dom/client/StyleInjectorTest.java ======================================= --- /trunk/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java Fri Oct 16 15:20:56 2009 +++ /trunk/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java Fri Oct 16 15:58:05 2009 @@ -177,7 +177,8 @@ }-*/; public static void scheduleIncrementalImpl(RepeatingCommand cmd) { - INCREMENTAL_COMMANDS.push(Task.create(cmd)); + // Push repeating commands onto the same initial queue for relative order + DEFERRED_COMMANDS.push(Task.create(cmd)); maybeSchedulePostEventPumpCommands(); } ======================================= --- /trunk/user/src/com/google/gwt/dom/client/StyleInjector.java Tue Sep 8 07:17:09 2009 +++ /trunk/user/src/com/google/gwt/dom/client/StyleInjector.java Fri Oct 16 15:58:05 2009 @@ -19,9 +19,15 @@ import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.core.client.JsArray; import com.google.gwt.core.client.JsArrayInteger; +import com.google.gwt.core.client.JsArrayString; +import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.ScheduledCommand; /** - * Used to add stylesheets to the document. + * Used to add stylesheets to the document. The one-argument versions of + * {...@link #inject}, {...@link #injectAtEnd}, and {...@link #injectAtStart} use + * {...@link Scheduler#scheduleFinally} to minimize the number of individual style + * elements created. */ public class StyleInjector { @@ -120,8 +126,8 @@ * This assertion can fail if the max number of style elements exist * before this module can inject any style elements, so STYLE_ELEMENTS * will be empty. However, the fix would degrade performance for the - * general case. - * TODO(jlabanca): Can we handle this scenario efficiently? + * general case. TODO(jlabanca): Can we handle this scenario + * efficiently? */ assert shortestIdx != -1; @@ -178,6 +184,93 @@ return $doc.styleSheets.length; }-*/; } + + private static final JsArrayString toInject = JavaScriptObject.createArray().cast(); + private static final JsArrayString toInjectAtEnd = JavaScriptObject.createArray().cast(); + private static final JsArrayString toInjectAtStart = JavaScriptObject.createArray().cast(); + + private static ScheduledCommand flusher = new ScheduledCommand() { + public void execute() { + if (needsInjection) { + flush(null); + } + } + }; + + private static boolean needsInjection = false; + + /** + * Add a stylesheet to the document. + * + * @param css the CSS contents of the stylesheet + */ + public static void inject(String css) { + inject(css, false); + } + + /** + * Add a stylesheet to the document. + * + * @param css the CSS contents of the stylesheet + * @param immediate if <code>true</code> the DOM will be updated immediately + * instead of just before returning to the event loop. Using this + * option excessively will decrease performance, especially if used + * with an inject-css-on-init coding pattern + */ + public static void inject(String css, boolean immediate) { + toInject.push(css); + inject(immediate); + } + + /** + * Add stylesheet data to the document as though it were declared after all + * stylesheets previously created by {...@link #inject(String)}. + * + * @param css the CSS contents of the stylesheet + */ + public static void injectAtEnd(String css) { + injectAtEnd(css, false); + } + + /** + * Add stylesheet data to the document as though it were declared after all + * stylesheets previously created by {...@link #inject(String)}. + * + * @param css the CSS contents of the stylesheet + * @param immediate if <code>true</code> the DOM will be updated immediately + * instead of just before returning to the event loop. Using this + * option excessively will decrease performance, especially if used + * with an inject-css-on-init coding pattern + */ + public static void injectAtEnd(String css, boolean immediate) { + toInjectAtEnd.push(css); + inject(immediate); + } + + /** + * Add stylesheet data to the document as though it were declared before all + * stylesheets previously created by {...@link #inject(String)}. + * + * @param css the CSS contents of the stylesheet + */ + public static void injectAtStart(String css) { + injectAtStart(css, false); + } + + /** + * Add stylesheet data to the document as though it were declared before all + * stylesheets previously created by {...@link #inject(String)}. + * + * @param css the CSS contents of the stylesheet + * @param immediate if <code>true</code> the DOM will be updated immediately + * instead of just before returning to the event loop. Using this + * option excessively will decrease performance, especially if used + * with an inject-css-on-init coding pattern + */ + public static void injectAtStart(String css, boolean immediate) { + toInjectAtStart.unshift(css); + inject(immediate); + } /** * Add a stylesheet to the document. The StyleElement returned by this method @@ -185,9 +278,13 @@ * * @param contents the CSS contents of the stylesheet * @return the StyleElement that contains the newly-injected CSS + * @deprecated The returned StyleElement cannot be implemented consistently + * across all browsers */ + @Deprecated public static StyleElement injectStylesheet(String contents) { - return StyleInjectorImpl.IMPL.injectStyleSheet(contents); + toInject.push(contents); + return flush(toInject); } /** @@ -197,9 +294,13 @@ * * @param contents the CSS contents of the stylesheet * @return the StyleElement that contains the newly-injected CSS + * @deprecated The returned StyleElement cannot be implemented consistently + * across all browsers */ + @Deprecated public static StyleElement injectStylesheetAtEnd(String contents) { - return StyleInjectorImpl.IMPL.injectStyleSheetAtEnd(contents); + toInjectAtEnd.push(contents); + return flush(toInjectAtEnd); } /** @@ -209,9 +310,13 @@ * * @param contents the CSS contents of the stylesheet * @return the StyleElement that contains the newly-injected CSS + * @deprecated The returned StyleElement cannot be implemented consistently + * across all browsers */ + @Deprecated public static StyleElement injectStylesheetAtStart(String contents) { - return StyleInjectorImpl.IMPL.injectStyleSheetAtStart(contents); + toInjectAtStart.unshift(contents); + return flush(toInjectAtStart); } /** @@ -224,10 +329,66 @@ * @param style a StyleElement previously-returned from * {...@link #injectStylesheet(String)}. * @param contents the new contents of the stylesheet. + * @deprecated The associated StyleElement cannot be implemented consistently + * across all browsers */ + @Deprecated public static void setContents(StyleElement style, String contents) { StyleInjectorImpl.IMPL.setContents(style, contents); } + + /** + * The <code>which</code> parameter is used to support the deprecated API. + */ + private static StyleElement flush(JavaScriptObject which) { + StyleElement toReturn = null; + StyleElement maybeReturn; + + if (toInjectAtStart.length() != 0) { + String css = toInjectAtStart.join(""); + maybeReturn = StyleInjectorImpl.IMPL.injectStyleSheetAtStart(css); + if (toInjectAtStart == which) { + toReturn = maybeReturn; + } + toInjectAtStart.setLength(0); + } + + if (toInject.length() != 0) { + String css = toInject.join(""); + maybeReturn = StyleInjectorImpl.IMPL.injectStyleSheet(css); + if (toInject == which) { + toReturn = maybeReturn; + } + toInject.setLength(0); + } + + if (toInjectAtEnd.length() != 0) { + String css = toInjectAtEnd.join(""); + maybeReturn = StyleInjectorImpl.IMPL.injectStyleSheetAtEnd(css); + if (toInjectAtEnd == which) { + toReturn = maybeReturn; + } + toInjectAtEnd.setLength(0); + } + + needsInjection = false; + return toReturn; + } + + private static void inject(boolean immediate) { + if (immediate) { + flush(null); + } else { + schedule(); + } + } + + private static void schedule() { + if (!needsInjection) { + needsInjection = true; + Scheduler.get().scheduleFinally(flusher); + } + } /** * Utility class. ======================================= --- /trunk/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java Sat Oct 10 13:51:12 2009 +++ /trunk/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java Fri Oct 16 15:58:05 2009 @@ -902,7 +902,7 @@ sw.indent(); sw.println("if (!injected) {"); sw.indentln("injected = true;"); - sw.indentln(StyleInjector.class.getName() + ".injectStylesheet(getText());"); + sw.indentln(StyleInjector.class.getName() + ".inject(getText());"); sw.indentln("return true;"); sw.println("}"); sw.println("return false;"); ======================================= --- /trunk/user/test/com/google/gwt/dom/client/StyleInjectorTest.java Thu Jul 30 13:47:31 2009 +++ /trunk/user/test/com/google/gwt/dom/client/StyleInjectorTest.java Fri Oct 16 15:58:05 2009 @@ -26,13 +26,16 @@ */ public class StyleInjectorTest extends GWTTestCase { + private static final int TEST_DELAY = 500; + @Override public String getModuleName() { return "com.google.gwt.dom.DOMTest"; } - @DoNotRunWith({Platform.Htmlunit}) - public void testStyleInjector() { + @DoNotRunWith(value = {Platform.Htmlunit}) + @SuppressWarnings("deprecation") + public void testOldMethods() { final DivElement elt = Document.get().createDivElement(); elt.setId("styleInjectorTest"); elt.setInnerHTML("Hello StyleInjector!"); @@ -43,7 +46,7 @@ StyleInjector.injectStylesheetAtEnd("#styleInjectorTest {height: 100px;}"); // We need to allow the document to be redrawn - delayTestFinish(500); + delayTestFinish(TEST_DELAY); DeferredCommand.addCommand(new Command() { public void execute() { @@ -59,8 +62,9 @@ /** * Ensure that the IE createStyleSheet compatibility code is exercised. */ - @DoNotRunWith({Platform.Htmlunit}) - public void testLotsOfStyles() { + @DoNotRunWith(value = {Platform.Htmlunit}) + @SuppressWarnings("deprecation") + public void testOldMethodsWithLotsOfStyles() { StyleElement[] elements = new StyleElement[100]; for (int i = 0, j = elements.length; i < j; i++) { elements[i] = StyleInjector.injectStylesheet("#styleInjectorTest" + i @@ -78,7 +82,7 @@ Document.get().getBody().appendChild(elt); // We need to allow the document to be redrawn - delayTestFinish(500); + delayTestFinish(TEST_DELAY); DeferredCommand.addCommand(new Command() { public void execute() { @@ -89,4 +93,49 @@ } }); } -} + + @DoNotRunWith(value = {Platform.Htmlunit}) + public void testStyleInjectorBatched() { + testStyleInjector("testStyleInjectorBatched", false); + } + + @DoNotRunWith(value = {Platform.Htmlunit}) + public void testStyleInjectorImmediate() { + testStyleInjector("testStyleInjectorImmediate", true); + } + + private void testStyleInjector(String testName, final boolean immediate) { + + final DivElement elt = Document.get().createDivElement(); + elt.setId(testName); + elt.setInnerHTML("Hello"); + Document.get().getBody().appendChild(elt); + + StyleInjector.inject("#" + testName + + " {position: absolute; left: 100px; width: 50px; height 50px;}", + immediate); + StyleInjector.injectAtStart("#" + testName + + " {left: 25px; width: 100px !important;}", immediate); + StyleInjector.injectAtEnd("#" + testName + " {height: 100px;}", immediate); + + Command command = new Command() { + public void execute() { + assertEquals(100, elt.getOffsetLeft()); + assertEquals(100, elt.getClientHeight()); + assertEquals(100, elt.getClientWidth()); + + if (!immediate) { + finishTest(); + } + } + }; + + if (immediate) { + command.execute(); + } else { + DeferredCommand.addCommand(command); + // We need to allow the BatchedCommands to execute + delayTestFinish(TEST_DELAY); + } + } +} --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
