-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Initial internal support for multiple webviews #31417
Conversation
Co-authored-by: Delan Azabani <[email protected]>
There are some variable and comments still use browser as names. This commit renames them to webview.
@delan I think the PR is ready to review now. Could you review it if you have time? |
WebViewVisibilityChanged notifies script that a webview has become invisible due to the native window or app activity, whereas ShowWebView and HideWebView tell the compositor to add or remove it from webrender (and notifies script of the new visibility, taking native window visibility into account). I think we should keep it for now, but I’m open to changing the model if something else makes more sense, like @mrobinson did in #30842 (9e307cf). |
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.
Thanks for rebasing this, it’s coming along well. I’ve edited in a description of the changes in this patch, and updated the description in #30648.
I guess we either need to finish and enable the multiview feature in this patch, or fix the viewport coordinates when multiview is disabled, but given how old this patch is getting I think we should do the latter.
Replace visible_webviews and native_window_is_visible with shown_webviews and invisible_webviews. Add 4 more methods to set them accordingly. The behavior of is_effectively_visible will return true if the wbview is in shown_webviews set but not in invisible_webviews.
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.
Great updates, this is almost ready!
🔨 Triggering try run (#8198742693) for Linux WPT |
🔨 Triggering try run (#8533457028) for Linux WPT |
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
🔨 Triggering try run (#8533487478) for Linux WPT |
Test results for linux-wpt-layout-2020 from try job (#8533457028): Flaky unexpected result (23)
Stable unexpected results that are known to be intermittent (12)
|
✨ Try run (#8533457028) succeeded. |
Test results for linux-wpt-layout-2020 from try job (#8533487478): Flaky unexpected result (16)
Stable unexpected results that are known to be intermittent (17)
|
✨ Try run (#8533487478) succeeded. |
Reproduced in more detail:
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
fail https://github.com/wusyong/servo/actions/runs/8534437489/job/23378836637
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
@delan Thanks for addressing the rest of the issues in the PR! I was absent last week, but I'll catch up in the next few days. |
This patch improves support for multiple top-level browsing contexts aka “webviews” by adding several messages that allow the embedder to show, hide, move, and resize webviews in the compositor, and notify the embedder when painting order and focus changes.
This is a rebased version of #30648, while addressing review feedback given in #30767, #30840, #30841, and #30842. Notably, “browsers” have been renamed to “webviews”.
There are no user-facing changes in servoshell yet, but these changes can be used to create a multiple-document interface (#30785) or implement tabbed browsing (#31545). For the full design of this feature, see #30648.
limitations
compositor
servoshell
messages
embedder → compositor (no ipc)
embedder →* constellation
constellation → compositor
embedder ← constellation ←* compositor
* all of these are in the same enum ConstellationMsg (components/shared/compositing/constellation_msg.rs), which is a very confusing design that conflates the embedder with the compositor
./mach build -d
does not report any errors./mach test-tidy
does not report any errors