Bug 1533891 - Give up on optimizing out image loads load resolution for non-text. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 14 Mar 2019 22:24:37 +0000
changeset 521979 5be153ff04f4
parent 521978 ee3da1413835
child 521980 fa6af8c14ee4
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1533891, 1465474
milestone67.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 1533891 - Give up on optimizing out image loads load resolution for non-text. r=heycam ::first-line reparenting may make non-first continuations to get a new style on which we haven't run StartImageLoads when fragmenting out of the first-line. Given this was mostly an opportunistic optimization let's remove it rather than sacrificing correctness. With bug 1465474 we would be able to fix this... Differential Revision: https://phabricator.services.mozilla.com/D23525
layout/generic/nsFrame.cpp
layout/style/crashtests/1533891.html
layout/style/crashtests/crashtests.list
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1082,25 +1082,26 @@ void nsFrame::DidSetComputedStyle(Comput
 
   Document* doc = PresContext()->Document();
   ImageLoader* imageLoader = doc->StyleImageLoader();
   // Continuing text frame doesn't initialize its continuation pointer before
   // reaching here for the first time, so we have to exclude text frames. This
   // doesn't affect correctness because text can't match selectors.
   //
   // FIXME(emilio): We should consider fixing that.
-  const bool isNonTextFirstContinuation =
-      !IsTextFrame() && !GetPrevContinuation();
-  if (isNonTextFirstContinuation) {
+  //
+  // TODO(emilio): Can we avoid doing some / all of the image stuff when
+  // isNonTextFirstContinuation is false? We should consider doing this just for
+  // primary frames and pseudos, but the first-line reparenting code makes it
+  // all bad, should get around to bug 1465474 eventually :(
+  const bool isNonText = !IsTextFrame();
+  if (isNonText) {
     mComputedStyle->StartImageLoads(*doc);
   }
 
-  // TODO(emilio): Can we avoid doing some / all of this when
-  // isNonTextFirstContinuation is false? We should consider doing this just for
-  // primary frames and pseudos.
   const nsStyleImageLayers* oldLayers =
       aOldComputedStyle ? &aOldComputedStyle->StyleBackground()->mImage
                         : nullptr;
   const nsStyleImageLayers* newLayers = &StyleBackground()->mImage;
   AddAndRemoveImageAssociations(*imageLoader, this, oldLayers, newLayers);
 
   oldLayers =
       aOldComputedStyle ? &aOldComputedStyle->StyleSVGReset()->mMask : nullptr;
@@ -1223,16 +1224,17 @@ void nsFrame::DidSetComputedStyle(Comput
     if (newShapeImage) {
       imageLoader->AssociateRequestToFrame(
           newShapeImage, this, ImageLoader::REQUEST_REQUIRES_REFLOW);
     }
   }
 
   // SVGObserverUtils::GetEffectProperties() asserts that we only invoke it with
   // the first continuation so we need to check that in advance.
+  const bool isNonTextFirstContinuation = isNonText && !GetPrevContinuation();
   if (isNonTextFirstContinuation) {
     // Kick off loading of external SVG resources referenced from properties if
     // any. This currently includes filter, clip-path, and mask.
     SVGObserverUtils::InitiateResourceDocLoads(this);
   }
 
   // If the page contains markup that overrides text direction, and
   // does not contain any characters that would activate the Unicode
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1533891.html
@@ -0,0 +1,13 @@
+<html>
+<head>
+    <style>
+        * {
+            border-image-source: url(data:image/gif;base64,R0lGODlhEAAQAMQAAORHHOVSKudfOulrSOp3WOyDZu6QdvCchPGolfO0o/XBs/fNwfjZ0frl3/zy7////wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAkAABAALAAAAAAQABAAAAVVICSOZGlCQAosJ6mu7fiyZeKqNKToQGDsM8hBADgUXoGAiqhSvp5QAnQKGIgUhwFUYLCVDFCrKUE1lBavAViFIDlTImbKC5Gm2hB0SlBCBMQiB0UjIQA7);
+        }
+        pre::first-line { }
+    </style>
+    <head>
+<body>
+<pre><data>]j% BAVxE</command>
+</body>
+</html>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -293,8 +293,9 @@ load 1469076.html
 load 1475003.html
 load 1479681.html
 load 1488817.html
 load 1490012.html
 load 1502893.html
 load 1509989.html
 load 1514086.html
 pref(layout.css.moz-binding.content.enabled,false) load 1517319.html
+load 1533891.html