Closed Bug 1034001 Opened 6 years ago Closed 5 years ago

[BrowserAPI] Fine-grained visilibity API

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED WORKSFORME
tracking-b2g +

People

(Reporter: kanru, Assigned: kanru)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Currently setVisible accepts a boolean indicating a frame's visibility. A lot of other state depends on the visibility, for example we throw away rendering state when the frame is not visible.

Sometimes we want to keep the latest rendering state instead of showing a white page. We want to extend the setVisible to accept a option object

  setVisible(bool) -> setVisible({state: bool, rendering: bool })

So we could control the other aspect of the visibility. The option object will support rendering initially, and maybe priority and other options later.
This also needs to be able to specify "audio: bool" as well.
Not only |audio: bool| but a more general |streams: { audio: bool, video: bool }|.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1054904#c18, for example.  Seems we never have time to do this because we identify it as necessary only once we're in bug fixing.
feature-b2g: --- → 2.2?
While I was working on this last week, I found we also need to think about how this should interact with multi-layer <iframe> because currently the inner <iframe> is considered inactive when the containing <iframe> is inactive.
Assignee: nobody → kchen
Blocks: 1069887
According to offline discussion today, we tend to change the implementation of this bug:
Move screenshoting mechanism from gaia to gecko.

One of the benefits is gecko has more power to do a better and faster implementation of getScreenshot(). We currently do not getScreenshot while the transition is opening and it is also why we don't put the mozbrowser iframe to background via setVisible(false). If we do get screenshot in gaia side we will run into performance issue because it's now very slow.

The second behefit is the simplification of the setVisible API than the initial proposal in comment 0. Gaia  just needs to call setVisible(true|false) as soon as it starts to perform the opening/closing transition.

The third is we could fix all the visibility coupling issues as listed below:
* Underlying app is still playing audio while callscreen is opening.
* App is able to lock the screen orientation during transition/edge wipe gesture.

What we are not able to fix is:
* The rotation of screenshot overlay (now in gecko) for two orientation-perpendicular apps at the same time.
** Possible solution: gecko will provide the screenshot blob to gaia and gaia could rotate and overlay on top of the gecko screenshot anyway if needed.
* Gecko sometimes needs to drop the rendering data anyway (memory pressure) so gaia needs a query/event to know *the cache* of this mozbrowser iframe is already dropped by gecko hence we could do something like to put identification overlay on top of the white mozbrowser iframe.

Kanru will provide more info afterwards. And for sure we need a specific gaia change.
Thanks Alive for the summary! My main concern of the original proposal is that when more features emerges the options to setVisible would pile on.

I think we could view the screenshot as a poster image that got automatically applied when document.hidden is true. We already optimize the animation when document.hidden is true (http://www.w3.org/TR/animation-timing/#processingmodel), why not also replace the displayed data by a static image?

It could solve other problems, like the media ones, if gaia could call setVisible without worrying the page flickering problem.
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #6)
> According to offline discussion today, we tend to change the implementation
> of this bug:
> Move screenshoting mechanism from gaia to gecko.
> 

Bug 744100?
Depends on: 744100
We need to decide quickly if this is 2.2 or not, if we need to move "get screenshot" into Gecko, and fix bug 744100 in the process.  In order for the graphics to prioritize that bug, we need to know soon, before we fill the capacity through Gecko 38 with other things.
I definitely think we should do this for 2.2. But I think we need to get Kan-Ru help since he already has a long list of tasks for the browser API.

Thinker and Ken can hopefully help find people who can help.
Flags: needinfo?(kchang)
After talking with Kan-Ru, he is working on this and wants to finish this bug in 2.2. But there are some discussions undergoing. He will put these info on this bug later.
feature-b2g: 2.2? → 2.2+
Flags: needinfo?(kchang)
Blocks: 1058269
(In reply to Ken Chang[:ken] from comment #11)
> After talking with Kan-Ru, he is working on this and wants to finish this
> bug in 2.2. But there are some discussions undergoing. He will put these
> info on this bug later.

Kanru, do we have the scope of this bug locked down? Let's not commit something into 2.2 without a clear scope and tell people this bug will solve everything.

(comment 0 was not explicit on what will be decoupled from the main visibility state)
Flags: needinfo?(kchen)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #12)
> (In reply to Ken Chang[:ken] from comment #11)
> > After talking with Kan-Ru, he is working on this and wants to finish this
> > bug in 2.2. But there are some discussions undergoing. He will put these
> > info on this bug later.
> 
> Kanru, do we have the scope of this bug locked down? Let's not commit
> something into 2.2 without a clear scope and tell people this bug will solve
> everything.
> 
> (comment 0 was not explicit on what will be decoupled from the main
> visibility state)

I made a new proposal on dev-webapi to clarify the idea in comment 6.
https://groups.google.com/forum/#!topic/mozilla.dev.webapi/7Kmvwax9-KI

If no one objects we will implement that proposal.
Flags: needinfo?(kchen)
Blocks: 1083605
feature-b2g: 2.2+ → ---
tracking-b2g: --- → +
Kanru,

I tried setActive() today and I really doubt it fulfill our original imagination to new setVisible().
In certain use case we want to tell the application it is going to background but want to keep the rendering , but setActive() does not provide this functionality. On the contrary it does not change the visibility state but only affect the priority a little as you mentioned. It smells like "setLowerPriorityToForegroundApp()" to me. I don't think it fix our issues anyway.

The core part of all visibility issues are lots of API logic depends on the visibility state, but it is bind to rendering and we cannot drop it until the frame animates away from the user or another frame animates on top of it. We need to re-consider how to fix this bug instead of utilize setActive().
Blocks: 1116397
I believe this issue had been or could be addressed by other means like don't aggressively clear gfx data, use browserElement.setActive, use dedicated browser-api like setNFCFocus and audioChannel API. Close as worksforme.

To solve the issue in comment 14 I think we should stop purge gfx resource in setVisible. Instead we should implement a LRU system that collect least used gfx resource when needed. See bug 1154231
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.