Constellation informs script about documents becoming inactive, active or fully active. #14971

asajeffrey · 2017-01-11T22:13:05Z

This PR replaces the current freeze/thaw mechanism by messages from the constellation to script informing it about when documents become inactive, active or fully active.

This means we can now implement |Document::is_active()| which is used in |document.write|.

This PR also changes how timers work: previously they were initialized running, and were then frozen/thawed. This means there was a transitory period when timers were running even though the document was not fully active. With this PR, timers are initially suspended, and are only resumed when the document is made fully active.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #14876
These changes do not require tests because it's an interal refactoring.

This change is

highfive · 2017-01-11T22:13:08Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/document.rs, components/script/timers.rs, components/script/dom/bindings/trace.rs, components/script/script_thread.rs, components/script/dom/window.rs, components/script_traits/lib.rs, components/script_traits/lib.rs
@KiChjang: components/script/dom/document.rs, components/script/timers.rs, components/script/dom/bindings/trace.rs, components/script/script_thread.rs, components/script/dom/window.rs, components/script_traits/lib.rs, components/script_traits/lib.rs

highfive · 2017-01-11T22:13:10Z

Warning

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

asajeffrey · 2017-01-11T22:13:56Z

cc @jdm @cbrewster

asajeffrey · 2017-01-12T02:26:33Z

Hmm, there are wpt failures for this, I've still not quite got timers sorted out. I'll let you know once I've got something that passes wpt locally.

asajeffrey · 2017-01-12T03:11:13Z

I rewrote this PR to explicitly initialize the document activity, which means we can go back to timers initially running. This fixes the wpt failures locally.

This PR is now ready for review.

asajeffrey · 2017-01-12T15:43:57Z

Oh good, an intermittent:

./mach test-wpt --release --repeat-until-unexpected ~/_mozilla/mozilla/websocket_connection_fail.html
...
Running 1 tests in web-platform-tests

Ran 185 tests finished in 1.0 seconds.
  • 185 ran as expected. 0 tests skipped.

Running 1 tests in web-platform-tests
...
  ▶ Unexpected subtest result in /_mozilla/mozilla/websocket_connection_fail.html:
  │ TIMEOUT [expected PASS] Untitled
  └   → Test timed out

asajeffrey · 2017-01-12T16:09:46Z

That intermittent is reproducable on master: #14985.

bors-servo · 2017-01-12T22:42:56Z

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

asajeffrey · 2017-01-12T23:10:12Z

Rebased.

bors-servo · 2017-01-13T23:42:44Z

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

jdm · 2017-01-19T21:47:10Z

@Ms2ger Review ping.

Ms2ger · 2017-01-20T10:57:40Z

Can we add a test for the document.write() change?

r? @cbrewster

asajeffrey · 2017-01-20T15:00:48Z

@Ms2ger OK, I'll give it a shot. The test might fail at the moment, because writing into iframe content documents isn't fully implemented IIRC. @nox?

nox · 2017-01-21T16:46:39Z

@asajeffrey I have no idea what you mean by "fully implemented". Why would iframe content documents be different than any other document? As long is it's same origin I don't see what's not implemented.

asajeffrey · 2017-01-23T15:07:45Z

@nox: I pushed a test to this PR (90fb8e3) which will show you what I mean. It passes in FF, but fails in servo with the error 'The object is in an invalid state'.

asajeffrey · 2017-01-27T18:43:54Z

Rebased.

asajeffrey · 2017-01-27T21:22:38Z

@bors-servo r=cbrewster

bors-servo · 2017-01-27T21:22:39Z

📌 Commit 631fefb has been approved by cbrewster

bors-servo · 2017-01-27T22:24:35Z

⌛ Testing commit 631fefb with merge 7cdc265...


        Auto merge of #14971 - asajeffrey:script-track-active-documents, r=cb…

…rewster Constellation informs script about documents becoming inactive, active or fully active.  This PR replaces the current freeze/thaw mechanism by messages from the constellation to script informing it about when documents become inactive, active or fully active. This means we can now implement |Document::is_active()| which is used in |document.write|. This PR also changes how timers work: previously they were initialized running, and were then frozen/thawed. This means there was a transitory period when timers were running even though the document was not fully active. With this PR, timers are initially suspended, and are only resumed when the document is made fully active. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #14876 - [X] These changes do not require tests because it's an interal refactoring.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14971)

bors-servo · 2017-01-27T23:11:40Z

💔 Test failed - linux-rel-wpt

cbrewster · 2017-01-27T23:16:21Z

  ▶ Unexpected subtest result in /html/dom/dynamic-markup-insertion/document-write/write-active-document.html:
  │ FAIL [expected PASS] document.write only writes to active documents
  └   → The object is in an invalid state.

asajeffrey · 2017-01-27T23:23:15Z

Oops, forgot to check in the .ini file.


        Fix document.write check for activity.

asajeffrey · 2017-01-27T23:28:25Z

@bors-servo r=cbrewster

