1034001 - [BrowserAPI] Fine-grained visilibity API

Status: RESOLVED WORKSFORME

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

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

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.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This also needs to be able to specify "audio: bool" as well.

Comment 2

[Salvador de la Puente González [:salva]]

Not only |audio: bool| but a more general |streams: { audio: bool, video: bool }|.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

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.

Comment 4

[Kan-Ru Chen [:kanru] (UTC+9)]

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.

Comment 5

[Hema Koka [:hema]]

Agree with Milan (comment 3)

See Also: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1069887
https://bugzilla.mozilla.org/show_bug.cgi?id=1051172
https://bugzilla.mozilla.org/show_bug.cgi?id=995540
https://bugzilla.mozilla.org/show_bug.cgi?id=1006200

Lets prioritize this for 2.2!

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

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.

Comment 7

[Kan-Ru Chen [:kanru] (UTC+9)]

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.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

(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?

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

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.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 11

[Ken Chang[:kenkai]]

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.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(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)

Comment 13

[Kan-Ru Chen [:kanru] (UTC+9)]

(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.

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

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().

Comment 15

[Kan-Ru Chen [:kanru] (UTC+9)]

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