Bugzilla

(In reply to Robert Longson [:longsonr] from comment #1)
> We shouldn't recurse any more now that SMIL no longer flushes style.

(That's via bug 1354393 it looks like, which fixed bug bug 1858792 -- thanks!)

Here's a code block for my own reference, since I had forgotten the relevant details here:
https://searchfox.org/mozilla-central/rev/1f27a4022f9f1269d897526c1c892a57743e650c/dom/base/Document.cpp#13680,13683-13684,13703-13704
```cpp
void Document::DoUpdateSVGUseElementShadowTrees() {
...
  MOZ_ASSERT(!mCloningForSVGUse);
  mCloningForSVGUse = true;
...
  mCloningForSVGUse = false;
}
```

As long as we're still doing this^ pattern of temporarily setting this bool, it seems like we might as well use an AutoRestore as a belt-and-suspenders sort of thing.  Even if we 100%-for-sure-never-do-recursion here, it's still useful to protect against early returns that forget to unset the bool, etc.

Having said that, it does look like we're never using this `mCloningForSVGUse` variable or its accessor `CloningForSVGUse()`  at all in opt builds -- we only check it in the assert above -- so maybe we can just remove this bool at this point?

We did originally have some usages in opt builds, in the patch that added it here:
https://hg.mozilla.org/mozilla-central/rev/954b734084c2f140d223b99dbe4434ff80bfc869#l3.14
...but those seem to have been removed.

(In reply to Robert Longson [:longsonr] from comment #1)
> We shouldn't recurse any more now that SMIL no longer flushes style.

(That's via bug 1354393 it looks like, which fixed bug bug 1858792 -- thanks!)

Here's a code block for my own reference, since I had forgotten the relevant details here:
https://searchfox.org/mozilla-central/rev/1f27a4022f9f1269d897526c1c892a57743e650c/dom/base/Document.cpp#13680,13683-13684,13703-13704
```cpp
void Document::DoUpdateSVGUseElementShadowTrees() {
...
  MOZ_ASSERT(!mCloningForSVGUse);
  mCloningForSVGUse = true;
...
  mCloningForSVGUse = false;
}
```

As long as we're still doing this^ pattern of temporarily setting this bool, it seems like we might as well use an AutoRestore as a belt-and-suspenders sort of thing.  Even if we 100%-for-sure-never-do-recursion here, it's still useful to protect against early returns that forget to unset the bool, etc.

[EDIT: the following is wrong, ignore it, sorry]
~Having said that, it does look like we're never using this `mCloningForSVGUse` variable or its accessor `CloningForSVGUse()`  at all in opt builds -- we only check it in the assert above -- so maybe we can just remove this bool at this point?~