Skip to content

Commit bfdb529

Browse files
author
bors-servo
authored
Auto merge of #22395 - jdm:initial-iframe-size, r=Manishearth
Initialize iframe viewport immediately This is probably a significant cause of unstable test results, and it finally bothered me enough to fix it. The solution isn't great for performance (a sync layout query every time a iframe is added to a document), but performance needs to follow correctness. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14364 and fix #15689 and fix #15688. - [x] There are tests for these changes <!-- 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/22395) <!-- Reviewable:end -->
2 parents a067620 + 496e2f6 commit bfdb529

File tree

19 files changed

+210
-154
lines changed

19 files changed

+210
-154
lines changed

‎components/compositing/lib.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@ pub use crate::compositor::IOCompositor;
1111
pub use crate::compositor::RenderNotifier;
1212
pub use crate::compositor::ShutdownState;
1313
pub use crate::compositor_thread::CompositorProxy;
14-
use euclid::TypedSize2D;
1514
use ipc_channel::ipc::IpcSender;
1615
use msg::constellation_msg::PipelineId;
1716
use msg::constellation_msg::TopLevelBrowsingContextId;
1817
use script_traits::{ConstellationControlMsg, LayoutControlMsg};
19-
use style_traits::CSSPixel;
2018

2119
mod compositor;
2220
pub mod compositor_thread;
@@ -27,7 +25,6 @@ pub mod windowing;
2725

2826
pub struct SendableFrameTree {
2927
pub pipeline: CompositionPipeline,
30-
pub size: Option<TypedSize2D<f32, CSSPixel>>,
3128
pub children: Vec<SendableFrameTree>,
3229
}
3330

