1502421 - fix undefined behavior in PendingFullscreenChangeList

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Nathan Froyd [:froydnj]]

The order of evaluation of x and y in x->Func(y) is undefined, but the
code in PendingFullscreenChangeList depends on x being evaluated before
y.  We should make the ordering explicit.  (The ordering is fully
specified in C++17, but we're not ready to make that switch yet.)

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: fix undefined behavior in PendingFullscreenChangeList

None of our current compilers implement the bad evaluation order, but MSVC for
AArch64 does, and we might as well remove some undefined behavior from our code
in any event.

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 9020374 [details] [diff] [review]
fix undefined behavior in PendingFullscreenChangeList

Review of attachment 9020374 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsDocument.cpp
@@ +10543,5 @@
>        , mRootShellForIteration(aDoc->GetDocShell())
>      {
>        if (mCurrent) {
>          if (mRootShellForIteration && aOption == eDocumentsWithSameRoot) {
> +          nsCOMPtr<nsIDocShellTreeItem> root;

Maybe put a comment here? This one looks a little odd and may tempt somebody to "clean" it up.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6baeb0cb9aeb
fix undefined behavior in PendingFullscreenChangeList; r=mccr8

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6baeb0cb9aeb