Bug 1506842 - Always restyle / repaint when a visited query finishes. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 04 Nov 2019 16:55:33 +0000
changeset 500412 89fad029456188f03a670ef5f08a5d0856a728b1
parent 500411 75a7a3400888018a5f9b09c624dd22308f166807
child 500413 62a0c94786693fdc3612ec253fd80c9f855aeadf
push id36764
push userrmaries@mozilla.com
push dateTue, 05 Nov 2019 04:08:22 +0000
treeherdermozilla-central@fa1fe1088a5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1506842
milestone72.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 1506842 - Always restyle / repaint when a visited query finishes. r=dholbert Differential Revision: https://phabricator.services.mozilla.com/D50810
dom/base/Link.cpp
layout/base/RestyleManager.cpp
modules/libpref/init/StaticPrefList.yaml
servo/components/style/invalidation/element/state_and_attributes.rs
--- a/dom/base/Link.cpp
+++ b/dom/base/Link.cpp
@@ -5,16 +5,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "Link.h"
 
 #include "mozilla/EventStates.h"
 #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"
 #include "nsIDocShell.h"
 #include "nsIPrefetchService.h"
 #include "nsStyleLinkElement.h"
 
 #include "nsEscape.h"
@@ -104,17 +106,22 @@ void Link::VisitedQueryFinished(bool aVi
 
   MOZ_ASSERT(LinkState() == NS_EVENT_STATE_VISITED ||
                  LinkState() == NS_EVENT_STATE_UNVISITED,
              "Unexpected state obtained from LinkState()!");
 
   // 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 {
   // We are a constant method, but we are just lazily doing things and have to
   // track that state.  Cast away that constness!
   //
   // XXX(emilio): that's evil.
   Link* self = const_cast<Link*>(this);
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -3200,23 +3200,35 @@ void RestyleManager::ContentStateChanged
 
   Element& element = *aContent->AsElement();
   if (!element.HasServoData()) {
     return;
   }
 
   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;
+      }
     }
   }
 
   if (auto changeHint = ChangeForContentStateChange(element, aChangedBits)) {
     Servo_NoteExplicitHints(&element, RestyleHint{0}, changeHint);
   }
 
   // Don't bother taking a snapshot if no rules depend on these state bits.
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -4715,16 +4715,22 @@
   mirror: always
 
 # Whether we get notified of history queries for visited even if unvisited.
 - name: layout.css.notify-of-unvisited
   type: RelaxedAtomicBool
   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
   value: @IS_NIGHTLY_BUILD@
   mirror: always
   rust: true
 
 # Allow <number> and <number>/<number> both for <aspect-ratio>
--- a/servo/components/style/invalidation/element/state_and_attributes.rs
+++ b/servo/components/style/invalidation/element/state_and_attributes.rs
@@ -153,16 +153,24 @@ where
 
         if !snapshot.has_attrs() && state_changes.is_empty() {
             return false;
         }
 
         // 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
             // changes as well that imply additional hints for siblings.
             self.data.hint.insert(RestyleHint::restyle_subtree());
         }
 
         let mut classes_removed = SmallVec::<[Atom; 8]>::new();