1411754 - Rename PresShell::DestroyFramesFor to something a bit more descriptive of what it really does.

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Emilio Cobos Álvarez (:emilio)]

I suggested DestroyFramesForAndRestyle over bug 1404789, and Boris seemed to agree, but open to suggestions.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1411754: Rename PresShell::DestroyFramesFor to DestroyFramesForAndRestyle.

I'm drive-by removing the comment about the frame tree state because I looked
into it, and the answer is: we properly restore it.

The gotcha is that we retain it too much, indeed, we retain it enough that it
can leak. See bug 1397239.

Review commit: https://reviewboard.mozilla.org/r/193094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/193094/

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 8922093 [details]
Bug 1411754: Rename PresShell::DestroyFramesFor to DestroyFramesForAndRestyle.

https://reviewboard.mozilla.org/r/193094/#review198360

LGTM, r=mats

BTW, it seems we first post a "nsRestyleHint(0), nsChangeHint_ReconstructFrame" (in the !didReconstruct case)
and then after that a "eRestyle_Subtree, nsChangeHint(0)".  Seems like a weird order to do things.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8922093 [details]
Bug 1411754: Rename PresShell::DestroyFramesFor to DestroyFramesForAndRestyle.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193094/diff/1-2/

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Mats Palmgren (:mats) from comment #2)
> Comment on attachment 8922093 [details]
> Bug 1411754: Rename PresShell::DestroyFramesFor to
> DestroyFramesForAndRestyle.
> 
> https://reviewboard.mozilla.org/r/193094/#review198360
> 
> LGTM, r=mats
> 
> BTW, it seems we first post a "nsRestyleHint(0),
> nsChangeHint_ReconstructFrame" (in the !didReconstruct case)
> and then after that a "eRestyle_Subtree, nsChangeHint(0)".  Seems like a
> weird order to do things.

Fair enough, I just unified both.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/58801e0c84bc
Rename PresShell::DestroyFramesFor to DestroyFramesForAndRestyle. r=mats

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/58801e0c84bc