Skip to content

Commit b5c94ba

Browse files
author
bors-servo
authored
Auto merge of #14971 - asajeffrey:script-track-active-documents, r=cbrewster
Constellation informs script about documents becoming inactive, active or fully active. <!-- Please describe your changes on the following line: --> 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. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [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. <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14971) <!-- Reviewable:end -->
2 parents d4ee8a3 + ca9cee0 commit b5c94ba

File tree

18 files changed

+214
-104
lines changed

18 files changed

+214
-104
lines changed

‎components/constellation/constellation.rs‎

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ use profile_traits::mem;
9595
use profile_traits::time;
9696
use script_traits::{AnimationState, AnimationTickType, CompositorEvent};
9797
use script_traits::{ConstellationControlMsg, ConstellationMsg as FromCompositorMsg, DiscardBrowsingContext};
98-
use script_traits::{DocumentState, LayoutControlMsg, LoadData};
98+
use script_traits::{DocumentActivity, DocumentState, LayoutControlMsg, LoadData};
9999
use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, IFrameSandboxState, TimerEventRequest};
100100
use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, ScriptThreadFactory};
101101
use script_traits::{LogEntry, ServiceWorkerMsg, webdriver_msg};
@@ -1405,10 +1405,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
14051405

14061406
let window_size = old_pipeline.and_then(|old_pipeline| old_pipeline.size);
14071407

1408-
if let Some(old_pipeline) = old_pipeline {
1409-
old_pipeline.freeze();
1410-
}
1411-
14121408
(load_data, window_size, is_private)
14131409
};
14141410

@@ -1628,11 +1624,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
16281624
});
16291625
self.new_pipeline(new_pipeline_id, root_frame_id, None, window_size, load_data, sandbox, false);
16301626

1631-
// Send message to ScriptThread that will suspend all timers
1632-
match self.pipelines.get(&source_id) {
1633-
Some(source) => source.freeze(),
1634-
None => warn!("Pipeline {:?} loaded after closure", source_id),
1635-
};
16361627
Some(new_pipeline_id)
16371628
}
16381629
}
@@ -2050,13 +2041,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
20502041
self.focus_pipeline_id = Some(pipeline_id);
20512042
}
20522043

2053-
// Suspend the old pipeline, and resume the new one.
2054-
if let Some(pipeline) = self.pipelines.get(&old_pipeline_id) {
2055-
pipeline.freeze();
2056-
}
2057-
if let Some(pipeline) = self.pipelines.get(&pipeline_id) {
2058-
pipeline.thaw();
2059-
}
2044+
// Deactivate the old pipeline, and activate the new one.
2045+
self.update_activity(old_pipeline_id);
2046+
self.update_activity(pipeline_id);
20602047

20612048
// Set paint permissions correctly for the compositor layers.
20622049
self.send_frame_tree();
@@ -2125,22 +2112,24 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
21252112
}
21262113
}
21272114

