Backed out 6 changesets (bug 1328509) for Windows 7 failures in test_visited_reftests.html

Backed out changeset 8388d8c76fed (bug 1328509) Backed out changeset 182a88607e08 (bug 1328509) Backed out changeset 008efac7764b (bug 1328509) Backed out changeset c2ce10b8bc37 (bug 1328509) Backed out changeset 65bbc73c87aa (bug 1328509) Backed out changeset e20d3d5170c2 (bug 1328509) MozReview-Commit-ID: BSmqdFP1Zom
2017-05-24 22:12:53 -07:00 · 2017-05-24 22:12:53 -07:00 · 30a91cb819
--- a/layout/reftests/css-visited/color-on-visited-attr-1.html
+++ b/layout/reftests/css-visited/color-on-visited-attr-1.html
@ -1,14 +0,0 @@
-<!DOCTYPE HTML>
-<title>Test for privacy restrictions on :visited (Bug 147777)</title>
-<style type="text/css">
-a { text-decoration: none }
-:link { color: fuchsia; }
-[bob]:visited { color: purple; }
-</style>
-<a href="visited-page.html">link</a>
-<script>
-  // Attempt to set attribute _after_ the visited state takes effect
-  setTimeout(() => {
-    document.querySelector("a").setAttribute("bob", "");
-  }, 100);
-</script>
--- a/layout/reftests/css-visited/color-on-visited-focus-1.html
+++ b/layout/reftests/css-visited/color-on-visited-focus-1.html
@ -1,14 +0,0 @@
-<!DOCTYPE HTML>
-<title>Test for privacy restrictions on :visited (Bug 147777)</title>
-<style type="text/css">
-a { text-decoration: none; outline: none; }
-:link { color: fuchsia; }
-:focus:visited { color: purple; }
-</style>
-<a href="visited-page.html">link</a>
-<script>
-  // Attempt to set focus _after_ the visited state takes effect
-  setTimeout(() => {
-    document.querySelector("a").focus();
-  }, 100);
-</script>
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@ -410,9 +410,6 @@ SERVO_BINDING_FUNC(Servo_ComputedValues_Inherit, ServoComputedValuesStrong,
                   RawServoStyleSetBorrowed set,
                   ServoComputedValuesBorrowedOrNull parent_style,
                   mozilla::InheritTarget target)
