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 upallow_navigation: use channel to send response #17684
Conversation
|
@cbrewster do you think this makes sense? We haven't used channels in WindowMethods yet. |
|
|
|
@paulrouget what is the advantage of making the embedder handle the channel? It seems like this would add extra complexity for the embedder. |
|
@cbrewster it doesn't require the embedder to return the boolean right away. It can be done later. For example, if the embedder wants to ask the user, with a prompt, if he agrees to navigate away or not, if we do not use a channel, we have to block With a channel, we don't have to block. Does it make sense?
Yes. But at this stage, we don't try to make the embedding API "simple". We will later work on the cosmetic part of the API to make things easier. |
|
Sounds reasonable to me. |
|
|
|
|
|
@bors-servo delegate+ |
|
|
paulrouget
force-pushed the
paulrouget:fix_allow_navigation
branch
from
3c95200
to
cd3172c
|
Rebased. Also fixed cef. @bors-servo r+ |
|
|
allow_navigation: use channel to send response From the embedder perspective, this makes things easier in term of synchronicity. --- <!-- 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 - [ ] These changes fix #__ (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/17684) <!-- Reviewable:end -->
|
|
paulrouget commentedJul 12, 2017
•
edited by larsbergstrom
From the embedder perspective, this makes things easier in term of synchronicity.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is