From a57fd16889c59fee48f512e71f4912a3c8631eb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 4 Nov 2019 16:55:33 +0000
Subject: [PATCH] Bug 1506842 - Always restyle / repaint when a visited query
 finishes. r=dholbert

Differential Revision: https://phabricator.services.mozilla.com/D50810

--HG--
extra : moz-landing-system : lando
---
 dom/base/Link.cpp                             |  9 ++++++-
 layout/base/RestyleManager.cpp                | 26 ++++++++++++++-----
 modules/libpref/init/StaticPrefList.yaml      |  6 +++++
 .../element/state_and_attributes.rs           |  8 ++++++
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/dom/base/Link.cpp b/dom/base/Link.cpp
index bb9d5015dfef..bc10e011da3c 100644
--- a/dom/base/Link.cpp
+++ b/dom/base/Link.cpp
@@ -10,6 +10,8 @@
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/IHistory.h"
+#include "mozilla/StaticPrefs_layout.h"
+#include "nsLayoutUtils.h"
 #include "nsIURL.h"
 #include "nsIURIMutator.h"
 #include "nsISizeOf.h"
@@ -109,7 +111,12 @@ void Link::VisitedQueryFinished(bool aVisited) {
   // Tell the element to update its visited state
   mElement->UpdateState(true);
 
-  // FIXME(emilio, bug 1506842): Repaint here unconditionally.
+  if (StaticPrefs::layout_css_always_repaint_on_unvisited()) {
+    // Even if the state didn't actually change, we need to repaint in order for
+    // the visited state not to be observable.
+    nsLayoutUtils::PostRestyleEvent(GetElement(), RestyleHint::RestyleSubtree(),
+                                    nsChangeHint_RepaintFrame);
+  }
 }
 
 EventStates Link::LinkState() const {
diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp
index 32a0286e4b53..5b856666189c 100644
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -3205,13 +3205,25 @@ void RestyleManager::ContentStateChanged(nsIContent* aContent,
 
   const EventStates kVisitedAndUnvisited =
       NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED;
-  // NOTE: We want to return ASAP for visitedness changes, but we don't want to
-  // mess up the situation where the element became a link or stopped being one.
-  if (aChangedBits.HasAllStates(kVisitedAndUnvisited) &&
-      !Gecko_VisitedStylesEnabled(element.OwnerDoc())) {
-    aChangedBits &= ~kVisitedAndUnvisited;
-    if (aChangedBits.IsEmpty()) {
-      return;
+
+  // When visited links are disabled, they cannot influence style for obvious
+  // reasons.
+  //
+  // When layout.css.always-repaint-on-unvisited is true, we'll restyle when the
+  // relevant visited query finishes, regardless of the style (see
+  // Link::VisitedQueryFinished). So there's no need to do anything as a result
+  // of this state change just yet.
+  //
+  // Note that this check checks for _both_ bits: This is only true when visited
+  // changes to unvisited or vice-versa, but not when we start or stop being a
+  // link itself.
+  if (aChangedBits.HasAllStates(kVisitedAndUnvisited)) {
+    if (!Gecko_VisitedStylesEnabled(element.OwnerDoc()) ||
+        StaticPrefs::layout_css_always_repaint_on_unvisited()) {
+      aChangedBits &= ~kVisitedAndUnvisited;
+      if (aChangedBits.IsEmpty()) {
+        return;
+      }
     }
   }
 
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
index bef06544a9e1..0b1b6817891f 100644
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -4720,6 +4720,12 @@
   value: @IS_NIGHTLY_BUILD@
   mirror: always
 
+# Whether we always restyle / repaint as a result of a visited query
+- name: layout.css.always-repaint-on-unvisited
+  type: RelaxedAtomicBool
+  value: false
+  mirror: always
+
 # Make `zoom` a `transform` + `transform-origin` alias.
 - name: layout.css.zoom-transform-hack.enabled
   type: RelaxedAtomicBool
diff --git a/servo/components/style/invalidation/element/state_and_attributes.rs b/servo/components/style/invalidation/element/state_and_attributes.rs
index 30521fb8c002..c4671b0011ce 100644
--- a/servo/components/style/invalidation/element/state_and_attributes.rs
+++ b/servo/components/style/invalidation/element/state_and_attributes.rs
@@ -158,6 +158,14 @@ where
         // If we the visited state changed, we force a restyle here. Matching
         // doesn't depend on the actual visited state at all, so we can't look
         // at matching results to decide what to do for this case.
+        //
+        // TODO(emilio): This should be contains(), to avoid doing subtree
+        // restyles when adding or removing an href attribute, but invalidation
+        // for that case is broken right now (bug 1591987).
+        //
+        // This piece of code should be removed when
+        // layout.css.always-repaint-on-unvisited is true, since we cannot get
+        // into this situation in that case.
         if state_changes.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) {
             trace!(" > visitedness change, force subtree restyle");
             // We can't just return here because there may also be attribute