‎components/constellation/browsingcontext.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub struct BrowsingContext {
4141
pub top_level_id: TopLevelBrowsingContextId,
4242

4343
/// The size of the frame.
44-
pub size: Option<TypedSize2D<f32, CSSPixel>>,
44+
pub size: TypedSize2D<f32, CSSPixel>,
4545

4646
/// Whether this browsing context is in private browsing mode.
4747
pub is_private: bool,
@@ -70,6 +70,7 @@ impl BrowsingContext {
7070
top_level_id: TopLevelBrowsingContextId,
7171
pipeline_id: PipelineId,
7272
parent_pipeline_id: Option<PipelineId>,
73+
size: TypedSize2D<f32, CSSPixel>,
7374
is_private: bool,
7475
is_visible: bool,
7576
) -> BrowsingContext {
@@ -78,7 +79,7 @@ impl BrowsingContext {
7879
BrowsingContext {
7980
id: id,
8081
top_level_id: top_level_id,
81-
size: None,
82+
size: size,
8283
is_private: is_private,
8384
is_visible: is_visible,
8485
pipeline_id: pipeline_id,

‎components/constellation/constellation.rs‎

Lines changed: 56 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ where
734734
top_level_browsing_context_id: TopLevelBrowsingContextId,
735735
parent_pipeline_id: Option<PipelineId>,
736736
opener: Option<BrowsingContextId>,
737-
initial_window_size: Option<TypedSize2D<f32, CSSPixel>>,
737+
initial_window_size: TypedSize2D<f32, CSSPixel>,
738738
// TODO: we have to provide ownership of the LoadData
739739
// here, because it will be send on an ipc channel,
740740
// and ipc channels take onership of their data.
@@ -889,6 +889,7 @@ where
889889
top_level_id: TopLevelBrowsingContextId,
890890
pipeline_id: PipelineId,
891891
parent_pipeline_id: Option<PipelineId>,
892+
size: TypedSize2D<f32, CSSPixel>,
892893
is_private: bool,
893894
is_visible: bool,
894895
) {
@@ -898,6 +899,7 @@ where
898899
top_level_id,
899900
pipeline_id,
900901
parent_pipeline_id,
902+
size,
901903
is_private,
902904
is_visible,
903905
);
@@ -1599,14 +1601,20 @@ where
15991601
EmbedderMsg::Panic(reason, backtrace),
16001602
));
16011603

1602-
let browsing_context = self.browsing_contexts.get(&browsing_context_id);
1603-
let window_size = browsing_context.and_then(|ctx| ctx.size);
1604-
let pipeline_id = browsing_context.map(|ctx| ctx.pipeline_id);
1605-
let is_visible = browsing_context.map(|ctx| ctx.is_visible);
1604+
let browsing_context = match self.browsing_contexts.get(&browsing_context_id) {
1605+
Some(context) => context,
1606+
None => return warn!("failed browsing context is missing"),
1607+
};
1608+
let window_size = browsing_context.size;
1609+
let pipeline_id = browsing_context.pipeline_id;
1610+
let is_visible = browsing_context.is_visible;
16061611

1607-
let pipeline = pipeline_id.and_then(|id| self.pipelines.get(&id));
1608-
let pipeline_url = pipeline.map(|pipeline| pipeline.url.clone());
1609-
let opener = pipeline.and_then(|pipeline| pipeline.opener);
1612+
let pipeline = match self.pipelines.get(&pipeline_id) {
1613+
Some(p) => p,
1614+
None => return warn!("failed pipeline is missing"),
1615+
};
1616+
let pipeline_url = pipeline.url.clone();
1617+
let opener = pipeline.opener;
16101618

16111619
self.close_browsing_context_children(
16121620
browsing_context_id,
@@ -1616,10 +1624,8 @@ where
16161624

16171625
let failure_url = ServoUrl::parse("about:failure").expect("infallible");
16181626

1619-
if let Some(pipeline_url) = pipeline_url {
1620-
if pipeline_url == failure_url {
1621-
return error!("about:failure failed");
1622-
}
1627+
if pipeline_url == failure_url {
1628+
return error!("about:failure failed");
16231629
}
16241630

16251631
warn!("creating replacement pipeline for about:failure");
@@ -1638,7 +1644,7 @@ where
16381644
load_data,
16391645
sandbox,
16401646
is_private,
1641-
is_visible.unwrap_or(true),
1647+
is_visible,
16421648
);
16431649
self.add_pending_change(SessionHistoryChange {
16441650
top_level_browsing_context_id: top_level_browsing_context_id,
@@ -1737,7 +1743,7 @@ where
17371743
top_level_browsing_context_id,
17381744
None,
17391745
None,
1740-
Some(window_size),
1746+
window_size,
17411747
load_data,
17421748
sandbox,
17431749
is_private,
@@ -3275,6 +3281,7 @@ where
32753281
change.top_level_browsing_context_id,
32763282
change.new_pipeline_id,
32773283
new_context_info.parent_pipeline_id,
3284+
self.window_size.initial_viewport, //XXXjdm is this valid?
32783285
new_context_info.is_private,
32793286
new_context_info.is_visible,
32803287
);
@@ -3612,44 +3619,41 @@ where
36123619
}
36133620

36143621
// Check the visible rectangle for this pipeline. If the constellation has received a
3615-
// size for the pipeline, then its painting should be up to date. If the constellation
3616-
// *hasn't* received a size, it could be that the layer was hidden by script before the
3617-
// compositor discovered it, so we just don't check the layer.
3618-
if let Some(size) = browsing_context.size {
3619-
// If the rectangle for this pipeline is zero sized, it will
3620-
// never be painted. In this case, don't query the layout
3621-
// thread as it won't contribute to the final output image.
3622-
if size == TypedSize2D::zero() {
3623-
continue;
3624-
}
3622+
// size for the pipeline, then its painting should be up to date.
3623+
//
3624+
// If the rectangle for this pipeline is zero sized, it will
3625+
// never be painted. In this case, don't query the layout
3626+
// thread as it won't contribute to the final output image.
3627+
if browsing_context.size == TypedSize2D::zero() {
3628+
continue;
3629+
}
36253630

3626-
// Get the epoch that the compositor has drawn for this pipeline.
3627-
let compositor_epoch = pipeline_states.get(&browsing_context.pipeline_id);
3628-
match compositor_epoch {
3629-
Some(compositor_epoch) => {
3630-
// Synchronously query the layout thread to see if the current
3631-
// epoch matches what the compositor has drawn. If they match
3632-
// (and script is idle) then this pipeline won't change again
3633-
// and can be considered stable.
3634-
let message = LayoutControlMsg::GetCurrentEpoch(epoch_sender.clone());
3635-
if let Err(e) = pipeline.layout_chan.send(message) {
3636-
warn!("Failed to send GetCurrentEpoch ({}).", e);
3637-
}
3638-
match epoch_receiver.recv() {
3639-
Err(e) => warn!("Failed to receive current epoch ({}).", e),
3640-
Ok(layout_thread_epoch) => {
3641-
if layout_thread_epoch != *compositor_epoch {
3642-
return ReadyToSave::EpochMismatch;
3643-
}
3644-
},
3645-
}
3646-
},
3647-
None => {
3648-
// The compositor doesn't know about this pipeline yet.
3649-
// Assume it hasn't rendered yet.
3650-
return ReadyToSave::PipelineUnknown;
3651-
},
3652-
}
3631+
// Get the epoch that the compositor has drawn for this pipeline.
3632+
let compositor_epoch = pipeline_states.get(&browsing_context.pipeline_id);
3633+
match compositor_epoch {
3634+
Some(compositor_epoch) => {
3635+
// Synchronously query the layout thread to see if the current
3636+
// epoch matches what the compositor has drawn. If they match
3637+
// (and script is idle) then this pipeline won't change again
3638+
// and can be considered stable.
3639+
let message = LayoutControlMsg::GetCurrentEpoch(epoch_sender.clone());
3640+
if let Err(e) = pipeline.layout_chan.send(message) {
3641+
warn!("Failed to send GetCurrentEpoch ({}).", e);
3642+
}
3643+
match epoch_receiver.recv() {
3644+
Err(e) => warn!("Failed to receive current epoch ({}).", e),
3645+
Ok(layout_thread_epoch) => {
3646+
if layout_thread_epoch != *compositor_epoch {
3647+
return ReadyToSave::EpochMismatch;
3648+
}
3649+
},
3650+
}
3651+
},
3652+
None => {
3653+
// The compositor doesn't know about this pipeline yet.
3654+
// Assume it hasn't rendered yet.
3655+
return ReadyToSave::PipelineUnknown;
3656+
},
36533657
}
36543658
}
36553659

@@ -3715,7 +3719,7 @@ where
37153719
browsing_context_id: BrowsingContextId,
37163720
) {
37173721
if let Some(browsing_context) = self.browsing_contexts.get_mut(&browsing_context_id) {
3718-
browsing_context.size = Some(new_size.initial_viewport);
3722+
browsing_context.size = new_size.initial_viewport;
37193723
// Send Resize (or ResizeInactive) messages to each pipeline in the frame tree.
37203724
let pipeline_id = browsing_context.pipeline_id;
37213725
let pipeline = match self.pipelines.get(&pipeline_id) {
@@ -4001,7 +4005,6 @@ where
40014005
.map(|pipeline| {
40024006
let mut frame_tree = SendableFrameTree {
40034007
pipeline: pipeline.to_sendable(),
4004-
size: browsing_context.size,
40054008
children: vec![],
40064009
};
40074010

‎components/constellation/pipeline.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ pub struct InitialPipelineState {
152152
pub mem_profiler_chan: profile_mem::ProfilerChan,
153153

154154
/// Information about the initial window size.
155-
pub window_size: Option<TypedSize2D<f32, CSSPixel>>,
155+
pub window_size: TypedSize2D<f32, CSSPixel>,
156156

157157
/// Information about the device pixel ratio.
158158
pub device_pixel_ratio: TypedScale<f32, CSSPixel, DevicePixel>,
@@ -200,11 +200,10 @@ impl Pipeline {
200200
let (layout_content_process_shutdown_chan, layout_content_process_shutdown_port) =
201201
ipc::channel().expect("Pipeline layout content shutdown chan");
202202

203-
let device_pixel_ratio = state.device_pixel_ratio;
204-
let window_size = state.window_size.map(|size| WindowSizeData {
205-
initial_viewport: size,
206-
device_pixel_ratio: device_pixel_ratio,
207-
});
203+
let window_size = WindowSizeData {
204+
initial_viewport: state.window_size,
205+
device_pixel_ratio: state.device_pixel_ratio,
206+
};
208207

209208
let url = state.load_data.url.clone();
210209

@@ -475,7 +474,7 @@ pub struct UnprivilegedPipelineContent {
475474
resource_threads: ResourceThreads,
476475
time_profiler_chan: time::ProfilerChan,
477476
mem_profiler_chan: profile_mem::ProfilerChan,
478-
window_size: Option<WindowSizeData>,
477+
window_size: WindowSizeData,
479478
script_chan: IpcSender<ConstellationControlMsg>,
480479
load_data: LoadData,
481480
script_port: IpcReceiver<ConstellationControlMsg>,

‎components/gfx/platform/macos/font.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,15 +239,14 @@ impl FontHandleMethods for FontHandle {
239239

240240
let result = unsafe {
241241
self.ctfont
242-
.get_glyphs_for_characters(&characters[0], &mut glyphs[0], count)
242+
.get_glyphs_for_characters(characters.as_ptr(), glyphs.as_mut_ptr(), count)
243243
};
244244

245-
if !result {
245+
if !result || glyphs[0] == 0 {
246246
// No glyph for this character
247247
return None;
248248
}
249249

250-
assert_ne!(glyphs[0], 0); // FIXME: error handling
251250
return Some(glyphs[0] as GlyphId);
252251
}
253252

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44

55
//! Base classes to work with IDL callbacks.
66
7+
use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
78
use crate::dom::bindings::error::{report_pending_exception, Error, Fallible};
9+
use crate::dom::bindings::inheritance::Castable;
810
use crate::dom::bindings::reflector::DomObject;
911
use crate::dom::bindings::root::{Dom, DomRoot};
1012
use crate::dom::bindings::settings_stack::{AutoEntryScript, AutoIncumbentScript};
1113
use crate::dom::bindings::utils::AsCCharPtrPtr;
1214
use crate::dom::globalscope::GlobalScope;
15+
use crate::dom::window::Window;
1316
use js::jsapi::Heap;
1417
use js::jsapi::JSAutoCompartment;
1518
use js::jsapi::{AddRawValueRoot, IsCallable, JSContext, JSObject};
@@ -242,6 +245,9 @@ impl CallSetup {
242245
#[allow(unrooted_must_root)]
243246
pub fn new<T: CallbackContainer>(callback: &T, handling: ExceptionHandling) -> CallSetup {
244247
let global = unsafe { GlobalScope::from_object(callback.callback()) };
248+
if let Some(window) = global.downcast::<Window>() {
249+
window.Document().ensure_safe_to_run_script_or_layout();
250+
}
245251
let cx = global.get_cx();
246252

247253
let aes = AutoEntryScript::new(&global);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ use crate::dom::bindings::utils::WindowProxyHandler;
4949
use crate::dom::document::PendingRestyle;
5050
use crate::dom::htmlimageelement::SourceSet;
5151
use crate::dom::htmlmediaelement::MediaFrameRenderer;
52+
use crate::task::TaskBox;
5253
use crossbeam_channel::{Receiver, Sender};
5354
use cssparser::RGBA;
5455
use devtools_traits::{CSSError, TimelineMarkerType, WorkerId};
@@ -139,6 +140,8 @@ pub unsafe trait JSTraceable {
139140
unsafe fn trace(&self, trc: *mut JSTracer);
140141
}
141142

143+
unsafe_no_jsmanaged_fields!(Box<dyn TaskBox>);
144+
142145
unsafe_no_jsmanaged_fields!(CSSError);
143146

144147
unsafe_no_jsmanaged_fields!(&'static Encoding);

0 commit comments

Comments
 (0)