Auto merge of #23128 - kamal-umudlu:pass_not_none_value_in_setfullscreen, r=jdm

Pass not none value in setfullscreen

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

Entering fullscreen mode is passing `None` value to `Window` when `set_fullscreen` function is called which prevents
Servo actually entering fullscreen mode.
In addition, the function `exit_fullscreen` in `document.rs` is passing True value to
`SetFullscreenState` which doesn't allow to exit from fullscreen mode.

# Solution

1. Instead of passing `None` value when `FullScreenState` is true, `window.get_primary_monitor()` is called in order to pass a monitor id.
This fix make Servo actually enter fullscreen mode.
2. Changed `SetFullscreenState` to false when `exit_fullscreen` function is called.
3. In addition, added new implementation to support exiting from fullscreen mode by pressing `Escape` button.

# Testing Plan

After my change in [windows.rs and document.rs](af6b598154),
the Servo app can enter/exit fullscreen mode.
In addition, the [`ESC button support`](14ebd5bbb0)
allows to exit from fullscreenmode.

---
<!-- 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 #22853

<!-- Either: -->
- [X] 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/23128)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-04-01 20:57:07 -04:00 committed by GitHub
commit c5da3306b2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 81 additions and 3 deletions

View file

@ -78,6 +78,8 @@ pub enum WindowEvent {
Navigation(TopLevelBrowsingContextId, TraversalDirection), Navigation(TopLevelBrowsingContextId, TraversalDirection),
/// Sent when the user quits the application /// Sent when the user quits the application
Quit, Quit,
/// Sent when the user exits from fullscreen mode
ExitFullScreen(TopLevelBrowsingContextId),
/// Sent when a key input state changes /// Sent when a key input state changes
Keyboard(KeyboardEvent), Keyboard(KeyboardEvent),
/// Sent when Ctr+R/Apple+R is called to reload the current page. /// Sent when Ctr+R/Apple+R is called to reload the current page.
@ -125,6 +127,7 @@ impl Debug for WindowEvent {
WindowEvent::ToggleWebRenderDebug(..) => write!(f, "ToggleWebRenderDebug"), WindowEvent::ToggleWebRenderDebug(..) => write!(f, "ToggleWebRenderDebug"),
WindowEvent::CaptureWebRender => write!(f, "CaptureWebRender"), WindowEvent::CaptureWebRender => write!(f, "CaptureWebRender"),
WindowEvent::ToggleSamplingProfiler(..) => write!(f, "ToggleSamplingProfiler"), WindowEvent::ToggleSamplingProfiler(..) => write!(f, "ToggleSamplingProfiler"),
WindowEvent::ExitFullScreen(..) => write!(f, "ExitFullScreen"),
} }
} }
} }

View file

