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

Implement visibility API and the notion of non-visible pipeline #9751

Closed
paulrouget opened this issue Feb 25, 2016 · 12 comments
Closed

Implement visibility API and the notion of non-visible pipeline #9751

paulrouget opened this issue Feb 25, 2016 · 12 comments
Labels
C-assigned There is someone working on resolving the issue

Comments

@paulrouget
Copy link
Contributor

Spec: https://w3c.github.io/page-visibility/
MDN: https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API

When a document is not visible (usually a background tab), the document is in "slow mode". setTimeout and setInterval are called less often, requestAnimationFrame is not called and there's probably a lot more things that can slowed down, or stopped.

We can't just use pipeline.freeze() / pipeline.thaw() as we still need events to occur.

Related issue: #9566

@jmr0
Copy link
Contributor

jmr0 commented Feb 28, 2016

I could probably take a stab at this with a little guidance. Let me know if you have any other thoughts I could use as a starting point or any places in the codebase off the top of your head where you'd expect to see some major changes.

@jdm
Copy link
Member

jdm commented Feb 28, 2016

This has several parts:

  • adding the APIs to the Document interface per the specification (Document.webidl, document.rs)
  • adding the notion of visibility to Pipeline (pipeline.rs)
  • making the Document APIs communicate with the constellation to update the pipeline visibility
  • using the pipeline visibility to inhibit animation callbacks (compositor.rs?)
  • changing the behaviour of timers based on visibility (maybe timers.rs and timer_scheduler.rs?)

@paulrouget
Copy link
Contributor Author

And I'll add:

This is needed to make a pipeline non visible.

@paulrouget
Copy link
Contributor Author

Re starting point:

I built a fake setVisible method that freeze the pipeline: paulrouget@fe29671

This is wrong because it uses thaw/freeze which is too aggressive, but it gives you an idea how to expose an iframe method that would call a pipeline method.

You need to start Servo with --pref dom.mozbrowser.enabled and use a <iframe mozbrowser="true"> to make the setVisible method callable.

Hope that helps.

@jmr0
Copy link
Contributor

jmr0 commented Feb 28, 2016

Thanks guys -- I'll start looking at adapting the pipeline and preparing the Document API changes and circle back with any questions. Feel free to assign.

@KiChjang KiChjang added the C-assigned There is someone working on resolving the issue label Feb 28, 2016
@jdm
Copy link
Member

jdm commented Mar 14, 2016

@jmr0 Any progress? Any questions?

@jmr0
Copy link
Contributor

jmr0 commented Mar 14, 2016

Hey @jdm, I caught up with Paul over on IRC yesterday and decided I'd focus on the iframe part, since it seems we currently don't support the events necessary to hook to the Document API (e.g. OS lock screen, screen minimized). Ongoing work is here https://github.com/jmr0/servo/commits/visibility_api -- hope to have something deliverable over the weekend. Thanks!

@jmr0
Copy link
Contributor

jmr0 commented Mar 21, 2016

Almost there with this one. Ended up implementing set/getVisible (Browser API) and some of the timer/compositor reactions to Pipeline visibility changes. Need to take a closer look at requestAnimationFrame and add some tests but all the other parts are pretty much complete.

https://github.com/servo/servo/compare/master...jmr0:visibility_api?expand=1

@paulrouget
Copy link
Contributor Author

@jmr0 is it possible to set the iframe visibility property in ScriptThread::handle_visible_msg? Then you won't have to do it in iframe::SetVisible.

GetVisibility is usually called after the event mozbrowservisibilitychange is received (which will happen in handle_visible_msg), so we don't really care about the intermediate stage between when SetVisible is called and when the actually visibility changed.

Does that make sense?

Also - the fact that 1) there is an event and 2) getVisibility() is async, doesn't make a lot of sense. It's a consequence of how it was built in Gecko. I think getVisibility() should not be async.

@jmr0
Copy link
Contributor

jmr0 commented Mar 21, 2016

@paulrouget Yeah that makes perfect sense, I had added that hack to SetVisible for now but there's no need to do that if we stick to a sync GetVisible. I'll go ahead and make that change.

bors-servo pushed a commit that referenced this issue Jun 16, 2016
Implement non-visible pipeline and iframe visibility methods

This addresses #9566 and a good part of #9751, specifically:

* Pipeline has a notion of visibility
* IFrame setVisible/getVisible interface with IFrame's pipeline visibility
* IFrame mozbrowservisibilitychange responds to changes in visibility
* Pipeline visibility is used to limit animations (requestAnimationFrame does not tick animations when hidden) and to increase timer intervals (currently set to a minimum of 1 second while hidden)

Absent for now are any changes to the Document API and general implementation of the Page Visibility API, since the more interesting parts require knowledge of whether the user agent is minimized, OS screen locked, etc.

cc @paulrouget @jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10225)
<!-- Reviewable:end -->
@jmr0
Copy link
Contributor

jmr0 commented Jun 16, 2016

@paulrouget @jdm We can probably close this and #9566, and possibly open a new related issue with the caveats and parts that weren't covered in #10225 (as per my PR comment) and include anything else you folks can think of. Happy to file that if you agree!

@jdm
Copy link
Member

jdm commented Jun 16, 2016

That's reasonable. Please do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-assigned There is someone working on resolving the issue
Projects
None yet
Development

No branches or pull requests

4 participants