Bug 1540220 - Remove some useless usage of LazyComputeBehavior::Allow. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 09 Apr 2019 18:03:51 +0000
changeset 468609 ae9783be99324502017f3c9bfc92cb811fd1993e
parent 468608 9194b56c65fa97430a2badbb583565e33a1c5275
child 468610 07a87bbee9ca37c2852b620e52b62679719e64f4
push id35843
push usernbeleuzu@mozilla.com
push dateTue, 09 Apr 2019 22:08:13 +0000
treeherdermozilla-central@a31032a16330 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1540220
milestone68.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 1540220 - Remove some useless usage of LazyComputeBehavior::Allow. r=dholbert This switches nsFrameSetFrame's hacky frame construction codepath to operate on the flattened tree, since it made me a bit more comfortable about it (all layout should operate on is the flattened tree, though in this cause this should not cause any web-observable behavior change, since <frameset> can't be a shadow host per spec, and we no longer support XBL-in-content). That doesn't need to compute styles lazily. You only need to compute styles lazily in descendants of display: none elements, and even though this code doesn't check on display: none _on the children_, the parent element should never have display: none (since we're creating an nsFrameSetFrame for it). There are other places that still call into this (apart from getComputedStyle). One is nsImageFrame's cursor code. Given the <area> elements referencing an image map could be anywhere, we need to have lazy computation here. The other is the viewport style propagation stuff. There shouldn't be a need for LazyComputeBehavior::Allow on the document element in order to check the viewport styles propagation, since the root element can't have display: none ancestors. But we run that code before actually constructing the doc element containing block, which is when we do the initial document styling. We could remove that with some more effort, but it's not worth it right now, since we need to keep using it for the <body>, since the document element could be display: none itself, and we propagate the overflow styles in that case still. I filed https://github.com/w3c/csswg-drafts/issues/3779 to potentially change that. Differential Revision: https://phabricator.services.mozilla.com/D25455
layout/generic/nsFrameSetFrame.cpp
--- a/layout/generic/nsFrameSetFrame.cpp
+++ b/layout/generic/nsFrameSetFrame.cpp
@@ -31,16 +31,17 @@
 #include "nsNameSpaceManager.h"
 #include "nsCSSAnonBoxes.h"
 #include "mozilla/ServoStyleSet.h"
 #include "mozilla/dom/Element.h"
 #include "nsDisplayList.h"
 #include "nsNodeUtils.h"
 #include "mozAutoDocUpdate.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/dom/ChildIterator.h"
 #include "mozilla/dom/HTMLFrameSetElement.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/MouseEvents.h"
 #include "nsSubDocumentFrame.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using namespace mozilla::gfx;
@@ -254,55 +255,55 @@ void nsHTMLFramesetFrame::Init(nsIConten
                                                  NS_MAX_FRAMESET_SPEC_COUNT,
                 "Should not overflow numCells");
   mChildFrameborder = MakeUnique<nsFrameborder[]>(numCells);
   mChildBorderColors = MakeUnique<nsBorderColor[]>(numCells);
 
   // create the children frames; skip content which isn't <frameset> or <frame>
   mChildCount = 0;  // number of <frame> or <frameset> children
 
-  for (nsIContent* child = mContent->GetFirstChild(); child;
-       child = child->GetNextSibling()) {
-    if (mChildCount ==
-        numCells) {  // we have more <frame> or <frameset> than cells
+  FlattenedChildIterator children(mContent);
+  for (nsIContent* child = children.GetNextChild(); child;
+       child = children.GetNextChild()) {
+    if (mChildCount == numCells) {
+      // we have more <frame> or <frameset> than cells
       // Clear the lazy bits in the remaining children.  Also clear
       // the restyle flags, like nsCSSFrameConstructor::ProcessChildren does.
       for (; child; child = child->GetNextSibling()) {
         child->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES | NODE_NEEDS_FRAME);
       }
       break;
     }
     child->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES | NODE_NEEDS_FRAME);
 
     // IMPORTANT: This must match the conditions in
     // nsCSSFrameConstructor::ContentAppended/Inserted/Removed
-    if (!child->IsHTMLElement()) {
-      continue;
-    }
-
     if (!child->IsAnyOfHTMLElements(nsGkAtoms::frameset, nsGkAtoms::frame)) {
       continue;
     }
 
-    RefPtr<ComputedStyle> kidSC = presShell->StyleSet()->ResolveStyleFor(
-        child->AsElement(), LazyComputeBehavior::Allow);
-
+    // FIXME(emilio): This doesn't even respect display: none, but that matches
+    // other browsers ;_;
+    //
+    // Maybe we should change that though.
+    RefPtr<ComputedStyle> kidStyle =
+        presShell->StyleSet()->ResolveServoStyle(*child->AsElement());
     nsIFrame* frame;
     if (child->IsHTMLElement(nsGkAtoms::frameset)) {
-      frame = NS_NewHTMLFramesetFrame(presShell, kidSC);
+      frame = NS_NewHTMLFramesetFrame(presShell, kidStyle);
 
       nsHTMLFramesetFrame* childFrame = (nsHTMLFramesetFrame*)frame;
       childFrame->SetParentFrameborder(frameborder);
       childFrame->SetParentBorderWidth(borderWidth);
       childFrame->SetParentBorderColor(borderColor);
       frame->Init(child, this, nullptr);
 
       mChildBorderColors[mChildCount].Set(childFrame->GetBorderColor());
     } else {  // frame
-      frame = NS_NewSubDocumentFrame(presShell, kidSC);
+      frame = NS_NewSubDocumentFrame(presShell, kidStyle);
 
       frame->Init(child, this, nullptr);
 
       mChildFrameborder[mChildCount] = GetFrameBorder(child);
       mChildBorderColors[mChildCount].Set(GetBorderColor(child));
     }
     child->SetPrimaryFrame(frame);