Constellation informs script about documents becoming inactive, active or fully active. by asajeffrey · Pull Request #14971 · servo/servo

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 Determine if a Document is active #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...

…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.

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...

…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 There is new code that needs to be reviewed. label Jan 11, 2017

asajeffrey added A-content/script Related to the script thread A-constellation Involves the constellation labels Jan 11, 2017

asajeffrey force-pushed the script-track-active-documents branch from 5852156 to 3e9bda1 Compare January 12, 2017 03:08

highfive added the S-needs-rebase There are merge conflict errors. label Jan 12, 2017

asajeffrey force-pushed the script-track-active-documents branch from 3e9bda1 to ee2bf98 Compare January 12, 2017 23:09

highfive added the S-needs-rebase There are merge conflict errors. label Jan 13, 2017

asajeffrey force-pushed the script-track-active-documents branch from ee2bf98 to 6fc5b69 Compare January 19, 2017 23:04

asajeffrey removed the S-needs-rebase There are merge conflict errors. 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 script-track-active-documents branch from 6fc5b69 to 90fb8e3 Compare January 23, 2017 15:04

asajeffrey force-pushed the script-track-active-documents branch from 3a8fc25 to 3816799 Compare January 23, 2017 18:58

asajeffrey force-pushed the script-track-active-documents branch from d7210ea to 631fefb Compare January 27, 2017 21:20

asajeffrey removed the S-needs-squash Some (or all) of the commits in the PR should be combined. label Jan 27, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 27, 2017

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 27, 2017

Fix document.write check for activity.

ca9cee0

asajeffrey force-pushed the script-track-active-documents branch from 631fefb to ca9cee0 Compare January 27, 2017 23:25

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 27, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 27, 2017

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

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. 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

Uh oh!

Conversation

asajeffrey commented Jan 11, 2017 • edited by larsbergstrom Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highfive commented Jan 11, 2017

Uh oh!

highfive commented Jan 11, 2017

Uh oh!

asajeffrey commented Jan 11, 2017

Uh oh!

asajeffrey commented Jan 12, 2017

Uh oh!

asajeffrey commented Jan 12, 2017

Uh oh!

asajeffrey commented Jan 12, 2017

Uh oh!

asajeffrey commented Jan 12, 2017

Uh oh!

bors-servo commented Jan 12, 2017

Uh oh!

asajeffrey commented Jan 12, 2017

Uh oh!

bors-servo commented Jan 13, 2017

Uh oh!

jdm commented Jan 19, 2017

Uh oh!

Ms2ger commented Jan 20, 2017

Uh oh!

asajeffrey commented Jan 20, 2017

Uh oh!

nox commented Jan 21, 2017

Uh oh!

asajeffrey commented Jan 23, 2017

Uh oh!

asajeffrey commented Jan 27, 2017

Uh oh!

asajeffrey commented Jan 27, 2017

Uh oh!

bors-servo commented Jan 27, 2017

Uh oh!

bors-servo commented Jan 27, 2017

Uh oh!

bors-servo commented Jan 27, 2017

Uh oh!

cbrewster commented Jan 27, 2017

Uh oh!

asajeffrey commented Jan 27, 2017

Uh oh!

asajeffrey commented Jan 27, 2017

Uh oh!

bors-servo commented Jan 27, 2017

Uh oh!

bors-servo commented Jan 28, 2017

Uh oh!

bors-servo commented Jan 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

asajeffrey commented Jan 11, 2017 •

edited by larsbergstrom

Loading