Implement history.length

[cbrewster]

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #__ (github issue number if applicable).

• There are tests for these changes OR
• These changes do not require tests because _____

---

This change is

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/constellation/constellation.rs
• @KiChjang: components/script/dom/history.rs, components/script/dom/webidls/History.webidl, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs

[highfive]

Warning

• These commits modify script code, but no tests are modified. Please consider adding a test!

[cbrewster]

@bors-servo try

[bors-servo]

⌛ Trying commit affe472 with merge 926e9df...

[bors-servo]

💔 Test failed - linux-rel

[cbrewster]

  ▶ Unexpected subtest result in /html/browsers/history/the-location-interface/location_assign_about_blank.html:
  │ FAIL [expected PASS] location.assign with initial about:blank browsing context
  │   → assert_equals: expected 1 but got 2
  │ 
  │ onload</</iframe.onload</<@http://web-platform.test:8000/html/browsers/history/the-location-interface/location_assign_about_blank.html:8:11
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1402:20
  └ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1426:20

I am going to mark this as failing, since about:blank is not replaced after navigating. I will investigate that issue later.

[Manishearth]

r? @asajeffrey

[asajeffrey]

Yay, another property implemented! A question about code reuse, but apart from that LGTM.

---

Reviewed 4 of 4 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

components/constellation/constellation.rs, line 1499 [r2] (raw file):

    }

    fn handle_joint_session_history_length(&self, pipeline_id: PipelineId, sender: IpcSender<u32>) {

Can we not reuse some code from joint_session_future and joint_session_past? Something like have functions joint_session_future_iterator, which iterates over the unsorted joint session future, and ditto past. Then the joint session future would be something like let mut result = joint_session_future_iterator().collect(); result.sort(); result, and this function could be joint_session_past_iterator().length() + joint_session_future_iterator().length().

---

Comments from Reviewable

[cbrewster]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

components/constellation/constellation.rs, line 1499 [r2] (raw file):

Previously, asajeffrey (Alan Jeffrey) wrote…

Can we not reuse some code from joint_session_future and joint_session_past? Something like have functions joint_session_future_iterator, which iterates over the unsorted joint session future, and ditto past. Then the joint session future would be something like let mut result = joint_session_future_iterator().collect(); result.sort(); result, and this function could be joint_session_past_iterator().length() + joint_session_future_iterator().length().

Creating an iterator would be a little too complicated for this scenario. Since it would be a massive chain of `Chain`s. The only other option would to create a new struct that impls Iterator, which I think ends up being too complicated for this.

---

Comments from Reviewable

[asajeffrey]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

components/constellation/constellation.rs, line 1499 [r2] (raw file):

Previously, ConnorGBrewster (Connor Brewster) wrote…

Creating an iterator would be a little too complicated for this scenario. Since it would be a massive chain of Chains. The only other option would to create a new struct that impls Iterator, which I think ends up being too complicated for this.

OK, if you reckon it would be more complicated. It looks to me like you've done most of the work. Perhaps a version of `FrameTreeIterator` that iterated over every frame, not just the fully active ones?

---

Comments from Reviewable

[cbrewster]

Review status: 7 of 8 files reviewed at latest revision, 1 unresolved discussion.

---

components/constellation/constellation.rs, line 1499 [r2] (raw file):

Previously, asajeffrey (Alan Jeffrey) wrote…

OK, if you reckon it would be more complicated. It looks to me like you've done most of the work. Perhaps a version of FrameTreeIterator that iterated over every frame, not just the fully active ones?

I like that idea, I have done that and it does reduce code duplication quite a bit. I would like to use it in `clear_joint_session_future` but that requires a mutable iterator. I don't want to make a separate mutable iterator just for use in one place, nor do I want to make it always mutable. Any thoughts?

---

Comments from Reviewable

[asajeffrey]

LGTM, squash and r=me.

---

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

---

components/constellation/constellation.rs, line 1499 [r2] (raw file):

Previously, ConnorGBrewster (Connor Brewster) wrote…

I like that idea, I have done that and it does reduce code duplication quite a bit. I would like to use it in clear_joint_session_future but that requires a mutable iterator. I don't want to make a separate mutable iterator just for use in one place, nor do I want to make it always mutable. Any thoughts?

This is much nicer now. I'm afraid I can't see an easy way to share code between the mutable and immutable iterators, Rust doesn't have polymorphism over mutability.

---

Comments from Reviewable

[cbrewster]

@bors-servo r=asajeffrey

[bors-servo]

📌 Commit 611de2a has been approved by asajeffrey

[bors-servo]

⌛ Testing commit 611de2a with merge 7aafc0d...

[bors-servo]

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