URL duplication in mouse cursor tests mean minor typos can cause flakiness
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Jan 2013 22:45:30 +0000 (22:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Jan 2013 22:45:30 +0000 (22:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=104952

Patch by Rick Byers <rbyers@chromium.org> on 2013-01-04
Reviewed by Eric Seidel.

Replace the explicit list of images to preload with code that extracts
URLs from style definitions.  Also move all this image preload code to
a common helper file so that it can easily be used by other tests.

* fast/events/mouse-cursor-image-set.html:
* fast/events/mouse-cursor-multiframecur.html:
* fast/events/mouse-cursor.html:
* fast/js/resources/image-preload-helper.js: Added.
(preloadImagesFromStyle):
(onComplete):
(imagesToLoad):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@138857 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/mouse-cursor-image-set.html
LayoutTests/fast/events/mouse-cursor-multiframecur.html
LayoutTests/fast/events/mouse-cursor.html
LayoutTests/fast/js/resources/image-preload-helper.js [new file with mode: 0644]

index b0e34c2..9df70ad 100644 (file)
@@ -1,3 +1,22 @@
+2013-01-04  Rick Byers  <rbyers@chromium.org>
+
+        URL duplication in mouse cursor tests mean minor typos can cause flakiness
+        https://bugs.webkit.org/show_bug.cgi?id=104952
+
+        Reviewed by Eric Seidel.
+
+        Replace the explicit list of images to preload with code that extracts
+        URLs from style definitions.  Also move all this image preload code to
+        a common helper file so that it can easily be used by other tests.
+
+        * fast/events/mouse-cursor-image-set.html:
+        * fast/events/mouse-cursor-multiframecur.html:
+        * fast/events/mouse-cursor.html:
+        * fast/js/resources/image-preload-helper.js: Added.
+        (preloadImagesFromStyle):
+        (onComplete):
+        (imagesToLoad):
+
 2013-01-04  Xianzhu Wang  <wangxianzhu@chromium.org>
 
         Unreviewd. Fix break caused by using old internals.settings.setEnableCompositingForFixedPosition
index 3e8405c..822146c 100644 (file)
@@ -2,6 +2,7 @@
 <html>
 <head>
 <script src="../js/resources/js-test-pre.js"></script>
+<script src="../js/resources/image-preload-helper.js"></script>
 <style type="text/css">
 </style>
 </head>
 <div id="console"></div>
 <script>
 var imagesLeftToLoad = 0;
-
-function onImageLoad(success, ondone, e) {
-
-    // This debug output is non-deterministic and contains absolute URLs - only show when
-    // not running in DRT
-    if (!window.testRunner)
-        debug( 'Event "' + e.type + '": ' + e.target.src);
-
-    if (!success)
-        testFailed('Got unexpected \'' + e.type + '\' event for image: ' + e.target.src); 
-
-    imagesLeftToLoad--;
-    
-    if (imagesLeftToLoad < 0)
-        testFailed('Got more load/error callback than expected.');
-    
-    if (imagesLeftToLoad == 0)
-        ondone();
-}
+var testContainer = document.getElementById('test-container');
 
 function checkCursors() {
     debug('Checking cursors with device pixel ratio of ' + window.devicePixelRatio);  
@@ -72,9 +55,9 @@ function runTests() {
         // Repeat in high-dpi mode
         testRunner.setBackingScaleFactor(2, function() {
             // Failed images are apparently reset on scale factor change. 
-            ensureImagesLoaded([{ url: 'doesntexist.png', error: true }], function() {
+            loadImages([{ url: 'doesntexist.png', error: true }], function() {
                 checkCursors();
-                document.getElementById('test-container').style.display = 'none';
+                testContainer.style.display = 'none';
                 finishJSTest();
             });
        });     
@@ -83,18 +66,6 @@ function runTests() {
     }
 }
 
-function ensureImagesLoaded(imagesToLoad, callback)
-{
-    imagesLeftToLoad = imagesToLoad.length;
-    for (var i = 0; i < imagesToLoad.length; i++) {
-        var img = new Image();
-        var expectError = imagesToLoad[i].error; 
-        img.addEventListener('load', onImageLoad.bind(undefined, !expectError, callback));
-        img.addEventListener('error', onImageLoad.bind(undefined, expectError, callback));
-        img.src = imagesToLoad[i].url;
-    }
-}
-
 description("Test that mouse cursors are applied correctly.");
 
 if (!window.eventSender) {
@@ -109,14 +80,7 @@ if (window.testRunner) {
 
 // Now wait for each image to load or fail to load before starting tests.
 // Without this we can get null images in the cursors - eg. no known size.
-var imagesToLoad = [
- { url: 'resources/greenbox.png' },
- { url: 'resources/greenbox30.png' },
- { url: 'resources/greenbox200.png' },
- { url: 'resources/greenbox-hotspot5-4.cur' },
- { url: 'resources/greenbox30-hotspot28-3.cur' },
- { url: 'doesntexist.png', error: true } ];
-ensureImagesLoaded(imagesToLoad, runTests);
+preloadImagesFromStyle(testContainer, 6, runTests, /doesntexist/);
 
 </script>
 <script src="../../fast/js/resources/js-test-post.js"></script>
index 256a499..f15242e 100644 (file)
@@ -2,6 +2,7 @@
 <html>
 <head>
 <script src="../js/resources/js-test-pre.js"></script>
+<script src="../js/resources/image-preload-helper.js"></script>
 <style type="text/css">
 </style>
 </head>
 <div id="console"></div>
 <script>
 var imagesLeftToLoad = 0;
-
-function onImageLoad(success, path, e) {
-
-    // This debug output order is non-deterministic - only show when not running in DRT
-    if (!window.testRunner)
-        debug( 'Event "' + e.type + '": ' + path);
-        
-    imagesLeftToLoad--;
-    
-    if (!success) {
-        // Note that on the Mac port, CUR files are loaded using CoreGraphics, and it doesn't
-        // appear to support multi-frame CUR files, so the load will fail in this case.
-        // We explicitly look for this behavior on Mac so that if it ever starts being supported,
-        // we can ensure the behavior is consistent with the other ports.
-        testFailed('Got unexpected \'' + e.type + '\' event for image: ' + path); 
-    }
-
-    if (imagesLeftToLoad == 0)
-        runTests();
-    if (imagesLeftToLoad < 0)
-        testFailed('Got more load/error callback than expected.');
-}
+var testContainer = document.getElementById('test-container');
 
 function runTests() {
     // Can't do anything useful here without eventSender
@@ -59,7 +39,7 @@ function runTests() {
             debug('');
         }
         // This text is redundant with the test output - hide it
-        document.getElementById('test-container').style.display = 'none';
+        testContainer.style.display = 'none';
     }
     
     finishJSTest();
@@ -79,17 +59,7 @@ if (window.testRunner) {
 
 // Now wait for each image to load or fail to load before starting tests.
 // Without this we can get null images in the cursors - eg. no known size.
-var imagesToLoad = [
-    { url: 'resources/greenbox-3frames.cur' } ];
-imagesLeftToLoad = imagesToLoad.length;
-for (var i = 0; i < imagesToLoad.length; i++) {
-    var img = new Image();
-    var expectError = imagesToLoad[i].error; 
-    var url = imagesToLoad[i].url;
-    img.addEventListener('load', onImageLoad.bind(undefined, !expectError, url));
-    img.addEventListener('error', onImageLoad.bind(undefined, expectError, url));
-    img.src = url;
-}
+preloadImagesFromStyle(testContainer, 1, runTests);
 
 </script>
 <script src="../../fast/js/resources/js-test-post.js"></script>
index 9060806..184bfea 100644 (file)
@@ -2,6 +2,7 @@
 <html>
 <head>
 <script src="../js/resources/js-test-pre.js"></script>
+<script src="../js/resources/image-preload-helper.js"></script>
 <style type="text/css">
 </style>
 </head>
@@ -22,8 +23,8 @@
   <div style='cursor: pointer'>Pointer</div>
   <div style='cursor: -webkit-grabbing'>-webkit-grabbing</div>
   <div style='cursor: url(resources/greenbox.png), hand'>Existing 25x25 image</div>
-  <div style='cursor: url(doesntexist.png), pointer'>Invalid URL with fallback to pointer</div>
-  <div style='cursor: url(doesntexist.png), url(resources/greenbox.png), pointer'>Invalid with fallback to 25x25 image</div>
+  <div style='cursor: url(doesntexist_FAIL.png), pointer'>Invalid URL with fallback to pointer</div>
+  <div style='cursor: url(doesntexist_FAIL.png), url(resources/greenbox.png), pointer'>Invalid with fallback to 25x25 image</div>
   <div style='cursor: url(resources/greenbox.png) 0 0, pointer'>Image with explicit hot spot at (0,0)</div>
   <div style='cursor: url(resources/greenbox.png) 20 10, pointer'>Image with explicit hot spot at (20,10)</div>
   <div style='cursor: url(resources/greenbox.png) -1 -1, pointer'>Image with explicit hot spot at (-1,-1)</div>
@@ -33,8 +34,8 @@
   <div style='cursor: url(resources/greenbox-hotspot35-4.cur), pointer'>Image with implicit hot spot outside image at (35,4)</div>
   <div style='cursor: url(resources/onload-image.png), pointer'>Over large image with fallback to pointer</div>
   <div style='cursor: url(#greenbox), pointer'>SVG cursor</div>
-  <div style='cursor: url(mouse-cursor.html), url(unknown-scheme:cursor.png), pointer'>Multiple invalid cursors with fallback to pointer</div>
-  <div style='cursor: url(#nonexistent), pointer'>Nonexistent SVG cursor with fallback to pointer</div>
+  <div style='cursor: url(mouse-cursor_FAIL.html), url(unknown-scheme:cursor_FAIL.png), pointer'>Multiple invalid cursors with fallback to pointer</div>
+  <div style='cursor: url(#nonexistent_FAIL), pointer'>Nonexistent SVG cursor with fallback to pointer</div>
   <div><a href='#'>A link with default cursor</a></div>
   <div style='cursor: wait'><a href='#'>Link with default cursor overriding wait</a></div>
   <div style='cursor: wait'><div style='cursor: doesntexist'>Wait cursor which should not be affected by unknown cursor rule</div></div>
 <br/>
 <div id="console"></div>
 <script>
-var imagesLeftToLoad = 0;
 
-function onImageLoad(success, e) {
-
-    // This debug output is non-deterministic and contains absolute URLs - only show when
-    // not running in DRT
-    if (!window.testRunner)
-        debug( 'Event "' + e.type + '": ' + e.target.src);
+var testContainer = document.getElementById('test-container');
     
-    if (!success)
-        testFailed('Got unexpected \'' + e.type + '\' event for image: ' + e.target.src); 
-        
-    imagesLeftToLoad--;
-    if (imagesLeftToLoad == 0)
-        runTests();
-    if (imagesLeftToLoad < 0)
-        testFailed('Got more load/error callback than expected.');
-}
-
 function runTests() {
     // Can't do anything useful here without eventSender
     if (window.eventSender) {
@@ -81,7 +66,7 @@ function runTests() {
             debug('');
         }
         // This text is redundant with the test output - hide it
-        document.getElementById('test-container').style.display = 'none';
+        testContainer.style.display = 'none';
     }
     
     finishJSTest();
@@ -101,24 +86,7 @@ if (window.testRunner) {
 
 // Now wait for each image to load or fail to load before starting tests.
 // Without this we can get null images in the cursors - eg. no known size.
-var imagesToLoad = [
-    { url: 'resources/greenbox.png' },
-    { url: 'resources/onload-image.png' },
-    { url: 'resources/greenbox-hotspot5-4.cur' },
-    { url: 'resources/greenbox-hotspot35-4.cur' },
-    { url: 'doesntexist.png', error: true },
-    { url: 'unknown-scheme:cursor.png', error: true },
-    { url: 'mouse-cursor.html', error: true },
-    { url: '#greenbox', error: true },
-    { url: '#nonexistent', error: true } ];
-imagesLeftToLoad = imagesToLoad.length;
-for (var i = 0; i < imagesToLoad.length; i++) {
-    var img = new Image();
-    var expectError = imagesToLoad[i].error; 
-    img.addEventListener('load', onImageLoad.bind(undefined, !expectError));
-    img.addEventListener('error', onImageLoad.bind(undefined, expectError));
-    img.src = imagesToLoad[i].url;
-}
+preloadImagesFromStyle(testContainer, 9, runTests, /(#greenbox|_FAIL)/);
 
 </script>
 <script src="../../fast/js/resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/js/resources/image-preload-helper.js b/LayoutTests/fast/js/resources/image-preload-helper.js
new file mode 100644 (file)
index 0000000..27ea17d
--- /dev/null
@@ -0,0 +1,74 @@
+// This is used by several tests to help get images reliably preloaded.
+
+// Given a node, loads all urls specified in style declarations
+// attached to the node or it's decendants.
+// imageCount specifies the number of images we expect to find (to try to add some
+// protection against brittleness due to imperfect url parsing, since other missing a preload
+// will typically result in a test that fails only occasionally).
+// If failPattern is specified, then any url that matches the regex
+// will be expected to fail to load.
+function preloadImagesFromStyle(rootNode, imageCount, onComplete, failPattern) {
+    var basePath = location.href.substring(0, location.href.lastIndexOf('/') + 1);
+    var nodes = rootNode.querySelectorAll('[style]');
+    var imagesToLoad = [];
+    var seenUrls = {};
+    for (var i = 0; i < nodes.length; i++) {
+        var urls = nodes[i].style.cssText.split(/url\w*\(([^)]*)\)/);
+        for (var j = 1; j < urls.length; j += 2) {
+            // Attempt to convert URL to a relative path in order to have deterministic error messages.
+            var url = urls[j];
+            if (url.indexOf(basePath) == 0)
+                url = url.substring(basePath.length);
+
+            var error = false;
+            if (failPattern && failPattern.test(url))
+                error = true;
+            if (url in seenUrls)
+                continue;
+            seenUrls[url] = true;
+            imagesToLoad.push({url: url, error: error});
+        }
+    }
+
+    if (imageCount != imagesToLoad.length) {
+        var msg = 'Found the following ' + imagesToLoad.length + ' images, when expecting ' + imageCount + ': ';
+        for (var i = 0; i < imagesToLoad.length; i++) {
+            msg += '\n' + imagesToLoad[i].url;
+        }
+        testFailed(msg);
+    }
+
+    loadImages(imagesToLoad, onComplete);
+}
+
+// For each object in the given array, attempt to load the image specified by the
+// url property.  If the error property is specified and true, then the load is
+// expected to fail.  Once all loads have completed or failed, onComplete is invoked.
+function loadImages(imagesToLoad, onComplete) {
+
+    var imagesLeftToLoad = imagesToLoad.length;
+
+    function onImageLoad(url, success, e) {
+        // This debug output order is non-deterministic - only show when not running in DRT
+        if (!window.testRunner)
+            debug( 'Event "' + e.type + '": ' + url);
+
+        if (!success)
+            testFailed('Got unexpected \'' + e.type + '\' event for image: ' + url);
+        
+        imagesLeftToLoad--;
+        if (imagesLeftToLoad == 0) {
+            onComplete();
+        }
+        if (imagesLeftToLoad < 0)
+            testFailed('Got more load/error callbacks than expected.');
+    }
+
+    for (var i = 0; i < imagesToLoad.length; i++) {
+        var img = new Image();
+        var expectError = imagesToLoad[i].error;
+        img.addEventListener('load', onImageLoad.bind(undefined, imagesToLoad[i].url, !expectError));
+        img.addEventListener('error', onImageLoad.bind(undefined, imagesToLoad[i].url, !!expectError));
+        img.src = imagesToLoad[i].url;
+    }
+}