Add DOMLoad message to constellation that is sent after the DOM Load event is dispatched.

[jgraham]

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/5319

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[metajack]

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/compositing/constellation.rs, line 716 [r1] (raw file):}
Why are these split now? Some explanation would be useful as it's not obvious just from reading through the code here.

---

Comments from the review on Reviewable.io

[jgraham]

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/compositing/constellation.rs, line 716 [r1] (raw file):}
If the question is essentially "why do we have LoadComplete and "DOMLoad", the answer is "I'm not sure". I suspect that LodComplete should never be used because it's racy, but I just made the minimal change. @glennw might know more.

---

Comments from the review on Reviewable.io

[glennw]

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/compositing/constellation.rs, line 716 [r1] (raw file):}
I'm also not sure why they both exist. When I made the changes to fix the reftest race conditions, I considered removing LoadComplete because it seemed like it wasn't particularly useful due to what jgraham mentioned above. I left it at the time because the patch was already large and some parts of the mac miniservo build were using the LoadComplete callback (I don't think anything else does). Perhaps add a TODO / GH issue to investigate whether it can be removed?

---

Comments from the review on Reviewable.io

[jgraham]

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/compositing/constellation.rs, line 716 [r1] (raw file):}
Issue #6455 filed on this. Does it also need another comment in the code (the match statement that dispatches here already has a comment that describes when each message is produced)?

---

Comments from the review on Reviewable.io

[jdm]

-S-awaiting-review -S-awaiting-answer +S-needs-code-changes

---

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

^{components/compositing/constellation.rs, line 716 [r1] (raw file):}
Yes, LoadComplete should go away, but it will be easier for someone to do once this split it accomplished.

---

^{components/compositing/constellation.rs, line 720 [r2] (raw file):}
Nix the unused argument.

---

Comments from the review on Reviewable.io

[metajack]

@jgraham ping

[jgraham]

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

^{components/compositing/constellation.rs, line 716 [r1] (raw file):}
OK, I'm assuming no further action is required for now.

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+
-S-awaiting-review +S-awaiting-merge

---

Reviewed 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit 36da7e2 has been approved by jdm

[bors-servo]

⌛ Testing commit 36da7e2 with merge 361d94d...

[bors-servo]

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2, mac3