-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
servoshell: Move Minibrowser to headed window
#40792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
servoshell: Move Minibrowser to headed window
#40792
Conversation
|
🔨 Triggering try run (#19570325686) for Linux (WPT) |
|
I don't think CI WPT tests can cover this change. |
I just wanted to ensure that my changes did not interfere with functioning of headless mode which is used by WPT. |
yezhizhen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This make things much cleaner.
|
Test results for linux-wpt from try job (#19570325686): Flaky unexpected result (33)
Stable unexpected results that are known to be intermittent (27)
|
|
✨ Try run (#19570325686) succeeded. |
Instead of storing the `Minibrowser` egui struct in the `RunningAppState` which is used for both headed and headless windows, move it to the headed window implementation. This addresses a lot of bits of the code that would need to test the existance of the `Minibrowser` even when it should have always existed. In addition, event handling for the headed window is moved into the headed window implementation itself, which eliminates the need to expose so many methods in `WindowPortsMethods`. One downside is that it is impossible to support tracing of `AppEvent`s as accessibilty events are not cloneable. This seems like a small price to pay for this cleanup though. This is the first step toward supporting multiple windows in servoshell. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
a428595 to
328f385
Compare
Instead of storing the
Minibrowseregui struct in theRunningAppStatewhich is used for both headed and headless windows,move it to the headed window implementation. This addresses a lot of
bits of the code that would need to test the existance of the
Minibrowsereven when it should have always existed.In addition, event handling for the headed window is moved into the
headed window implementation itself, which eliminates the need to expose
so many methods in
WindowPortsMethods.One downside is that it is impossible to support tracing of
AppEventsas accessibilty events are not cloneable. This seems like a small price
to pay for this cleanup though.
This is the first step toward supporting multiple windows in servoshell.
Testing: This should not change behavior and is thus covered by existing tests.