2128-
let (evicted_id, new_frame, clear_future, location_changed) = if let Some(mut entry) = frame_change.replace {
2115+
let (evicted_id, new_frame, navigated, location_changed) = if let Some(mut entry) = frame_change.replace {
21292116
debug!("Replacing pipeline in existing frame.");
21302117
let evicted_id = entry.pipeline_id;
21312118
entry.replace_pipeline(frame_change.new_pipeline_id, frame_change.url.clone());
21322119
self.traverse_to_entry(entry);
2133-
(evicted_id, false, false, false)
2120+
(evicted_id, false, None, false)
21342121
} else if let Some(frame) = self.frames.get_mut(&frame_change.frame_id) {
21352122
debug!("Adding pipeline to existing frame.");
2123+
let old_pipeline_id = frame.pipeline_id;
21362124
frame.load(frame_change.new_pipeline_id, frame_change.url.clone());
21372125
let evicted_id = frame.prev.len()
21382126
.checked_sub(PREFS.get("session-history.max-length").as_u64().unwrap_or(20) as usize)
21392127
.and_then(|index| frame.prev.get_mut(index))
21402128
.and_then(|entry| entry.pipeline_id.take());
2141-
(evicted_id, false, true, true)
2129+
(evicted_id, false, Some(old_pipeline_id), true)
21422130
} else {
2143-
(None, true, false, true)
2131+
debug!("Adding pipeline to new frame.");
2132+
(None, true, None, true)
21442133
};
21452134

21462135
if let Some(evicted_id) = evicted_id {
@@ -2149,9 +2138,14 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
21492138

21502139
if new_frame {
21512140
self.new_frame(frame_change.frame_id, frame_change.new_pipeline_id, frame_change.url);
2141+
self.update_activity(frame_change.new_pipeline_id);
21522142
};
21532143

2154-
if clear_future {
2144+
if let Some(old_pipeline_id) = navigated {
2145+
// Deactivate the old pipeline, and activate the new one.
2146+
self.update_activity(old_pipeline_id);
2147+
self.update_activity(frame_change.new_pipeline_id);
2148+
// Clear the joint session future
21552149
let top_level_frame_id = self.get_top_level_frame_for_pipeline(frame_change.new_pipeline_id);
21562150
self.clear_joint_session_future(top_level_frame_id);
21572151
}
@@ -2165,7 +2159,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
21652159
}
21662160

21672161
fn handle_activate_document_msg(&mut self, pipeline_id: PipelineId) {
2168-
debug!("Document ready to activate {:?}", pipeline_id);
2162+
debug!("Document ready to activate {}", pipeline_id);
21692163

21702164
// Notify the parent (if there is one).
21712165
if let Some(pipeline) = self.pipelines.get(&pipeline_id) {
@@ -2359,6 +2353,53 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
23592353
ReadyToSave::Ready
23602354
}
23612355

2356+
/// Get the current activity of a pipeline.
2357+
fn get_activity(&self, pipeline_id: PipelineId) -> DocumentActivity {
2358+
let mut ancestor_id = pipeline_id;
2359+
loop {
2360+
if let Some(ancestor) = self.pipelines.get(&ancestor_id) {
2361+
if let Some(frame) = self.frames.get(&ancestor.frame_id) {
2362+
if frame.pipeline_id == ancestor_id {
2363+
if let Some((parent_id, FrameType::IFrame)) = ancestor.parent_info {
2364+
ancestor_id = parent_id;
2365+
continue;
2366+
} else {
2367+
return DocumentActivity::FullyActive;
2368+
}
2369+
}
2370+
}
2371+
}
2372+
if pipeline_id == ancestor_id {
2373+
return DocumentActivity::Inactive;
2374+
} else {
2375+
return DocumentActivity::Active;
2376+
}
2377+
}
2378+
}
2379+
2380+
/// Set the current activity of a pipeline.
2381+
fn set_activity(&self, pipeline_id: PipelineId, activity: DocumentActivity) {
2382+
debug!("Setting activity of {} to be {:?}.", pipeline_id, activity);
2383+
if let Some(pipeline) = self.pipelines.get(&pipeline_id) {
2384+
pipeline.set_activity(activity);
2385+
let child_activity = if activity == DocumentActivity::Inactive {
2386+
DocumentActivity::Active
2387+
} else {
2388+
activity
2389+
};
2390+
for child_id in &pipeline.children {
2391+
if let Some(child) = self.frames.get(child_id) {
2392+
self.set_activity(child.pipeline_id, child_activity);
2393+
}
2394+
}
2395+
}
2396+
}
2397+
2398+
/// Update the current activity of a pipeline.
2399+
fn update_activity(&self, pipeline_id: PipelineId) {
2400+
self.set_activity(pipeline_id, self.get_activity(pipeline_id));
2401+
}
2402+
23622403
fn clear_joint_session_future(&mut self, frame_id: FrameId) {
23632404
let frame_ids: Vec<FrameId> = self.full_frame_tree_iter(frame_id)
23642405
.map(|frame| frame.id)

‎components/constellation/pipeline.rs‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ use net_traits::{IpcSend, ResourceThreads};
1919
use net_traits::image_cache_thread::ImageCacheThread;
2020
use profile_traits::mem as profile_mem;
2121
use profile_traits::time;
22-
use script_traits::{ConstellationControlMsg, DevicePixel, DiscardBrowsingContext, InitialScriptState};
22+
use script_traits::{ConstellationControlMsg, DevicePixel, DiscardBrowsingContext};
23+
use script_traits::{DocumentActivity, InitialScriptState};
2324
use script_traits::{LayoutControlMsg, LayoutMsg, LoadData, MozBrowserEvent};
2425
use script_traits::{NewLayoutInfo, SWManagerMsg, SWManagerSenders, ScriptMsg};
2526
use script_traits::{ScriptThreadFactory, TimerEventRequest, WindowSizeData};
@@ -366,17 +367,11 @@ impl Pipeline {
366367
}
367368
}
368369

369-
/// Notify this pipeline that it is no longer fully active.
370-
pub fn freeze(&self) {
371-
if let Err(e) = self.event_loop.send(ConstellationControlMsg::Freeze(self.id)) {
372-
warn!("Sending freeze message failed ({}).", e);
373-
}
374-
}
375-
376-
/// Notify this pipeline that it is fully active.
377-
pub fn thaw(&self) {
378-
if let Err(e) = self.event_loop.send(ConstellationControlMsg::Thaw(self.id)) {
379-
warn!("Sending freeze message failed ({}).", e);
370+
/// Notify this pipeline of its activity.
371+
pub fn set_activity(&self, activity: DocumentActivity) {
372+
let msg = ConstellationControlMsg::SetDocumentActivity(self.id, activity);
373+
if let Err(e) = self.event_loop.send(msg) {
374+
warn!("Sending activity message failed ({}).", e);
380375
}
381376
}
382377

‎components/script/dom/bindings/trace.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ use profile_traits::time::ProfilerChan as TimeProfilerChan;
7474
use script_layout_interface::OpaqueStyleAndLayoutData;
7575
use script_layout_interface::reporter::CSSErrorReporter;
7676
use script_layout_interface::rpc::LayoutRPC;
77-
use script_traits::{TimerEventId, TimerSource, TouchpadPressurePhase};
77+
use script_traits::{DocumentActivity, TimerEventId, TimerSource, TouchpadPressurePhase};
7878
use script_traits::{UntrustedNodeAddress, WindowSizeData, WindowSizeType};
7979
use serde::{Deserialize, Serialize};
8080
use servo_atoms::Atom;
@@ -327,7 +327,7 @@ unsafe_no_jsmanaged_fields!(TrustedPromise);
327327
unsafe_no_jsmanaged_fields!(PropertyDeclarationBlock);
328328
// These three are interdependent, if you plan to put jsmanaged data
329329
// in one of these make sure it is propagated properly to containing structs
330-
unsafe_no_jsmanaged_fields!(FrameId, FrameType, WindowSizeData, WindowSizeType, PipelineId);
330+
unsafe_no_jsmanaged_fields!(DocumentActivity, FrameId, FrameType, WindowSizeData, WindowSizeType, PipelineId);
331331
unsafe_no_jsmanaged_fields!(TimerEventId, TimerSource);
332332
unsafe_no_jsmanaged_fields!(TimelineMarkerType);
333333
unsafe_no_jsmanaged_fields!(WorkerId);

‎components/script/dom/document.rs‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ use origin::Origin;
108108
use script_layout_interface::message::{Msg, ReflowQueryType};
109109
use script_runtime::{CommonScriptMsg, ScriptThreadEventCategory};
110110
use script_thread::{MainThreadScriptMsg, Runnable};
111-
use script_traits::{AnimationState, CompositorEvent, MouseButton, MouseEventType, MozBrowserEvent};
111+
use script_traits::{AnimationState, CompositorEvent, DocumentActivity};
112+
use script_traits::{MouseButton, MouseEventType, MozBrowserEvent};
112113
use script_traits::{ScriptMsg as ConstellationMsg, TouchpadPressurePhase};
113114
use script_traits::{TouchEventType, TouchId};
114115
use script_traits::UntrustedNodeAddress;
@@ -191,7 +192,7 @@ pub struct Document {
191192
last_modified: Option<String>,
192193
encoding: Cell<EncodingRef>,
193194
is_html_document: bool,
194-
is_fully_active: Cell<bool>,
195+
activity: Cell<DocumentActivity>,
195196
url: DOMRefCell<ServoUrl>,
196197
quirks_mode: Cell<QuirksMode>,
197198
/// Caches for the getElement methods
@@ -387,17 +388,33 @@ impl Document {
387388
self.trigger_mozbrowser_event(MozBrowserEvent::SecurityChange(https_state));
388389
}
389390

390-
// https://html.spec.whatwg.org/multipage/#fully-active
391391
pub fn is_fully_active(&self) -> bool {
392-
self.is_fully_active.get()
393-
}
394-
395-
pub fn fully_activate(&self) {
396-
self.is_fully_active.set(true)
397-
}
398-
399-
pub fn fully_deactivate(&self) {
400-
self.is_fully_active.set(false)
392+
self.activity.get() == DocumentActivity::FullyActive
393+
}
394+
395+
pub fn is_active(&self) -> bool {
396+
self.activity.get() != DocumentActivity::Inactive
397+
}
398+
399+
pub fn set_activity(&self, activity: DocumentActivity) {
400+
// This function should only be called on documents with a browsing context
401+
assert!(self.browsing_context.is_some());
402+
// Set the document's activity level, reflow if necessary, and suspend or resume timers.
403+
if activity != self.activity.get() {
404+
self.activity.set(activity);
405+
if activity == DocumentActivity::FullyActive {
406+
self.title_changed();
407+
self.dirty_all_nodes();
408+
self.window().reflow(
409+
ReflowGoal::ForDisplay,
410+
ReflowQueryType::NoQuery,
411+
ReflowReason::CachedPageNeededReflow
412+
);
413+
self.window().resume();
414+
} else {
415+
self.window().suspend();
416+
}
417+
}
401418
}
402419

403420
pub fn origin(&self) -> &Origin {
@@ -1892,6 +1909,7 @@ impl Document {
18921909
is_html_document: IsHTMLDocument,
18931910
content_type: Option<DOMString>,
18941911
last_modified: Option<String>,
1912+
activity: DocumentActivity,
18951913
source: DocumentSource,
18961914
doc_loader: DocumentLoader,
18971915
referrer: Option<String>,
@@ -1927,7 +1945,7 @@ impl Document {
19271945
// https://dom.spec.whatwg.org/#concept-document-encoding
19281946
encoding: Cell::new(UTF_8),
19291947
is_html_document: is_html_document == IsHTMLDocument::HTMLDocument,
1930-
is_fully_active: Cell::new(false),
1948+
activity: Cell::new(activity),
19311949
id_map: DOMRefCell::new(HashMap::new()),
19321950
tag_map: DOMRefCell::new(HashMap::new()),
19331951
tagns_map: DOMRefCell::new(HashMap::new()),
@@ -1995,6 +2013,7 @@ impl Document {
19952013
IsHTMLDocument::NonHTMLDocument,
19962014
None,
19972015
None,
2016+
DocumentActivity::Inactive,
19982017
DocumentSource::NotFromParser,
19992018
docloader,
20002019
None,
@@ -2008,6 +2027,7 @@ impl Document {
20082027
doctype: IsHTMLDocument,
20092028
content_type: Option<DOMString>,
20102029
last_modified: Option<String>,
2030+
activity: DocumentActivity,
20112031
source: DocumentSource,
20122032
doc_loader: DocumentLoader,
20132033
referrer: Option<String>,
@@ -2020,6 +2040,7 @@ impl Document {
20202040
doctype,
20212041
content_type,
20222042
last_modified,
2043+
activity,
20232044
source,
20242045
doc_loader,
20252046
referrer,
@@ -2093,6 +2114,7 @@ impl Document {
20932114
doctype,
20942115
None,
20952116
None,
2117+
DocumentActivity::Inactive,
20962118
DocumentSource::NotFromParser,
20972119
DocumentLoader::new(&self.loader()),
20982120
None,
@@ -3249,8 +3271,7 @@ impl DocumentMethods for Document {
32493271

32503272
// Step 2.
32513273
// TODO: handle throw-on-dynamic-markup-insertion counter.
3252-
// FIXME: this should check for being active rather than fully active
3253-
if !self.is_fully_active() {
3274+
if !self.is_active() {
32543275
// Step 3.
32553276
return Ok(());
32563277
}

‎components/script/dom/domimplementation.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use dom::htmltitleelement::HTMLTitleElement;
2323
use dom::node::Node;
2424
use dom::text::Text;
2525
use dom::xmldocument::XMLDocument;
26+
use script_traits::DocumentActivity;
2627

2728
// https://dom.spec.whatwg.org/#domimplementation
2829
#[dom_struct]
@@ -83,6 +84,7 @@ impl DOMImplementationMethods for DOMImplementation {
8384
IsHTMLDocument::NonHTMLDocument,
8485
Some(DOMString::from(content_type)),
8586
None,
87+
DocumentActivity::Inactive,
8688
DocumentSource::NotFromParser,
8789
loader);
8890
// Step 2-3.
@@ -129,6 +131,7 @@ impl DOMImplementationMethods for DOMImplementation {
129131
IsHTMLDocument::HTMLDocument,
130132
None,
131133
None,
134+
DocumentActivity::Inactive,
132135
DocumentSource::NotFromParser,
133136
loader,
134137
None,

‎components/script/dom/domparser.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use dom::document::{Document, IsHTMLDocument};
1919
use dom::document::DocumentSource;
2020
use dom::servoparser::ServoParser;
2121
use dom::window::Window;
22+
use script_traits::DocumentActivity;
2223

2324
#[dom_struct]
2425
pub struct DOMParser {
@@ -65,6 +66,7 @@ impl DOMParserMethods for DOMParser {
6566
IsHTMLDocument::HTMLDocument,
6667
Some(content_type),
6768
None,
69+
DocumentActivity::Inactive,
6870
DocumentSource::FromParser,
6971
loader,
7072
None,
@@ -82,6 +84,7 @@ impl DOMParserMethods for DOMParser {
8284
IsHTMLDocument::NonHTMLDocument,
8385
Some(content_type),
8486
None,
87+
DocumentActivity::Inactive,
8588
DocumentSource::NotFromParser,
8689
loader,
8790
None,

‎components/script/dom/node.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ use ref_slice::ref_slice;
6565
use script_layout_interface::{HTMLCanvasData, OpaqueStyleAndLayoutData, SVGSVGData};
6666
use script_layout_interface::{LayoutElementType, LayoutNodeType, TrustedNodeAddress};
6767
use script_layout_interface::message::Msg;
68+
use script_traits::DocumentActivity;
6869
use script_traits::UntrustedNodeAddress;
6970
use selectors::matching::{MatchingReason, matches};
7071
use selectors::parser::SelectorList;
@@ -1730,7 +1731,8 @@ impl Node {
17301731
// https://github.com/whatwg/dom/issues/378
17311732
document.origin().alias(),
17321733
is_html_doc, None,
1733-
None, DocumentSource::NotFromParser, loader,
1734+
None, DocumentActivity::Inactive,
1735+
DocumentSource::NotFromParser, loader,
17341736
None, None);
17351737
Root::upcast::<Node>(document)
17361738
},

0 commit comments

Comments
 (0)