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 upLet the embedder decide if servo should follow a link or not #15795
Conversation
highfive
commented
Mar 2, 2017
|
Heads up! This PR modifies the following files:
|
|
@glennw review ping |
|
Sorry, been OOO for a few days. I'm a bit confused about why the compositor is involved with making navigation decisions. Is this because all communication between the constellation and the embedding application goes via the compositor? |
Yes. If I'm not mistaken, only the compositor has access to the embedder window and can wake up the main thread. |
|
Hmm, this seems to be conflating two (at least conceptually) different threads: the compositor, and the embedding application. Are these separate entities? Certainly the naming implies that the compositor ix part of servo, whereas the embedding application is a third party. |
|
Can you recommend a different approach? How can the constellation make as synchronous request to the embedder? |
|
I think the basic approach of using an rpc-like call using channels is right, it's just that it's odd conflating the compositor and the embedder. They're running in the same thread, but the compositor is written by us, and the embedder is a third party. It might be worth using different channels for these? |
|
It seems to me that it's a more general problem. The compositor make some other calls like Is it ok if we land this as it is, and have a specific issue for creating a dedicated channel to communicate with the embedder? |
|
I filed #15934 |
|
Back from being ill :/ Thanks for filing the issue, it sounds like we're in agreement about where we'd like to go, so I'm happy to land this as-is and postpone thinking about the embedder/compositor split. @bors-servo r+ |
|
|
Let the embedder decide if servo should follow a link or not We want to give a chance to the embedder to handle a link itself. Is it a problem that this will add a round trip to the main thread every time `load_url` is called? --- <!-- 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 #15655 <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because I'm not sure how to test that <!-- 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/15795) <!-- Reviewable:end -->
|
|
paulrouget commentedMar 2, 2017
•
edited by larsbergstrom
We want to give a chance to the embedder to handle a link itself.
Is it a problem that this will add a round trip to the main thread every time
load_urlis called?./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is