@ -1230,6 +1230,9 @@ where
} }
} }
}, },
FromCompositorMsg::ExitFullScreen(top_level_browsing_context_id) => {
self.handle_exit_fullscreen_msg(top_level_browsing_context_id);
},
} }
} }
@ -3664,6 +3667,15 @@ where
self.window_size = new_size; self.window_size = new_size;
} }
/// Called when the window exits from fullscreen mode
fn handle_exit_fullscreen_msg(
&mut self,
top_level_browsing_context_id: TopLevelBrowsingContextId,
) {
let browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
self.switch_fullscreen_mode(browsing_context_id);
}
/// Handle updating actual viewport / zoom due to @viewport rules /// Handle updating actual viewport / zoom due to @viewport rules
fn handle_viewport_constrained_msg( fn handle_viewport_constrained_msg(
&mut self, &mut self,
@ -3900,6 +3912,25 @@ where
} }
} }
// Handle switching from fullscreen mode
fn switch_fullscreen_mode(&mut self, browsing_context_id: BrowsingContextId) {
if let Some(browsing_context) = self.browsing_contexts.get(&browsing_context_id) {
let pipeline_id = browsing_context.pipeline_id;
let pipeline = match self.pipelines.get(&pipeline_id) {
None => {
return warn!(
"Pipeline {:?} switched from fullscreen mode after closing.",
pipeline_id
)
},
Some(pipeline) => pipeline,
};
let _ = pipeline
.event_loop
.send(ConstellationControlMsg::ExitFullScreen(pipeline.id));
}
}
// Close a browsing context (and all children) // Close a browsing context (and all children)
fn close_browsing_context( fn close_browsing_context(
&mut self, &mut self,

View file

@ -3211,7 +3211,7 @@ impl Document {
let window = self.window(); let window = self.window();
// Step 8 // Step 8
let event = EmbedderMsg::SetFullscreenState(true); let event = EmbedderMsg::SetFullscreenState(false);
self.send_to_embedder(event); self.send_to_embedder(event);
// Step 9 // Step 9

View file

@ -1304,6 +1304,10 @@ impl ScriptThread {
// An event came-in from a document that is not fully-active, it has been stored by the task-queue. // An event came-in from a document that is not fully-active, it has been stored by the task-queue.
// Continue without adding it to "sequential". // Continue without adding it to "sequential".
}, },
FromConstellation(ConstellationControlMsg::ExitFullScreen(id)) => self
.profile_event(ScriptThreadEventCategory::ExitFullscreen, Some(id), || {
self.handle_exit_fullscreen(id);
}),
_ => { _ => {
sequential.push(event); sequential.push(event);
}, },
@ -1500,6 +1504,7 @@ impl ScriptThread {
Reload(id, ..) => Some(id), Reload(id, ..) => Some(id),
WebVREvents(id, ..) => Some(id), WebVREvents(id, ..) => Some(id),
PaintMetric(..) => None, PaintMetric(..) => None,
ExitFullScreen(id, ..) => Some(id),
} }
}, },
MixedMessage::FromDevtools(_) => None, MixedMessage::FromDevtools(_) => None,
@ -1731,6 +1736,7 @@ impl ScriptThread {
msg @ ConstellationControlMsg::Viewport(..) | msg @ ConstellationControlMsg::Viewport(..) |
msg @ ConstellationControlMsg::SetScrollState(..) | msg @ ConstellationControlMsg::SetScrollState(..) |
msg @ ConstellationControlMsg::Resize(..) | msg @ ConstellationControlMsg::Resize(..) |
msg @ ConstellationControlMsg::ExitFullScreen(..) |
msg @ ConstellationControlMsg::ExitScriptThread => { msg @ ConstellationControlMsg::ExitScriptThread => {
panic!("should have handled {:?} already", msg) panic!("should have handled {:?} already", msg)
}, },
@ -1953,6 +1959,19 @@ impl ScriptThread {
warn!("resize sent to nonexistent pipeline"); warn!("resize sent to nonexistent pipeline");
} }
// exit_fullscreen creates a new JS promise object, so we need to have entered a compartment
fn handle_exit_fullscreen(&self, id: PipelineId) {
let document = self.documents.borrow().find_document(id);
if let Some(document) = document {
let _ac = JSAutoCompartment::new(
document.global().get_cx(),
document.reflector().get_jsobject().get(),
);
document.exit_fullscreen();
return;
}
}
fn handle_viewport(&self, id: PipelineId, rect: Rect<f32>) { fn handle_viewport(&self, id: PipelineId, rect: Rect<f32>) {
let document = self.documents.borrow().find_document(id); let document = self.documents.borrow().find_document(id);
if let Some(document) = document { if let Some(document) = document {

View file

@ -262,6 +262,8 @@ pub enum ConstellationControlMsg {
Resize(PipelineId, WindowSizeData, WindowSizeType), Resize(PipelineId, WindowSizeData, WindowSizeType),
/// Notifies script that window has been resized but to not take immediate action. /// Notifies script that window has been resized but to not take immediate action.
ResizeInactive(PipelineId, WindowSizeData), ResizeInactive(PipelineId, WindowSizeData),
/// Window switched from fullscreen mode.
ExitFullScreen(PipelineId),
/// Notifies the script that the document associated with this pipeline should 'unload'. /// Notifies the script that the document associated with this pipeline should 'unload'.
UnloadDocument(PipelineId), UnloadDocument(PipelineId),
/// Notifies the script that a pipeline should be closed. /// Notifies the script that a pipeline should be closed.
@ -388,6 +390,7 @@ impl fmt::Debug for ConstellationControlMsg {
Reload(..) => "Reload", Reload(..) => "Reload",
WebVREvents(..) => "WebVREvents", WebVREvents(..) => "WebVREvents",
PaintMetric(..) => "PaintMetric", PaintMetric(..) => "PaintMetric",
ExitFullScreen(..) => "ExitFullScreen",
}; };
write!(formatter, "ConstellationControlMsg::{}", variant) write!(formatter, "ConstellationControlMsg::{}", variant)
} }
@ -782,6 +785,8 @@ pub enum ConstellationMsg {
EnableProfiler(Duration, Duration), EnableProfiler(Duration, Duration),
/// Disable the sampling profiler. /// Disable the sampling profiler.
DisableProfiler, DisableProfiler,
/// Request to exit from fullscreen mode
ExitFullScreen(TopLevelBrowsingContextId),
} }
impl fmt::Debug for ConstellationMsg { impl fmt::Debug for ConstellationMsg {
@ -811,6 +816,7 @@ impl fmt::Debug for ConstellationMsg {
SetCursor(..) => "SetCursor", SetCursor(..) => "SetCursor",
EnableProfiler(..) => "EnableProfiler", EnableProfiler(..) => "EnableProfiler",
DisableProfiler => "DisableProfiler", DisableProfiler => "DisableProfiler",
ExitFullScreen(..) => "ExitFullScreen",
}; };
write!(formatter, "ConstellationMsg::{}", variant) write!(formatter, "ConstellationMsg::{}", variant)
} }

