Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-08 19:04:45 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1579181 - Don't keep <use> shadow trees which we know we'll never render. r=longsonr

Browse Source 
This partially addresses the regression, but not fully. With this patch we don't
maintain shadow trees for nodes that we know won't get rendered.

This works fast in WebKit / Blink because of a bug in their implementation which
doesn't synchronize style attributes, introduced in [1].

You can see this clearly if you click on the bug's test-case and inspect the
<use> shadow trees (there's no style="stroke:orange" whatsoever).

They can kinda get away with it because they don't properly implement SVG 2. In
particular, in Blink / WebKit, the style of the element in the <use> shadow tree
is the style of the referenced element, which means that even if the style
attribute isn't properly synced it's ~ok since it doesn't end up mattering for
styling.

Easiest test-case for the behavior difference is:

```
<!doctype html>
<style>
  rect:hover {
    fill: green;
  }
</style>
<svg width=300 height=300>
  <g id="canvas">
    <rect fill=red width=100 height=100></rect>
  </g>
  <g>
    <use x=200 href="#canvas"></use>
  </g>
</svg>
```

Where Firefox will properly update each square independently when hovered, but
Blink / WebKit won't.

This used to work faster because in this particular test-case we have 3 hidden
<use> elements whose href is the #canvas, which is basically everything.

Before moving to shadow trees we'd do it using anonymous content, and since we
never got a frame we'd never clone the subtree in the first case.

This case was faster before bug 1450250, but this approach makes other cases
slow that were fixed by that bug, like bug 1485402.

So I'll try to optimize shadow tree syncing instead, I guess, but there's no
good reason not to land this in the meantime IMHO.

[1]: f4b022e64b%5E%21/third_party/WebKit/WebCore/svg/SVGElement.cpp

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-09-15 16:09:28 +00:00
parent f199696d60
commit c2bb091273
3 changed files with 50 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
dom/svg/SVGUseElement.cpp
View File

@ -236,25 +236,47 @@ void SVGUseElement::NodeWillBeDestroyed(const nsINode* aNode) {
UnlinkSource();
}
bool SVGUseElement::IsCyclicReferenceTo(const Element& aTarget) const {
// Returns whether this node could ever be displayed.
static bool NodeCouldBeRendered(const nsINode& aNode) {
if (aNode.IsSVGElement(nsGkAtoms::symbol)) {
// Only <symbol> elements in the root of a <svg:use> shadow tree are
// displayed.
auto* shadowRoot = ShadowRoot::FromNodeOrNull(aNode.GetParentNode());
return shadowRoot && shadowRoot->Host()->IsSVGElement(nsGkAtoms::use);
}
// TODO: Do we have other cases we can optimize out easily?
return true;
}
// Circular loop detection, plus detection of whether this shadow tree is
// rendered at all.
auto SVGUseElement::ScanAncestors(const Element& aTarget) const -> ScanResult {
if (&aTarget == this) {
return true;
return ScanResult::CyclicReference;
}
if (mOriginal && mOriginal->IsCyclicReferenceTo(aTarget)) {
return true;
if (mOriginal &&
mOriginal->ScanAncestors(aTarget) == ScanResult::CyclicReference) {
return ScanResult::CyclicReference;
}
auto result = ScanResult::Ok;
for (nsINode* parent = GetParentOrShadowHostNode(); parent;
parent = parent->GetParentOrShadowHostNode()) {
if (parent == &aTarget) {
return true;
return ScanResult::CyclicReference;
}
if (auto* use = SVGUseElement::FromNode(*parent)) {
if (mOriginal && use->mOriginal == mOriginal) {
return true;
return ScanResult::CyclicReference;
}
}
// Do we have other similar cases we can optimize out easily?
if (!NodeCouldBeRendered(*parent)) {
// NOTE(emilio): We can't just return here. If we're cyclic, we need to
// know.
result = ScanResult::Invisible;
}
}
return false;
return result;
}
//----------------------------------------------------------------------
@ -300,9 +322,7 @@ void SVGUseElement::UpdateShadowTree() {
return;
}
// circular loop detection
if (IsCyclicReferenceTo(*targetElement)) {
if (ScanAncestors(*targetElement) != ScanResult::Ok) {
return;
}

16
dom/svg/SVGUseElement.h
View File

@ -97,7 +97,21 @@ class SVGUseElement final : public SVGUseElementBase,
nsIPrincipal* aSubjectPrincipal, bool aNotify) final;
protected:
bool IsCyclicReferenceTo(const Element& aTarget) const;
// Information from walking our ancestors and a given target.
enum class ScanResult {
// Nothing that should stop us from rendering the shadow tree.
Ok,
// We're never going to be displayed, so no point in updating the shadow
// tree.
//
// However if we're referenced from another tree that tree may need to be
// rendered.
Invisible,
// We're a cyclic reference to either an ancestor or another shadow tree. We
// shouldn't render this <use> element.
CyclicReference,
};
ScanResult ScanAncestors(const Element& aTarget) const;
/**
* Helper that provides a reference to the element with the ID that is

6
layout/svg/svg.css
View File

@ -11,7 +11,11 @@ style, script {
display: none;
}
/* This is only to be overridden by the rule right below. */
/*
* This is only to be overridden by the rule right below.
*
* NOTE(emilio): NodeCouldBeRendered in SVGUseElement.cpp relies on this.
*/
symbol {
display: none !important;
}