Bug 1090555 - Fix visited link test in test_animations_omta.html to wait for visited link coloring properly. r=birtles
authorL. David Baron <dbaron@dbaron.org>
Tue, 24 Mar 2015 19:13:47 -0700
changeset 264327 d96242aad96d97d26030c59b27a34152931afcec
parent 264326 9d7079578e3dc8be2c54104b74b5b1422ae3c0bf
child 264328 cd1ef82e74a4a868caba52c9379b76a324c532bd
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1090555
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1090555 - Fix visited link test in test_animations_omta.html to wait for visited link coloring properly. r=birtles This patch contains two changes: (1) The addition of refVisitedLink and the use of waitForVisitedLinkColoring() on it. (2) Changing the URL of the visited lisks (both visitedLink and refVisitedLink) from "" to window.top.location.href, since the former doesn't work for Android mochitests while it does work on Linux mochitest-e10s. I tested locally that without the patch I get the failures, and with the patch the failures go away, using: ./mach mochitest-plain --e10s --setpref layers.acceleration.force-enabled=true --setpref layers.offmainthreadcomposition.async-animations=true layout/style/test/test_animations_omta.html Further, when running (and passing), I checked that waitForVisitedLinkColoring() does go through one setTimeout cycle. Also, I tested that if I effectively revert https://hg.mozilla.org/mozilla-central/rev/d13154302d77 by changing the third parameter to the GetContext call in nsStyleSet::ResolveStyleWithReplacement to be nullptr instead of visitedRuleNode, I get the failure: TEST-UNEXPECTED-FAIL | layout/style/test/test_animations_omta.html | visited link background color after animation-only flush - got rgb(255, 255, 0), expected rgb(0, 0, 255) which confirms that the test is still testing what it was designed to test.
layout/style/test/animation_utils.js
layout/style/test/mochitest.ini
layout/style/test/test_animations_omta.html
--- a/layout/style/test/animation_utils.js
+++ b/layout/style/test/animation_utils.js
@@ -671,8 +671,25 @@ function waitForPaints() {
 }
 
 // As with waitForPaints but also flushes pending style changes before waiting
 function waitForPaintsFlushed() {
   return new Promise(function(resolve, reject) {
     waitForAllPaintsFlushed(resolve);
   });
 }
+
+function waitForVisitedLinkColoring(visitedLink, waitProperty, waitValue) {
+  function checkLink(resolve) {
+    if (SpecialPowers.DOMWindowUtils
+          .getVisitedDependentComputedStyle(visitedLink, "", waitProperty) ==
+        waitValue) {
+      // Our link has been styled as visited.  Resolve.
+      resolve(true);
+    } else {
+      // Our link is not yet styled as visited.  Poll for completion.
+      setTimeout(checkLink, 0, resolve);
+    }
+  }
+  return new Promise(function(resolve, reject) {
+    checkLink(resolve);
+  });
+}
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -32,17 +32,16 @@ support-files =
   xbl_bindings.xml
 generated-files = css_properties.js
 
 [test_acid3_test46.html]
 [test_all_shorthand.html]
 [test_animations.html]
 skip-if = toolkit == 'android'
 [test_animations_omta.html]
-skip-if = buildapp == 'mulet'
 [test_animations_omta_start.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # bug 1041017
 [test_animations_pausing.html]
 [test_any_dynamic.html]
 [test_at_rule_parse_serialize.html]
 [test_bug73586.html]
 [test_bug74880.html]
 [test_bug98997.html]
--- a/layout/style/test/test_animations_omta.html
+++ b/layout/style/test/test_animations_omta.html
@@ -147,18 +147,18 @@ https://bugzilla.mozilla.org/show_bug.cg
 
     .target {
       /* The animation target needs geometry in order to qualify for OMTA */
       width: 100px;
       height: 100px;
       background-color: white;
     }
 
-    #visitedLink:link { background-color: yellow }
-    #visitedLink:visited { background-color: blue }
+    .visitedLink:link { background-color: yellow }
+    .visitedLink:visited { background-color: blue }
 
     @keyframes opacitymid {
       0% { opacity: 0.2 }
       100% { opacity: 0.8 }
     }
   </style>
 </head>
 <body>
@@ -2017,64 +2017,70 @@ addAsyncAnimTest(function *() {
                "empty keyframes rule");
 
   done_div();
 });
 
 // Bug 996796 patch 12 - test for correct visited styles during
 // animation-only style flush.
 addAsyncAnimTest(function *() {
+  var isb2g = SpecialPowers.Services.appinfo.name == "B2G";
+  if (isb2g) {
+    todo(false, "no global history on B2G; can't run test");
+    return;
+  }
+
   var div1 = document.createElement("div");
   div1.classList.add("target");
   div1.style.height = "10px";
   div1.style.animation = "anim2 linear 1s";
 
   var visitedLink = document.createElement("a");
-  visitedLink.setAttribute("href", "");
-  visitedLink.setAttribute("id", "visitedLink");
+  visitedLink.setAttribute("href", window.top.location.href);
+  visitedLink.classList.add("visitedLink");
   visitedLink.classList.add("target");
   visitedLink.style.display = "block";
   visitedLink.style.height = "10px";
   visitedLink.style.animation = "anim2 linear 1s";
 
+  var refVisitedLink = document.createElement("a");
+  refVisitedLink.setAttribute("href", window.top.location.href);
+  refVisitedLink.classList.add("visitedLink");
+
   gDisplay.appendChild(div1);
   gDisplay.appendChild(visitedLink);
+  gDisplay.appendChild(refVisitedLink);
 
-  // Wait for animations to start and for visited link coloring.
+  // Wait for visited link coloring.
+  yield waitForVisitedLinkColoring(refVisitedLink,
+                                   "background-color", "rgb(0, 0, 255)");
+
+  // Wait for animations to start.
   yield waitForPaintsFlushed();
 
   var bgColor = SpecialPowers.DOMWindowUtils
     .getVisitedDependentComputedStyle(visitedLink, "", "background-color");
-  var isb2g = SpecialPowers.Services.appinfo.name == "B2G";
-  // No global history in B2G.
-  (isb2g ? todo_is : is)(bgColor, "rgb(0, 0, 255)",
-                         "initial visited link background color");
-
-  if (isb2g) {
-    // The above failure makes the rest of the test pointless.
-    div1.remove();
-    visitedLink.remove();
-    return;
-  }
+  is(bgColor, "rgb(0, 0, 255)", "initial visited link background color");
 
   advance_clock(250);
 
   // Trigger a style change on div1 that will force us to do a miniflush,
   // but which will not trigger a style change on visitedLink.
   div1.style.color = "blue";
   advance_clock(250);
 
   bgColor = SpecialPowers.DOMWindowUtils
     .getVisitedDependentComputedStyle(visitedLink, "", "background-color");
 
   is(bgColor, "rgb(0, 0, 255)",
      "visited link background color after animation-only flush");
 
   div1.remove();
   visitedLink.remove();
+  refVisitedLink.remove();
 });
 
 /*
  * Bug 962594 - Turn off CSS animations when the element is display:none, or
  * is in a display:none subtree.
  */
 
 // Check that it works if the animated element itself becomes display:none