View file

@ -401,6 +401,13 @@ where
self.compositor.maybe_start_shutting_down(); self.compositor.maybe_start_shutting_down();
}, },
WindowEvent::ExitFullScreen(top_level_browsing_context_id) => {
let msg = ConstellationMsg::ExitFullScreen(top_level_browsing_context_id);
if let Err(e) = self.constellation_chan.send(msg) {
warn!("Sending exit fullscreen to constellation failed ({:?}).", e);
}
},
WindowEvent::Reload(top_level_browsing_context_id) => { WindowEvent::Reload(top_level_browsing_context_id) => {
let msg = ConstellationMsg::Reload(top_level_browsing_context_id); let msg = ConstellationMsg::Reload(top_level_browsing_context_id);
if let Err(e) = self.constellation_chan.send(msg) { if let Err(e) = self.constellation_chan.send(msg) {

View file

@ -159,7 +159,15 @@ impl Browser {
} }
}) })
.shortcut(Modifiers::empty(), Key::Escape, || { .shortcut(Modifiers::empty(), Key::Escape, || {
let state = self.window.get_fullscreen();
if state {
if let Some(id) = self.browser_id {
let event = WindowEvent::ExitFullScreen(id);
self.event_queue.push(event);
}
} else {
self.event_queue.push(WindowEvent::Quit); self.event_queue.push(WindowEvent::Quit);
}
}) })
.otherwise(|| self.platform_handle_key(key_event)); .otherwise(|| self.platform_handle_key(key_event));
} }

View file

@ -329,7 +329,7 @@ impl Window {
match self.kind { match self.kind {
WindowKind::Window(ref window, ..) => { WindowKind::Window(ref window, ..) => {
if self.fullscreen.get() != state { if self.fullscreen.get() != state {
window.set_fullscreen(None); window.set_fullscreen(Some(window.get_primary_monitor()));
} }
}, },
WindowKind::Headless(..) => {}, WindowKind::Headless(..) => {},
@ -337,6 +337,10 @@ impl Window {
self.fullscreen.set(state); self.fullscreen.set(state);
} }
pub fn get_fullscreen(&self) -> bool {
return self.fullscreen.get();
}
fn is_animating(&self) -> bool { fn is_animating(&self) -> bool {
self.animation_state.get() == AnimationState::Animating && !self.suspended.get() self.animation_state.get() == AnimationState::Animating && !self.suspended.get()
} }