Auto merge of #18514 - asajeffrey:layout-dont-panic-if-no-iframe-bc, r=emilio

Remove sources of panic when laying out an iframe without a nested browsing context

<!-- Please describe your changes on the following line: -->

At the moment, layout panics if it discovers an iframe without a nested browsing context. Under normal circumstances, this is reasonable, but it requires very tight synchronization between script, layout, the constellation and the compositor. In particular, if a layout is in progress when an iframe's browsing context is discarded, this can trigger panic.

This PR fixes this in two ways:

1. Making the pipeline and browsing context ids optional in layout's representation of an iframe.
2. Shutting down layout before discarding a browsing context, rather than after.

This is belt and braces.

---
<!-- 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 #17482 and #18477
- [X] These changes do not require tests because the PR is fixing a panic caused by a race condition

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/18514)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-22 09:34:12 -05:00 committed by GitHub
commit c48ef50b72
6 changed files with 58 additions and 30 deletions

View file

@ -2022,6 +2022,15 @@ impl FragmentDisplayListBuilding for Fragment {
}
SpecificFragmentInfo::Iframe(ref fragment_info) => {
if !stacking_relative_content_box.is_empty() {
let browsing_context_id = match fragment_info.browsing_context_id {
Some(browsing_context_id) => browsing_context_id,
None => return warn!("No browsing context id for iframe."),
};
let pipeline_id = match fragment_info.pipeline_id {
Some(pipeline_id) => pipeline_id,
None => return warn!("No pipeline id for iframe {}.", browsing_context_id),
};
let base = state.create_base_display_item(
&stacking_relative_content_box,
build_local_clip(&self.style),
@ -2030,12 +2039,12 @@ impl FragmentDisplayListBuilding for Fragment {
DisplayListSection::Content);
let item = DisplayItem::Iframe(box IframeDisplayItem {
base: base,
iframe: fragment_info.pipeline_id,
iframe: pipeline_id,
});
let size = Size2D::new(item.bounds().size.width.to_f32_px(),
item.bounds().size.height.to_f32_px());
state.iframe_sizes.push((fragment_info.browsing_context_id,
state.iframe_sizes.push((browsing_context_id,
TypedSize2D::from_untyped(&size)));
state.add_display_item(item);

View file

@ -488,10 +488,10 @@ impl ImageFragmentInfo {
/// size of this iframe can be communicated via the constellation to the iframe's own layout thread.
#[derive(Clone)]
pub struct IframeFragmentInfo {
/// The frame ID of this iframe.
pub browsing_context_id: BrowsingContextId,
/// The pipelineID of this iframe.
pub pipeline_id: PipelineId,
/// The frame ID of this iframe. None if there is no nested browsing context.
pub browsing_context_id: Option<BrowsingContextId>,
/// The pipelineID of this iframe. None if there is no nested browsing context.
pub pipeline_id: Option<PipelineId>,
}
impl IframeFragmentInfo {