Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upFix mocking and session ending event loop models #25
Conversation
|
|
|
OK I am confused about what's going on here. Why is the headless discovery impl spawning a thread? |
| } | ||
| fn set_quitter(&mut self, _: Quitter) { | ||
| // Glwindow currently doesn't have any way to end its own session | ||
| // XXXManishearth add something for this that listens for the window |
This comment has been minimized.
This comment has been minimized.
asajeffrey
Jul 24, 2019
Member
Yeah, this is annoying. Glutin only has one event loop, which makes it difficult to support examples like this, where events on the servo window should go to servo, and events on the glwindow window should go to it.
| /// Quit the session | ||
| fn quit(&mut self); | ||
|
|
||
| fn set_quitter(&mut self, quitter: Quitter); |
This comment has been minimized.
This comment has been minimized.
asajeffrey
Jul 24, 2019
Member
Hmm, this is annoying, it would be nice if we could just generate an end event for this, but that goes to script rather than the session thread. We could just use a polling+waker API, which is what Rust futures use.
We can just land this for now, perhaps with an issue to remind us to revisit it?
This comment has been minimized.
This comment has been minimized.
Manishearth
Jul 24, 2019
Author
Member
The quitter is basically a glorified waker, scoped so that all it can do is send quit events
This comment has been minimized.
This comment has been minimized.
asajeffrey
Jul 24, 2019
Member
I think we're going to need a general MainThreadWaker anyway, perhaps Quitter could just use that?
| let data_ = data.clone(); | ||
|
|
||
| thread::spawn(move || { | ||
| run_loop(receiver, data_); |
This comment has been minimized.
This comment has been minimized.
asajeffrey
Jul 24, 2019
Member
OK I am confused by this, why is HeadlessMockDiscovery spinning up a thread?
This comment has been minimized.
This comment has been minimized.
Manishearth
Jul 24, 2019
Author
Member
We need to be able to request sessions to the same device multiple times. The device is longer lived -- if a session ends, the device sticks around, but if the device disconnects you have to rerun simulatedeviceconnection.
Given that simulateDeviceConnection spawns a Discovery, the model I'm following is that each Discovery is a single device (which is forever lost once disconnected, though it stays in the array as an inert Discovery object)
This comment has been minimized.
This comment has been minimized.
asajeffrey
Jul 24, 2019
Member
Hmm, that's a different model than I was thinking of, which is that each Device object is only as long-lived as its session, so it made sense to have it owned by the SessionThread. If nothing else, that means the name isn't ideal, as Device should be ActiveDevice?
This comment has been minimized.
This comment has been minimized.
Manishearth
Jul 24, 2019
Author
Member
This won't work, because you need to be able to persist mock device state between sessions.
This comment has been minimized.
This comment has been minimized.
Manishearth
Jul 24, 2019
Author
Member
You're right, the Device trait is the session, and we could rename it. But we still need a separate event loop here.
| })) | ||
| } | ||
| } | ||
|
|
||
| fn run_loop(receiver: Receiver<MockDeviceMsg>, data: Arc<Mutex<HeadlessDeviceData>>) { | ||
| while let Ok(msg) = receiver.recv() { |
This comment has been minimized.
This comment has been minimized.
asajeffrey
Jul 24, 2019
Member
I don't understand why headless discovery is using its own event loop.
|
|
Manishearth
force-pushed the
loopy
branch
2 times, most recently
from
120f39f
to
37fe820
|
OK, at this point I'm okay with landing this and revisiting it once we've replaced |
|
|
|
Ready to land, waiting for the servo side of your PRs to make it first. |
|
@bors-servo r=asajeffrey |
|
|
Fix mocking and session ending event loop models r? @asajeffrey
|
|
Improve session test lifecycle code Requires servo/webxr#25 Fixes #23796, hopefully r? @asajeffrey <!-- 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/23839) <!-- Reviewable:end -->
Improve session test lifecycle code Requires servo/webxr#25 Fixes #23796, hopefully r? @asajeffrey <!-- 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/23839) <!-- Reviewable:end -->
Manishearth commentedJul 23, 2019
r? @asajeffrey