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.

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

Rework “visible” to “throttled” in constellation + script + compositor #31816

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

delan
Copy link
Member

@delan delan commented Mar 21, 2024

Servo has a pervasive but confusing concept of webviews or pipelines being “visible”, which actually controls whether script runs timers at a heavily limited rate and the compositor pauses animations.

Continuing from #31815, let’s replace this with a more concrete concept of a webview or pipeline being “throttled” (with the meanings of true and false inverted).

constellation → script

  • SetThrottled (was ChangeFrameVisibilityStatus) has been inverted and its concrete effects documented
  • SetThrottledInContainingIframe (was NotifyVisibilityChange) has been inverted and its purpose documented

constellation ← script

  • SetThrottledComplete (was VisibilityChangeComplete) has been inverted

constellation → compositor

  • SetThrottled (was PipelineVisibilityChanged) has been inverted and its concrete effects documented

Other changes in constellation

  • Constellation.new_pipeline() now takes an inverted “throttled” argument
  • Constellation.new_browsing_context() now takes an inverted “throttled” argument
  • BrowsingContext.throttled (was .is_visible) has been inverted and its concrete effects documented
  • InitialPipelineState.prev_throttled (was .prev_visibility) has been inverted and its concrete effects documented
  • NewBrowsingContextInfo.throttled (was .is_visible) has been inverted and its concrete effects documented

Other changes in script

  • InProgressLoad.throttled (was .is_visible) has been inverted and its concrete effects documented
  • Window.set_throttled() (was .alter_resource_utilization()) has been inverted
  • Window.throttled (was .visible) has been inverted
  • HTMLIFrameElement.throttled (was .visibility) has been inverted

Other changes in compositor

  • PipelineDetails.throttled (was .visible) has been inverted and its concrete effects documented

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because there are no functional changes

@delan delan requested a review from mrobinson March 21, 2024 18:42
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. This rename make it a lot more obvious what is going on.

Base automatically changed from was-visible-now-throttled to main March 21, 2024 20:21
@delan delan force-pushed the was-visible-now-throttled-2 branch from ab0676a to a66425b Compare March 22, 2024 03:15
@delan delan enabled auto-merge March 22, 2024 03:15
@delan delan added this pull request to the merge queue Mar 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 22, 2024
@delan delan added this pull request to the merge queue Mar 22, 2024
Merged via the queue into main with commit 8882507 Mar 22, 2024
9 checks passed
@delan delan deleted the was-visible-now-throttled-2 branch March 22, 2024 07:07
delan added a commit to wusyong/servo that referenced this pull request Mar 25, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2024
* Add multiple concurrent top-level browsing contexts

Co-authored-by: Delan Azabani <dazabani@igalia.com>

* Rename variables and comments

There are some variable and comments still use browser as names.
This commit renames them to webview.

* Update log message from web view to webview

* Revert offscreen_framebuffer_id rename

* Rename all web view to webview

* Cargo fmt

* Fix viewport/event/clear coordinates when multiview is disabled

* Only deprecate things when multiview is enabled

* Update WebViewManger with shown and invisible sets

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.

* Update variant behaviors

* Rename WebViewVisibilityChanged to MarkWebViewInvisible

* Fix unit test by marking id 3 visible again

* Update MarkWebViewInvisible and add UnmarkWebViewInvisible

* Update format and doc comments

* Clean up doc comments

* Address style and naming changes

* Rename UpdateWebView to UpdateFrameTreeForWebView

* constellation: send frame tree unconditionally over focus and feature

* Clarify shown and invisible sets in constellation WebViewManager

* Eliminate forward_to_constellation!()

* Actually remove the unused macro

* Don’t gate compositor changes on multiview feature flag

* Update todo in mouse event dispatch

* Pass all visible webview ids in a single ReadyToPresent message

* Fix compile and lint errors

* servoshell: fix gap between minibrowser toolbar and webview

* Fix failure in /_mozilla/mozilla/window_resizeTo.html

* Fix compile warnings

* Remove stray dbg!()

* Remove confusing “effectively visible” logic (see #31815, #31816)

* Allow embedder to show/hide/raise webviews without ipc

* Update root pipeline only when painting order actually changes

* Stop gating old focus and SetFrameTree behaviour behind Cargo feature

* Use webview_id and WebViewId in webview-related code

* Improve logging of webview-related embedder events

* Allow webview Show and Raise events to optionally hide all others

* Don’t do anything in response to WebViewPaintingOrder

* Remove WebViewPaintingOrder, since its payload is unreliable

* On MoveResizeWebView, only update root pipeline if rect changed

* Rename IOCompositor methods for clarity

* compositor: add event tracing; log webview ops even without ipc

* Add temporary debug logging

* Add more temporary debug logging

* Remove temporary logging in compositor

* Remove temporary debug logging

* Add temporary debug logging, but defer I/O until panic

* Capture a backtrace with each crash log entry

* Proper error handling without panicking in WebViewManager

* Clean up imports in constellation

---------

Co-authored-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants