Fix unloading, active BC, and clearing js runtime

[gterzian]

Do not set the window to be the currently active one for the windowproxy as part of load, as it will be done later when the document activity is set. And doing it later means that when unload runs, it is with the unloaded pipeline as the active window.

Only nullify the window proxy if it's not used by another (currently-active) window.

---

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

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

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/script_thread.rs, components/script/dom/window.rs, components/constellation/constellation.rs
• @cbrewster: components/constellation/constellation.rs
• @KiChjang: components/script/script_thread.rs, components/script/dom/window.rs, components/script_traits/lib.rs

[highfive]

Warning

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

[gterzian]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit c7c76b8 with merge acec9ef...

[paulrouget]

Will the document unload when the window is closed? For example, are we supposed to get an unload event when the glutin window is closed?

Also - this indeed fixes #24591 :)

[gterzian]

{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "document.open bailout should not have any side effects (ignore-opens-during-unload is greater than 0 during unload event)", 
    "test": "/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/bailout-side-effects-ignore-opens-during-unload.window.html", 
    "line": 121939, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "document.open bailout should not have any side effects (ignore-opens-during-unload is greater than 0 during pagehide event)", 
    "test": "/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/bailout-side-effects-ignore-opens-during-unload.window.html", 
    "line": 121941, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": "Test timed out", 
    "stack": null, 
    "subtest": "Traversing the history, unload event is fired on doucment", 
    "test": "/html/browsers/history/the-history-interface/traverse_the_history_unload_1.html", 
    "line": 172030, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/browsers/history/the-history-interface/traverse_the_history_unload_1.html", 
    "line": 172031, 
    "action": "test_result", 
    "expected": "OK"
}
{
    "status": "OK", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/semantics/scripting-1/the-script-element/execution-timing/083.html", 
    "line": 194814, 
    "action": "test_result", 
    "expected": "ERROR"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": "Test timed out", 
    "stack": null, 
    "subtest": "history.forward() with session history", 
    "test": "/html/browsers/history/the-history-interface/history_go_undefined.html", 
    "line": 200279, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/browsers/history/the-history-interface/history_go_undefined.html", 
    "line": 200280, 
    "action": "test_result", 
    "expected": "OK"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": "Test timed out", 
    "stack": null, 
    "subtest": "history.back() with session history", 
    "test": "/html/browsers/history/the-history-interface/history_back_1.html", 
    "line": 316729, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/browsers/history/the-history-interface/history_back_1.html", 
    "line": 316730, 
    "action": "test_result", 
    "expected": "OK"
} 

Yeah I think the problem is not all unload happens via a corresponding load of a new doc, so we still need the message from the constellation. Trying out with basically a boolean on whether the doc has unloaded already, to ignore further unload calls, because I think it will be hard to unify the unload flow in one go.

[gterzian]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit f8c79ca with merge 9e8d581...

[bors-servo]

💔 Test failed - status-taskcluster

[gterzian]

Ok so the unloading happens now "earlier", and this breaks something about the history flow. To be investigated.

[gterzian]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit fb2d2fa with merge 29d4f7d...

[bors-servo]

💔 Test failed - status-taskcluster

[gterzian]

I think the taskcluster results include intermittents, is there are way to know which ones are "real" failures?

[jdm]

Look at filtered-wpt-errorsummary.log under Artifacts.

[gterzian]

thanks, here is filtered-wpt-errorsummary.log as well as the crash log

{
    "status": "OK", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/semantics/scripting-1/the-script-element/execution-timing/083.html", 
    "line": 33327, 
    "action": "test_result", 
    "expected": "ERROR"
}
{
    "status": "CRASH", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/webvr/webvr-enabled-on-self-origin-by-feature-policy.https.sub.html", 
    "line": 68600, 
    "action": "test_result", 
    "expected": "OK"
}

▶ CRASH [expected OK] /webvr/webvr-enabled-on-self-origin-by-feature-policy.https.sub.html
  │ 
  │ 
  │ 
  │ 
  │ Layout thread disconnected.: "SendError(..)" (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at src/libcore/result.rs:1187)
  │ stack backtrace:
  │    0: servo::main::{{closure}}
  │    1: std::panicking::rust_panic_with_hook
  │              at src/libstd/panicking.rs:468
  │    2: std::panicking::continue_panic_fmt
  │              at src/libstd/panicking.rs:373
  │    3: rust_begin_unwind
  │              at src/libstd/panicking.rs:302
  │    4: core::panicking::panic_fmt
  │              at src/libcore/panicking.rs:82
  │    5: core::result::unwrap_failed
  │              at src/libcore/result.rs:1187
  │    6: script::dom::window::Window::force_reflow
  │    7: script::dom::window::Window::reflow
  │    8: script::dom::document::Document::finish_load
  │    9: script::dom::servoparser::ServoParser::do_parse_sync
  │   10: profile_traits::time::profile
  │   11: _ZN6script3dom11servoparser11ServoParser10parse_sync17hccbe7f353bb0c19dE.llvm.4139705411756622029
  │   12: <script::dom::servoparser::ParserContext as net_traits::FetchResponseListener>::process_response_eof
  │   13: script::script_thread::ScriptThread::handle_msg_from_constellation
  │   14: _ZN6script13script_thread12ScriptThread11handle_msgs17h702aaab883c81f98E.llvm.13163992065417716
  │   15: script::script_thread::ScriptThread::start
  │   16: profile_traits::mem::ProfilerChan::run_with_memory_reporting
  │   17: std::sys_common::backtrace::__rust_begin_short_backtrace

[jdm]

That crash is one that I would usually associated with #22955.

[bors-servo]

⌛ Testing commit fb2d2fa with merge 7367d9d...

[bors-servo]

💔 Test failed - status-taskcluster

[CYBAI]

{
    "status": "OK", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/semantics/scripting-1/the-script-element/execution-timing/083.html", 
    "line": 35742, 
    "action": "test_result", 
    "expected": "ERROR"
}

I've updated the test expectations for the unexpected OK.

@gterzian Maybe you want to mark this one as OK 👀?

Other failures are #24795 and #22276.

[gterzian]

Yep sorry I forgot to push the change with the updated expectation.

@bors-servo r=jdm

[bors-servo]

📌 Commit a21c0bf has been approved by jdm

[bors-servo]

⌛ Testing commit a21c0bf with merge 67e8086...

[bors-servo]

💔 Test failed - status-taskcluster

[jdm]

@bors-servo retry

• #21696

[bors-servo]

💣 Failed to start rebuilding: 405 Not Allowed

[bors-servo]

⌛ Testing commit a21c0bf with merge c060f00...

[bors-servo]

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing c060f00 to master...