compositing/script: Do not dispatch the resize event when initially l…

[notriddle]

…oading.

No bug report corresponds to this, but I noticed it while trying to
reduce #10593

---

This change is

[highfive]

Heads up! This PR modifies the following files:

• @KiChjang: components/script/script_thread.rs, components/script_traits/lib.rs, components/script_traits/lib.rs

[Ms2ger]

Do you have a spec to back this up?

[pcwalton]

I don't think we need a spec cite for this, because it makes us stop doing something that the spec does not require us to do and that is nonsensical to begin with. (That is, this essentially corrects a pre-existing spec violation.)

[notriddle]

You're only supposed to fire the event if the window size has changed since the last time the run the resize steps algorithm has been run. This is clearly false the first time the algorithm runs; it has not changed because it did not exist at all.

[pcwalton]

compositing changes look fine to me.

[notriddle]

Is there something wrong with the script part?

[asajeffrey]

A probably dumb question... why have the compositor send a window size message with type Initial, rather than just have the window track whether it's had its initial size set? It seems like the code would be simpler if we just had script keep track of this.

---

Review status: 0 of 8 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[notriddle]

The compositor knows how the message is supposed to be treated; it would be bad if the script were silently out of sync with that expectation. I admit it's just a heuristic "explicit is better than implicit" choice.

I can change it, if you would prefer it the other way.

---

Comments from Reviewable

[asajeffrey]

It's just a question of who keeps track of that bit of state -- the compositor (as you have it) or script (by keeping track of an optional size). Personally I prefer the latter, but this is just taste.

@notriddle would you like me to finish doing the code review?

[bors-servo]

☔ The latest upstream changes (presumably #10641) made this pull request unmergeable. Please resolve the merge conflicts.

[notriddle]

I'd say yeah, finish the code review.

[asajeffrey]

OK, looks good. If we're going to go with the compositor rather than script deciding which is the initial size, then this seems a good implementation. r=me if so.

---

Reviewed 1 of 8 files at r1, 1 of 1 files at r2, 5 of 6 files at r3.
Review status: 7 of 8 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[asajeffrey]

Reviewed 1 of 6 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[notriddle]

Still awaiting answer?

[asajeffrey]

@bors-servo r+

[bors-servo]

📌 Commit 419633c has been approved by asajeffrey

[bors-servo]

⌛ Testing commit 419633c with merge 577239f...

[bors-servo]

💔 Test failed - linux-rel

[notriddle]

@bors-servo try

[bors-servo]

⌛ Trying commit 3e18116 with merge 3859d44...

[bors-servo]

💔 Test failed - mac-rel-wpt

[notriddle]

• #10660

[notriddle]

Pinging @asajeffrey

[asajeffrey]

Oops. Minor nit below.

---

Reviewed 3 of 3 files at r4, 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.

---

components/script/script_thread.rs, line 1032 [r5] (raw file):
Does this reflow happen in the Resize case too? Should the force_reflow be moved out of the if? The code reads a bit oddly, like the reflow is only happening on initial sizing, not on resizing.

---

Comments from Reviewable

[notriddle]

components/script/script_thread.rs, line 1032 [r5] (raw file):
You're right.

---

Comments from Reviewable

[notriddle]

components/script/script_thread.rs, line 1032 [r5] (raw file):
Done.

---

Comments from Reviewable

[asajeffrey]

@bors-servo r+

---

Reviewed 5 of 5 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[bors-servo]

📌 Commit 7940b22 has been approved by asajeffrey

[bors-servo]

⌛ Testing commit 7940b22 with merge 47a0f58...

[bors-servo]

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt