New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 652877 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 659155
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

All ui::Windows should have a layer

Project Member Reported by sky@chromium.org, Oct 4 2016

Issue description

Said layer will be used for drawing/animation.
 
Owner: sadrul@chromium.org
Status: Started (was: Untriaged)
I think we need layers only for the ui::Windows in the window manager. i.e. this is not necessarily something we need in all mus clients. (chrome-browser already has ui::Layers for the ui::Windows by way of having a views::NativeWidgetMus associated with it, but chrome-renderer doesn't need any ui::Layers, for example)
sadrul@: I think we want a general API that gives us a cc::Layer or ui::Layer given a ui::Window. This is important for smooth resize regardless. Furthermore, OOPIF ui::Windows need a layer so that the embedder can slide the OOPIF ui::Window in sync with the embedder's scroll.
Maybe a ui::Window::CreateLayer() which returns a cc::Layer and a ui::Layer::GetForWindow(ui::Window). 

The cc::Layer should also hold callbacks that return the surface ref when the surfaceID changes. ui::Layer::SetShowSurface already kind of does this. Maybe we can simply do that?

We probably also need a WindowObserver that tells the parent when the child window has a new surface ID so that it can update its layer.

We can possibly introduce some syntactic sugar so that clients don't need to deal with surface IDs directly, but get a new cc::Layer or ui::Layer when the child submits a compositorframe with a new surface ID.
I am thinking the surface-id propagation on the client-side would happen through WindowTreeClient + WindowObserver.

I don't think clients need a new layer when there's a new surface id. We should simply tell the layer about the new id (and the new size).

For OOPIF, chrome-renderer of the main page (i.e. the embedder) is the one that will have the layer referencing the surface of the OOPIF, right? Or would that be the chrome-browser?
Yes, the embedder page will have a layer referencing the surface of the OOPIF. 

That is the simplest approach, I agree. I'd like to deal with lifetime management in a slightly less adhoc way. Users of Layers shouldn't need to know about lifetime management. In particular, as long as code holds on to a layer, the surface should continue to be valid until it changes. We do this today with callbacks, which ends up with code duplication. I'd like to hide the callbacks in the client lib or something.
When a ui::Window gets a new surface, presumably it's because something happened (e.g. the size changed). The ui::Window needs to be notified of that event anyway (to update internal state, e.g. to resize+relayout the widget, etc.). Including information about the new surface-id can happen at the same time, and doesn't feel like too much of a burden for the client to deal with.
The part that is a burden is lifetime management. At some point the parent needs to tell mus-ws it doesn't care about the surface anymore. That lifetime is not necessarily tied to the window because we can animate a window as its going away, for example. The lifetime of a surface ID reference should be tied to the lifetime of the layer not the window.

This is handled by callbacks today: https://cs.chromium.org/chromium/src/ui/compositor/layer.cc?q=SetShowSurface&sq=package:chromium&l=579

I would rather avoid having every client implement a callback that essentially does the same thing.

Comment 8 by sky@chromium.org, Oct 6 2016

One tricky aspect with the windowmanager is that for certain windows the windowmanager should get the layer for the underlay.

> We probably also need a WindowObserver that tells the parent when the child window has a new surface ID so that it can update its layer.

The framework should provide this for consumers. Is there a reason we can't implement this directly in the client lib?
Mergedinto: 659155
Status: Duplicate (was: Started)
I think  issue 659155  is going to take care of this.

Sign in to add a comment