Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Tabs support: add methods to create additional top level browsing context #17226

Closed
paulrouget opened this issue Jun 8, 2017 · 4 comments
Closed

Tabs support: add methods to create additional top level browsing context #17226

paulrouget opened this issue Jun 8, 2017 · 4 comments
Assignees
@paulrouget
Labels
A-embedding C-assigned There is someone working on resolving the issue

Comments

@paulrouget
Copy link
Contributor

Now that #17077 has landed, and after #17200 and #16477 are fixed, we will need a way to create new top level browsing context.

We will need new Browser methods. This will impact the initialization code. And Browser will need to be renamed to something else as we will be create multiple browsers.
@paulrouget paulrouget added A-embedding C-assigned There is someone working on resolving the issue labels Jun 8, 2017
@paulrouget paulrouget self-assigned this Jun 8, 2017
@paulrouget paulrouget mentioned this issue Jun 20, 2017
Merged
@gterzian
Copy link
Member

gterzian commented Jul 12, 2017 •
edited

Somewhat unrelated to this issue, but I wasn't sure where else to post it. Came across this project which is an interesting embedding use case: https://github.com/scrapinghub/splash/

They're using PyQt5, which seems to offer a kind of windowing wrapper around Webkit.

The project has it's own "Tab" solution, implemented in Python, which wraps around a PyQt5.QWebview. It's worth taking a look to see what things they had to support themselves: https://github.com/scrapinghub/splash/blob/master/splash/browser_tab.py

It's also interesting to see what access to the browser engine they require, for example access to console.log output.

As I understand the "main" event loop of the project is a Twisted event loop (Python).

@paulrouget
Copy link
Contributor Author

paulrouget commented Jul 12, 2017 •
edited

Thanks.

There are a lot of embedding projects we have been studying. For example:

Our conclusion was to not abstract as much at these do, and stay as close to Servo as possible. A more user-friendly API will be build on top of this low level API.

@paulrouget
Copy link
Contributor Author

Somewhat unrelated to this issue, but I wasn't sure where else to post it

When it comes to the way the embedder and servo communicate, #15645 is a good place to discuss. Also, see #15934.

bors-servo pushed a commit that referenced this issue Aug 12, 2017
Auto merge of #17425 - paulrouget:attach-pipeline-2, r=asajeffrey
76700a3 
cleanup embedder/compositor/constellation/script messages

Fix: #17226 #17200 #17201

This is work in progress. Some tests still fail.
I'd like to get early feedback as it's a pretty large PR.

There is nothing fundamentally new. Basically, I added TopLevelBrowsingContrextId to the relevant messages between the embedder, the compositor and the constellation, and enforced the PipelineId to be attached to each ScriptMsg (see #17201).

I unaliased all the ScriptMsg. It was getting difficult to understand the nature of the message as ScriptMsg was used aliased CompositorMsg sometimes (CompositorMsg is an actually type of message already). I renamed constellation_chan to script_to_constellation_chan, again, for clarification.

This cleanup code is necessary for #15934 and for tabs support.

/cc @asajeffrey can I ask you to look at this? No need for a formal review, I need feedback at this stage.

<!-- 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/17425)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Aug 15, 2017
Auto merge of #17425 - paulrouget:attach-pipeline-2, r=asajeffrey
7455899 
cleanup embedder/compositor/constellation/script messages

Fix: #17226 #17200 #17201

This is work in progress. Some tests still fail.
I'd like to get early feedback as it's a pretty large PR.

There is nothing fundamentally new. Basically, I added TopLevelBrowsingContrextId to the relevant messages between the embedder, the compositor and the constellation, and enforced the PipelineId to be attached to each ScriptMsg (see #17201).

I unaliased all the ScriptMsg. It was getting difficult to understand the nature of the message as ScriptMsg was used aliased CompositorMsg sometimes (CompositorMsg is an actually type of message already). I renamed constellation_chan to script_to_constellation_chan, again, for clarification.

This cleanup code is necessary for #15934 and for tabs support.

/cc @asajeffrey can I ask you to look at this? No need for a formal review, I need feedback at this stage.

<!-- 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/17425)
<!-- Reviewable:end -->
@paulrouget
Copy link
Contributor Author

Fixed by #17425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@paulrouget paulrouget

Labels
A-embedding C-assigned There is someone working on resolving the issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@paulrouget @gterzian