bors-servo · 2017-01-27T23:28:25Z

📌 Commit ca9cee0 has been approved by cbrewster

bors-servo · 2017-01-28T00:30:39Z

⌛ Testing commit ca9cee0 with merge b5c94ba...


        Auto merge of #14971 - asajeffrey:script-track-active-documents, r=cb…

…rewster Constellation informs script about documents becoming inactive, active or fully active.  This PR replaces the current freeze/thaw mechanism by messages from the constellation to script informing it about when documents become inactive, active or fully active. This means we can now implement |Document::is_active()| which is used in |document.write|. This PR also changes how timers work: previously they were initialized running, and were then frozen/thawed. This means there was a transitory period when timers were running even though the document was not fully active. With this PR, timers are initially suspended, and are only resumed when the document is made fully active. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #14876 - [X] These changes do not require tests because it's an interal refactoring.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14971)

bors-servo · 2017-01-28T01:27:22Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev

highfive assigned Ms2ger Jan 11, 2017

highfive added the S-awaiting-review label Jan 11, 2017

asajeffrey added A-content/script A-constellation labels Jan 11, 2017

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from 5852156 to 3e9bda1 Jan 12, 2017

highfive added the S-needs-rebase label Jan 12, 2017

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from 3e9bda1 to ee2bf98 Jan 12, 2017

highfive added the S-needs-rebase label Jan 13, 2017

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from ee2bf98 to 6fc5b69 Jan 19, 2017

asajeffrey removed the S-needs-rebase label Jan 19, 2017

highfive assigned cbrewster and unassigned Ms2ger Jan 20, 2017

asajeffrey mentioned this pull request Jan 20, 2017

Document fully_activate may not be correct #15093

Closed

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from 6fc5b69 to 90fb8e3 Jan 23, 2017

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from 3a8fc25 to 3816799 Jan 23, 2017

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from d7210ea to 631fefb Jan 27, 2017

asajeffrey removed the S-needs-squash label Jan 27, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 27, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 27, 2017

Fix document.write check for activity.

Loading status checks…

ca9cee0

asajeffrey force-pushed the asajeffrey:script-track-active-documents branch from 631fefb to ca9cee0 Jan 27, 2017

highfive added S-awaiting-review and removed S-tests-failed labels Jan 27, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 27, 2017

bors-servo merged commit ca9cee0 into servo:master Jan 28, 2017
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jan 28, 2017

This was referenced Jan 28, 2017

Allow windows to share browsing contexts. #15120

Merged

History State API #15261

Closed

[WIP] Make subsequent about:blank loads async #14764

Closed

servo / servo

Constellation informs script about documents becoming inactive, active or fully active. #14971

Constellation informs script about documents becoming inactive, active or fully active. #14971

asajeffrey commented Jan 11, 2017 •

edited by larsbergstrom

highfive commented Jan 11, 2017

highfive commented Jan 11, 2017

asajeffrey commented Jan 11, 2017

asajeffrey commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

bors-servo commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

bors-servo commented Jan 13, 2017

jdm commented Jan 19, 2017

Ms2ger commented Jan 20, 2017

asajeffrey commented Jan 20, 2017

nox commented Jan 21, 2017

asajeffrey commented Jan 23, 2017

asajeffrey commented Jan 27, 2017

asajeffrey commented Jan 27, 2017

bors-servo commented Jan 27, 2017

bors-servo commented Jan 27, 2017

bors-servo commented Jan 27, 2017

cbrewster commented Jan 27, 2017

asajeffrey commented Jan 27, 2017

asajeffrey commented Jan 27, 2017

bors-servo commented Jan 27, 2017

bors-servo commented Jan 28, 2017

bors-servo commented Jan 28, 2017

servo / servo

Join GitHub today

Constellation informs script about documents becoming inactive, active or fully active. #14971

Constellation informs script about documents becoming inactive, active or fully active. #14971

Conversation

asajeffrey commented Jan 11, 2017 • edited by larsbergstrom

highfive commented Jan 11, 2017

highfive commented Jan 11, 2017

asajeffrey commented Jan 11, 2017

asajeffrey commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

bors-servo commented Jan 12, 2017

asajeffrey commented Jan 12, 2017

bors-servo commented Jan 13, 2017

jdm commented Jan 19, 2017

Ms2ger commented Jan 20, 2017

asajeffrey commented Jan 20, 2017

nox commented Jan 21, 2017

asajeffrey commented Jan 23, 2017

asajeffrey commented Jan 27, 2017

asajeffrey commented Jan 27, 2017

bors-servo commented Jan 27, 2017

bors-servo commented Jan 27, 2017

bors-servo commented Jan 27, 2017

cbrewster commented Jan 27, 2017

asajeffrey commented Jan 27, 2017

asajeffrey commented Jan 27, 2017

bors-servo commented Jan 27, 2017

bors-servo commented Jan 28, 2017

bors-servo commented Jan 28, 2017

asajeffrey commented Jan 11, 2017 •

edited by larsbergstrom