Auto merge of #18149 - paulrouget:hittest-issue, r=cbrewster

make sure proper frame tree is sent when iframes change

We ran into a hittest issue because the frame tree was not correctly updated after #17425.

The frame tree should be updated on tab change (via send_frame_tree) or when the frame sub tree changes and it's the active one (via update_frame_tree_if_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 #18101 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/18149)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-08-21 03:02:21 -05:00 committed by GitHub
commit a1006a52a7

View file

@ -2307,12 +2307,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
self.update_activity(pipeline_id); self.update_activity(pipeline_id);
self.notify_history_changed(top_level_id); self.notify_history_changed(top_level_id);
// Set paint permissions correctly for the compositor layers. self.update_frame_tree_if_active(top_level_id);
if let Some(id) = self.active_browser_id {
if id == top_level_id {
self.send_frame_tree(top_level_id);
}
}
// Update the owning iframe to point to the new pipeline id. // Update the owning iframe to point to the new pipeline id.
// This makes things like contentDocument work correctly. // This makes things like contentDocument work correctly.
@ -2470,12 +2465,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
self.trigger_mozbrowserlocationchange(change.top_level_browsing_context_id); self.trigger_mozbrowserlocationchange(change.top_level_browsing_context_id);
} }
// Build frame tree self.update_frame_tree_if_active(change.top_level_browsing_context_id);
if let Some(id) = self.active_browser_id {
if id == change.top_level_browsing_context_id {
self.send_frame_tree(change.top_level_browsing_context_id );
}
}
} }
fn handle_activate_document_msg(&mut self, pipeline_id: PipelineId) { fn handle_activate_document_msg(&mut self, pipeline_id: PipelineId) {
@ -2908,12 +2898,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
}) })
} }
// Send the current frame tree to compositor /// Re-send the frame tree to the compositor.
fn send_frame_tree(&mut self, top_level_browsing_context_id: TopLevelBrowsingContextId) { fn update_frame_tree_if_active(&mut self, mut top_level_browsing_context_id: TopLevelBrowsingContextId) {
// This might be a mozbrowser iframe, so we need to climb the parent hierarchy, // This might be a mozbrowser iframe, so we need to climb the parent hierarchy,
// even though it's a top-level browsing context. // even though it's a top-level browsing context.
self.active_browser_id = Some(top_level_browsing_context_id); // FIXME(paul): to remove once mozbrowser API is removed.
let mut browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id); let browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
let mut pipeline_id = match self.browsing_contexts.get(&browsing_context_id) { let mut pipeline_id = match self.browsing_contexts.get(&browsing_context_id) {
Some(browsing_context) => browsing_context.pipeline_id, Some(browsing_context) => browsing_context.pipeline_id,
None => return warn!("Sending frame tree for discarded browsing context {}.", browsing_context_id), None => return warn!("Sending frame tree for discarded browsing context {}.", browsing_context_id),
@ -2923,12 +2913,25 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
match pipeline.parent_info { match pipeline.parent_info {
Some((parent_id, _)) => pipeline_id = parent_id, Some((parent_id, _)) => pipeline_id = parent_id,
None => { None => {
browsing_context_id = pipeline.browsing_context_id; top_level_browsing_context_id = pipeline.top_level_browsing_context_id;
break; break;
}, },
} }
} }
// Only send the frame tree if it's the active one or if no frame tree
// has been sent yet.
if self.active_browser_id.is_none() || Some(top_level_browsing_context_id) == self.active_browser_id {
self.send_frame_tree(top_level_browsing_context_id);
}
}
/// Send the current frame tree to compositor
fn send_frame_tree(&mut self, top_level_browsing_context_id: TopLevelBrowsingContextId) {
self.active_browser_id = Some(top_level_browsing_context_id);
let browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
// Note that this function can panic, due to ipc-channel creation failure. // Note that this function can panic, due to ipc-channel creation failure.
// avoiding this panic would require a mechanism for dealing // avoiding this panic would require a mechanism for dealing
// with low-resource scenarios. // with low-resource scenarios.