-SERVO_BINDING_FUNC(Servo_ComputedValues_GetVisitedStyle,
-                   ServoComputedValuesStrong,
-                   ServoComputedValuesBorrowed values)

 // Initialize Servo components. Should be called exactly once at startup.
 SERVO_BINDING_FUNC(Servo_Initialize, void,
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@ -216,54 +216,10 @@ ServoStyleSet::GetContext(already_AddRefed<ServoComputedValues> aComputedValues,
                          CSSPseudoElementType aPseudoType,
                          Element* aElementForAnimation)
 {
-  bool isLink = false;
-  bool isVisitedLink = false;
-  // If we need visited styles for callers where `aElementForAnimation` is null,
-  // we can precompute these and pass them as flags, similar to nsStyleSet.cpp.
-  if (aElementForAnimation) {
-    isLink = nsCSSRuleProcessor::IsLink(aElementForAnimation);
-    isVisitedLink = nsCSSRuleProcessor::GetContentState(aElementForAnimation)
-                                       .HasState(NS_EVENT_STATE_VISITED);
-  }
+  // XXXbholley: nsStyleSet does visited handling here.

-  RefPtr<ServoComputedValues> computedValues = Move(aComputedValues);
-  RefPtr<ServoComputedValues> visitedComputedValues =
-    Servo_ComputedValues_GetVisitedStyle(computedValues).Consume();
-
-  // If `visitedComputedValues` is non-null, then there was a relevant link and
-  // visited styles were computed.  This corresponds to the cases where Gecko's
-  // style system produces `aVisitedRuleNode`.
-  // Set up `parentIfVisited` depending on whether our parent context has a
-  // a visited style.  If it doesn't but we do have visited styles, use the
-  // regular parent context for visited.
-  nsStyleContext *parentIfVisited =
-    aParentContext ? aParentContext->GetStyleIfVisited() : nullptr;
-  if (!parentIfVisited) {
-    if (visitedComputedValues) {
-      parentIfVisited = aParentContext;
-    }
-  }
-
-  // The true visited state of the relevant link is used to decided whether
-  // visited styles should be consulted for all visited dependent properties.
-  bool relevantLinkVisited = isLink ? isVisitedLink :
-    (aParentContext && aParentContext->RelevantLinkVisited());
-
-  RefPtr<nsStyleContext> result =
-    NS_NewStyleContext(aParentContext, mPresContext, aPseudoTag, aPseudoType,
-                       computedValues.forget());
-
-  if (visitedComputedValues) {
-    RefPtr<nsStyleContext> resultIfVisited =
-      NS_NewStyleContext(parentIfVisited, mPresContext, aPseudoTag, aPseudoType,
-                         visitedComputedValues.forget());
-    resultIfVisited->SetIsStyleIfVisited();
-    result->SetStyleIfVisited(resultIfVisited.forget());
-
-    if (relevantLinkVisited) {
-      result->AddStyleBit(NS_STYLE_RELEVANT_LINK_VISITED);
-    }
-  }
+  RefPtr<nsStyleContext> result = NS_NewStyleContext(aParentContext, mPresContext, aPseudoTag,
+                                                     aPseudoType, Move(aComputedValues));

  // Set the body color on the pres context. See nsStyleSet::GetContext
  if (aElementForAnimation &&
--- a/layout/style/crashtests/content-only-on-link-before.html
+++ b/layout/style/crashtests/content-only-on-link-before.html
@ -1,4 +1,5 @@
 <!DOCTYPE HTML>
+<title>crashtest for NS_ABORT_IF_FALSE during development of 147777</title>
 <style type="text/css">
 :link::before { content: "link: " }
 </style>
--- a/layout/style/crashtests/content-only-on-visited-before.html
+++ b/layout/style/crashtests/content-only-on-visited-before.html
@ -1,5 +0,0 @@
-<!DOCTYPE HTML>
-<style type="text/css">
-:visited::before { content: "link: " }
-</style>
-<a href="http://www.example.com/">example</a>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@ -1,4 +1,5 @@
 load 105619-1.html
+load 147777-1.html
 load 187671-1.html
 load 192408-1.html
 load 285727-1.html
@ -171,5 +172,3 @@ HTTP load 1333001-1.html
 pref(dom.animations-api.core.enabled,true) load 1340344.html
 load 1342316-1.html
 load 1356601-1.html
-load content-only-on-link-before.html
-load content-only-on-visited-before.html
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@ -1234,7 +1234,7 @@ nsCSSRuleProcessor::GetWindowsThemeIdentifier()

 /* static */
 EventStates
-nsCSSRuleProcessor::GetContentState(Element* aElement, bool aUsingPrivateBrowsing)
+nsCSSRuleProcessor::GetContentState(Element* aElement, const TreeMatchContext& aTreeMatchContext)
 {
  EventStates state = aElement->StyleState();

@ -1245,32 +1245,13 @@ nsCSSRuleProcessor::GetContentState(Element* aElement, bool aUsingPrivateBrowsin
  if (state.HasState(NS_EVENT_STATE_VISITED) &&
      (!gSupportVisitedPseudo ||
       aElement->OwnerDoc()->IsBeingUsedAsImage() ||
-       aUsingPrivateBrowsing)) {
+       aTreeMatchContext.mUsingPrivateBrowsing)) {
    state &= ~NS_EVENT_STATE_VISITED;
    state |= NS_EVENT_STATE_UNVISITED;
  }
  return state;
 }

-/* static */
-EventStates
-nsCSSRuleProcessor::GetContentState(Element* aElement, const TreeMatchContext& aTreeMatchContext)
-{
-  return nsCSSRuleProcessor::GetContentState(
-    aElement,
-    aTreeMatchContext.mUsingPrivateBrowsing
-  );
-}
-
-/* static */
-EventStates
-nsCSSRuleProcessor::GetContentState(Element* aElement)
-{
-  nsILoadContext* loadContext = aElement->OwnerDoc()->GetLoadContext();
-  bool usingPrivateBrowsing = loadContext && loadContext->UsePrivateBrowsing();
-  return nsCSSRuleProcessor::GetContentState(aElement, usingPrivateBrowsing);
-}
-
 /* static */
 bool
 nsCSSRuleProcessor::IsLink(const Element* aElement)
@ -1283,33 +1264,31 @@ nsCSSRuleProcessor::IsLink(const Element* aElement)
 EventStates
 nsCSSRuleProcessor::GetContentStateForVisitedHandling(
                     Element* aElement,
+                     const TreeMatchContext& aTreeMatchContext,
                     nsRuleWalker::VisitedHandlingType aVisitedHandling,
                     bool aIsRelevantLink)
 {
-  // It's unnecessary to call GetContentState() here (which may flip visited to
-  // unvisited) since this function will remove both unvisited and visited if
-  // either is set and produce a new value.
-  EventStates state = aElement->StyleState();
-  if (state.HasAtLeastOneOfStates(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED)) {
+  EventStates contentState = GetContentState(aElement, aTreeMatchContext);
+  if (contentState.HasAtLeastOneOfStates(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED)) {
    MOZ_ASSERT(IsLink(aElement), "IsLink() should match state");
-    state &= ~(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED);
+    contentState &= ~(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED);
    if (aIsRelevantLink) {
      switch (aVisitedHandling) {
        case nsRuleWalker::eRelevantLinkUnvisited:
-          state |= NS_EVENT_STATE_UNVISITED;
+          contentState |= NS_EVENT_STATE_UNVISITED;
          break;
        case nsRuleWalker::eRelevantLinkVisited:
-          state |= NS_EVENT_STATE_VISITED;
+          contentState |= NS_EVENT_STATE_VISITED;
          break;
        case nsRuleWalker::eLinksVisitedOrUnvisited:
-          state |= NS_EVENT_STATE_UNVISITED | NS_EVENT_STATE_VISITED;
+          contentState |= NS_EVENT_STATE_UNVISITED | NS_EVENT_STATE_VISITED;
          break;
      }
    } else {
-      state |= NS_EVENT_STATE_UNVISITED;
+      contentState |= NS_EVENT_STATE_UNVISITED;
    }
  }
-  return state;
+  return contentState;
 }

 /**
@ -1625,6 +1604,7 @@ StateSelectorMatches(Element* aElement,
    EventStates contentState =
      nsCSSRuleProcessor::GetContentStateForVisitedHandling(
                                   aElement,
+                                   aTreeMatchContext,
                                   aTreeMatchContext.VisitedHandling(),
                                   aNodeMatchContext.mIsRelevantLink);
    if (!contentState.HasAtLeastOneOfStates(aStatesToCheck)) {
--- a/layout/style/nsCSSRuleProcessor.h
+++ b/layout/style/nsCSSRuleProcessor.h
@ -102,20 +102,16 @@ public:
   * Helper to get the content state for a content node.  This may be
   * slightly adjusted from IntrinsicState().
   */
-  static mozilla::EventStates GetContentState(
-                                mozilla::dom::Element* aElement,
-                                bool aUsingPrivateBrowsing);
  static mozilla::EventStates GetContentState(
                                mozilla::dom::Element* aElement,
                                const TreeMatchContext& aTreeMatchContext);
-  static mozilla::EventStates GetContentState(
-                                mozilla::dom::Element* aElement);

  /*
   * Helper to get the content state for :visited handling for an element
   */
  static mozilla::EventStates GetContentStateForVisitedHandling(
             mozilla::dom::Element* aElement,
+             const TreeMatchContext& aTreeMatchContext,
             nsRuleWalker::VisitedHandlingType aVisitedHandling,
             bool aIsRelevantLink);

--- a/layout/style/nsHTMLStyleSheet.cpp
+++ b/layout/style/nsHTMLStyleSheet.cpp
@ -308,6 +308,7 @@ nsHTMLStyleSheet::RulesMatching(ElementRuleProcessorData* aData)
        EventStates state =
          nsCSSRuleProcessor::GetContentStateForVisitedHandling(
                                  aData->mElement,
+                                  aData->mTreeMatchContext,
                                  aData->mTreeMatchContext.VisitedHandling(),
                                  // If the node being matched is a link,
                                  // it's the relevant link.
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@ -1166,9 +1166,6 @@ nsStyleContext::CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
    // Presume a difference.
    hint |= nsChangeHint_RepaintFrame;
  } else if (thisVis && !NS_IsHintSubset(nsChangeHint_RepaintFrame, hint)) {
-    // Bug 1364484: Update comments here and potentially remove the assertion
-    // below once we return a non-null visited context in CalcStyleDifference
-    // using Servo values.  The approach is becoming quite similar to Gecko.
    // We'll handle visited style differently in servo. Assert against being
    // in the parallel traversal to avoid static analysis hazards when calling
    // StyleFoo() below.
@ -1252,11 +1249,8 @@ public:
  }

  nsStyleContext* GetStyleIfVisited() {
-    // Bug 1364484: Figure out what to do here for Stylo visited values.  We can
-    // get the visited computed values:
-    // RefPtr<ServoComputedValues> visitedComputedValues =
-    //   Servo_ComputedValues_GetVisitedStyle(mComputedValues).Consume();
-    // But what's the best way to create the nsStyleContext?
+    // XXXbholley: This is wrong. Need to implement to get visited handling
+    // corrrect!
    return nullptr;
  }

--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@ -274,7 +274,7 @@ public:
  nsStyleContext* GetStyleIfVisited() const
    { return mStyleIfVisited; }

-  // To be called only from nsStyleSet / ServoStyleSet.
+  // To be called only from nsStyleSet.
  void SetStyleIfVisited(already_AddRefed<nsStyleContext> aStyleIfVisited)
  {
    MOZ_ASSERT(!IsStyleIfVisited(), "this context is not visited data");
@ -288,15 +288,13 @@ public:
               "other context does not have visited data");
    NS_ASSERTION(GetStyleIfVisited()->GetPseudo() == GetPseudo(),
                 "pseudo tag mismatch");
-    if (GetParentAllowServo() && GetParentAllowServo()->GetStyleIfVisited()) {
-      NS_ASSERTION(GetStyleIfVisited()->GetParentAllowServo() ==
-                     GetParentAllowServo()->GetStyleIfVisited() ||
-                   GetStyleIfVisited()->GetParentAllowServo() ==
-                     GetParentAllowServo(),
+    if (GetParent() && GetParent()->GetStyleIfVisited()) {
+      NS_ASSERTION(GetStyleIfVisited()->GetParent() ==
+                     GetParent()->GetStyleIfVisited() ||
+                   GetStyleIfVisited()->GetParent() == GetParent(),
                   "parent mismatch");
    } else {
-      NS_ASSERTION(GetStyleIfVisited()->GetParentAllowServo() ==
-                     GetParentAllowServo(),
+      NS_ASSERTION(GetStyleIfVisited()->GetParent() == GetParent(),
                   "parent mismatch");
    }
  }
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@ -313,14 +313,14 @@ support-files = support/external-variable-url.css
 [test_video_object_fit.html]
 [test_viewport_units.html]
 [test_visited_image_loading.html]
-skip-if = toolkit == 'android' # TIMED_OUT for android
+skip-if = (toolkit == 'android' || stylo) # TIMED_OUT for android, timeout bug 1328511 for stylo
 [test_visited_image_loading_empty.html]
-skip-if = toolkit == 'android' # TIMED_OUT for android
+skip-if = (toolkit == 'android' || stylo) # TIMED_OUT for android, timeout bug 1328511 for stylo
 [test_visited_lying.html]
-skip-if = toolkit == 'android' # TIMED_OUT for android
+skip-if = (toolkit == 'android' || stylo) # TIMED_OUT for android, timeout bug 1328511 for stylo
 [test_visited_pref.html]
-skip-if = toolkit == 'android' # TIMED_OUT for android
+skip-if = (toolkit == 'android' || stylo) # TIMED_OUT for android, timeout bug 1328511 for stylo
 [test_visited_reftests.html]
-skip-if = toolkit == 'android' # TIMED_OUT for android
+skip-if = (toolkit == 'android' || stylo) # TIMED_OUT for android, timeout bug 1328511 for stylo
 [test_webkit_device_pixel_ratio.html]
 [test_webkit_flex_display.html]
--- a/layout/style/test/moz.build
+++ b/layout/style/test/moz.build
@ -59,9 +59,7 @@ TEST_HARNESS_FILES.testing.mochitest.tests.layout.style.test['css-visited'] += [
    '/layout/reftests/css-visited/color-on-text-decoration-1.html',
    '/layout/reftests/css-visited/color-on-visited-1-ref.html',
    '/layout/reftests/css-visited/color-on-visited-1.html',
-    '/layout/reftests/css-visited/color-on-visited-attr-1.html',
    '/layout/reftests/css-visited/color-on-visited-before-1.html',
-    '/layout/reftests/css-visited/color-on-visited-focus-1.html',
    '/layout/reftests/css-visited/column-rule-1-notref.html',
    '/layout/reftests/css-visited/column-rule-1-ref.html',
    '/layout/reftests/css-visited/column-rule-1.html',
--- a/layout/style/test/stylo-failures.md
+++ b/layout/style/test/stylo-failures.md
@ -143,13 +143,6 @@ to mochitest command.
 * test_author_specified_style.html: support serializing color as author specified bug 1348165 [27]
 * browser_newtab_share_rule_processors.js: agent style sheet sharing [1]
 * test_default_computed_style.html: getDefaultComputedStyle bug 1366157 [1]
-* :visited support (bug 1328509)
-  * test_visited_reftests.html `selector-descendant-2.xhtml` [2]
-  * ... `selector-child-2.xhtml` [2]
-  * ... `color-on-bullets-1.html` [2]
-  * ... `inherit-keyword-1.xhtml` [2]
-  * ... `mathml-links.html` [2]
-  * ... `caret-color-on-visited-1.html` [2]

 ## Assertions

--- a/layout/style/test/test_visited_reftests.html
+++ b/layout/style/test/test_visited_reftests.html
@ -38,7 +38,6 @@ var gTests = [
  "== color-on-link-before-1.html color-on-link-1-ref.html",
  "== color-on-visited-1.html color-on-visited-1-ref.html",
  "== color-on-visited-before-1.html color-on-visited-1-ref.html",
-  "needs-focus == color-on-visited-focus-1.html color-on-visited-1-ref.html",
  "!= content-color-on-link-before-1-ref.html content-color-on-visited-before-1-ref.html",
  "== content-color-on-link-before-1.html content-color-on-link-before-1-ref.html",
  "== content-color-on-visited-before-1.html content-color-on-visited-before-1-ref.html",
@ -87,12 +86,6 @@ var gTests = [
  "== mathml-links.html mathml-links-ref.html",
 ];

-let isStylo = SpecialPowers.getBoolPref("layout.css.servo.enabled");
-if (isStylo) {
-  // Bug 1366908: Gecko misses this case currently, but it seems unintentional
-  gTests.push("== color-on-visited-attr-1.html color-on-visited-1-ref.html");
-}
-
 // We record the maximum number of times we had to look at a test before
 // it switched to the passing state (though we assume it's 10 to start
 // rather than 0 so that we have a reasonable default).  Then we make a
--- a/testing/mochitest/tests/SimpleTest/WindowSnapshot.js
+++ b/testing/mochitest/tests/SimpleTest/WindowSnapshot.js
@ -62,14 +62,10 @@ function assertSnapshots(s1, s2, expectEqual, fuzz, s1name, s2name) {
  var sym = expectEqual ? "==" : "!=";
  ok(passed, "reftest comparison: " + sym + " " + s1name + " " + s2name);
  if (!passed) {
-    let status = "TEST-UNEXPECTED-FAIL";
-    if (usesFailurePatterns() && recordIfMatchesFailurePattern(s1name)) {
-      status = "TEST-KNOWN-FAIL";
-    }
    // The language / format in this message should match the failure messages
    // displayed by reftest.js's "RecordResult()" method so that log output
    // can be parsed by reftest-analyzer.xhtml
-    var report = "REFTEST " + status + " | " + s1name +
+    var report = "REFTEST TEST-UNEXPECTED-FAIL | " + s1name +
                 " | image comparison (" + sym + "), max difference: " +
                 maxDifference + ", number of differing pixels: " +
                 numDifferentPixels + "\n";