Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2254733003:
Add ui::LayerObserver and use it to update Alt+Tab previews as needed. (Closed)
|
Created:
4 years, 4 months ago by Evan Stade Modified:
4 years, 3 months ago CC:
chromium-reviews, sadrul, Ian Vollick, sievers+watch_chromium.org, jbauman+watch_chromium.org, piman+watch_chromium.org, kalyank, danakj+watch_chromium.org, cc-bugs_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd ui::LayerObserver and use it to update Alt+Tab previews as needed.
This fixes the Google Maps app part of the bug, and is necessary but
not sufficient to fix the minimized window/swapped-out renderer part of
the bug.
BUG=636594
Committed: https://crrev.com/696aa48c01ff79afbde347312f790fb54406d430
Cr-Commit-Position: refs/heads/master@{#416071}
Patch Set 1 #Patch Set 2 : appease compiler #
Total comments: 2
Patch Set 3 : with other fix too #Patch Set 4 : just this fix #Patch Set 5 : bad at git #Patch Set 6 : layer owner #
Total comments: 6
Patch Set 7 : no children #Patch Set 8 : curlies #
Total comments: 1
Patch Set 9 : ok no dcheck #Patch Set 10 : fix unit tests #Patch Set 11 : fix for other test #
Messages
Total messages: 48 (21 generated)
estade@chromium.org changed reviewers: + sky@chromium.org, varkha@chromium.org
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc File ui/compositor/layer.cc (right): https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc#... ui/compositor/layer.cc:769: for (Layer* ancestor = this; ancestor; ancestor = ancestor->parent()) { I would like to avoid having to walk the tree on every delegatedframedamage. CloneChildren() is iterating over the tree, can it handle the case of installing an observer on every cloned layer? Seems like ForwardingLayerDelegate could easily install an observer on the original layer. I think this would make ForwardingLayerDelegate lookup of the delegate easier too (it wouldn't have to validate). It's not clear to me why OnDelegatedFrameDamage() means you need to recreate all the layers. Could you motivate that better? I'm also wondering if you make ForwardingLayerDelegate the LayerObserver that perhaps you don't need to recreate all, or maybe you can just recreate the subtree?
https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc File ui/compositor/layer.cc (right): https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc#... ui/compositor/layer.cc:769: for (Layer* ancestor = this; ancestor; ancestor = ancestor->parent()) { On 2016/08/18 13:36:21, sky wrote: > I would like to avoid having to walk the tree on every delegatedframedamage. > CloneChildren() is iterating over the tree, can it handle the case of installing > an observer on every cloned layer? Seems like ForwardingLayerDelegate could > easily install an observer on the original layer. I think this would make > ForwardingLayerDelegate lookup of the delegate easier too (it wouldn't have to > validate). This only walks up (as opposed to a full tree traversal); it's equivalent to GetRoot(this). It's true that we could make ForwardingLayerDelegate an observer and it might simplify that class. > > It's not clear to me why OnDelegatedFrameDamage() means you need to recreate all > the layers. Could you motivate that better? I'm using this as a pseudo-equivalent to View::ViewHierarchyChanged, except for layers. // Notifies the layer that one of its children has received a new // delegated frame. void OnDelegatedFrameDamage(const gfx::Rect& damage_rect_in_dip); Admittedly I'm not 100% sure what a delegated frame is because I haven't found any useful documentation ("[...]DelegatedFrameHost, which manages delegated frames[...]"). In fact, walking up the entire callstack (SwapDelegatedFrame, ViewHostMsg_SwapCompositorFrame, etc.) I have found essentially zero documentation anywhere. So this is my best guess at what makes sense after perusing the code. OnDelegatedFrameDamage seems to only be called right after a ui::Layer gets a new SurfaceLayer. At this point it makes sense that we need to recreate the layers. In practice this doesn't seem to happen often (I've only seen it happen soon after a renderer spins up). > > I'm also wondering if you make ForwardingLayerDelegate the LayerObserver that > perhaps you don't need to recreate all, or maybe you can just recreate the > subtree? You mean recreating just the subtree that got the notification? It seems like it would complicate the code more than it's worth since I'm not sure how much it would save. You have noted that it's expensive to recreate layers, but we only do this for on-screen Alt+Tab previews (depending on display metrics, this is probably only about 5 windows) and I haven't noticed any perceptible jankiness. During my spelunking yesterday I did notice cc::Layer::RequestCopyOfOutput which seems like it could be an alternative, except that I don't think it would allow for live updates. I could be wrong.
On 2016/08/18 15:57:31, Evan Stade wrote: > https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc > File ui/compositor/layer.cc (right): > > https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc#... > ui/compositor/layer.cc:769: for (Layer* ancestor = this; ancestor; ancestor = > ancestor->parent()) { > On 2016/08/18 13:36:21, sky wrote: > > I would like to avoid having to walk the tree on every delegatedframedamage. > > CloneChildren() is iterating over the tree, can it handle the case of > installing > > an observer on every cloned layer? Seems like ForwardingLayerDelegate could > > easily install an observer on the original layer. I think this would make > > ForwardingLayerDelegate lookup of the delegate easier too (it wouldn't have to > > validate). > > This only walks up (as opposed to a full tree traversal); it's equivalent to > GetRoot(this). > > It's true that we could make ForwardingLayerDelegate an observer and it might > simplify that class. > So I wrote the patch to do this and it doesn't work. The complexity in ForwardingLayerDelegate and the reason that it takes a delegate rather than a layer seems to be that layers get deleted unexpectedly, but this class hopes the delegate stays the same in that case. (I wan't the original author of this class.) When I try to observe the layer itself, it's often destroyed before the OnDelegatedFrameDamage. I think the InTree variant is necessary so that we can just observe at the root (alternatively, we could only notify observers of the root, but that still requires walking up the parent chain).
ForwardingLayerDelegate is a bit error prone, in so far as it relies on pointers to check validity. This is never a good thing. If you're going to go down the route of adding observers then you should make it so ForwardingLayerDelegate can really stay in sync with the layer, which means notification on events that are needed to keep the trees in sync. -Scott On Thu, Aug 18, 2016 at 10:29 AM, <estade@chromium.org> wrote: > On 2016/08/18 15:57:31, Evan Stade wrote: >> >> https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc >> File ui/compositor/layer.cc (right): >> >> > https://codereview.chromium.org/2254733003/diff/20001/ui/compositor/layer.cc#... >> ui/compositor/layer.cc:769: for (Layer* ancestor = this; ancestor; >> ancestor = >> ancestor->parent()) { >> On 2016/08/18 13:36:21, sky wrote: >> > I would like to avoid having to walk the tree on every >> > delegatedframedamage. >> > CloneChildren() is iterating over the tree, can it handle the case of >> installing >> > an observer on every cloned layer? Seems like ForwardingLayerDelegate >> > could >> > easily install an observer on the original layer. I think this would >> > make >> > ForwardingLayerDelegate lookup of the delegate easier too (it wouldn't >> > have > to >> > validate). >> >> This only walks up (as opposed to a full tree traversal); it's equivalent >> to >> GetRoot(this). >> >> It's true that we could make ForwardingLayerDelegate an observer and it >> might >> simplify that class. >> > > So I wrote the patch to do this and it doesn't work. The complexity in > ForwardingLayerDelegate and the reason that it takes a delegate rather than > a > layer seems to be that layers get deleted unexpectedly, but this class hopes > the > delegate stays the same in that case. (I wan't the original author of this > class.) When I try to observe the layer itself, it's often destroyed before > the > OnDelegatedFrameDamage. I think the InTree variant is necessary so that we > can > just observe at the root (alternatively, we could only notify observers of > the > root, but that still requires walking up the parent chain). > > > https://codereview.chromium.org/2254733003/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
estade@chromium.org changed reviewers: + oshima@chromium.org
ForwardingLayerDelegate does not try to stay in sync with the layer, but with the layer's delegate. I attempted to change it to track they layer and it didn't work because of layers getting swapped out (while retaining the same delegate). ForwardingLayerDelegate predates the Alt+Tab UI and I believe it was written by oshima@. Perhaps he can give a more satisfactory explanation for why it works the way it does?
Oshima created it for dragging across displays, which needs to mirror too. I believe the changes I've outlined are applicable for it as well. -Scott On Thu, Aug 18, 2016 at 12:27 PM, <estade@chromium.org> wrote: > ForwardingLayerDelegate does not try to stay in sync with the layer, but > with > the layer's delegate. I attempted to change it to track they layer and it > didn't > work because of layers getting swapped out (while retaining the same > delegate). > > ForwardingLayerDelegate predates the Alt+Tab UI and I believe it was written > by > oshima@. Perhaps he can give a more satisfactory explanation for why it > works > the way it does? > > https://codereview.chromium.org/2254733003/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/08/18 19:37:09, sky wrote: > Oshima created it for dragging across displays, which needs to mirror > too. I believe the changes I've outlined are applicable for it as > well. > > -Scott Indeed, and I've done my best to explain why these changes don't work (after assuming they would work and subsequently finding differently). I'm hoping oshima@ can weigh in with better insights than I've been able to provide. > > On Thu, Aug 18, 2016 at 12:27 PM, <mailto:estade@chromium.org> wrote: > > ForwardingLayerDelegate does not try to stay in sync with the layer, but > > with > > the layer's delegate. I attempted to change it to track they layer and it > > didn't > > work because of layers getting swapped out (while retaining the same > > delegate). > > > > ForwardingLayerDelegate predates the Alt+Tab UI and I believe it was written > > by > > oshima@. Perhaps he can give a more satisfactory explanation for why it > > works > > the way it does? > > > > https://codereview.chromium.org/2254733003/ > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. >
estade@chromium.org changed reviewers: + piman@chromium.org
+piman, is this a bad or good use of OnDelegatedFrameDamage? Best summary I can provide: we use this RecreateLayers trick to steal/recreate layers from a window to show a preview in Alt+Tab. Sometimes this leads to a blank preview because there are changes to the actual window subsequent to layer recreation. If we re-recreate layers after OnDelegatedFrameDamage, the problem goes away. We are concerned about recreating layers being expensive and OnDelegatedFrameDamage happening too much. Note that this would only trigger the layer creation if the user is Alt+Tabbing (and has gotten to the window in question). So the question might boil down to: will OnDelegatedFrameDamage happen super frequently? In my testing it seemed to happen infrequently (like once, when first painting a web contents).
On 2016/08/18 20:02:41, Evan Stade wrote: > +piman, is this a bad or good use of OnDelegatedFrameDamage? > > Best summary I can provide: we use this RecreateLayers trick to steal/recreate > layers from a window to show a preview in Alt+Tab. Sometimes this leads to a > blank preview because there are changes to the actual window subsequent to layer > recreation. If we re-recreate layers after OnDelegatedFrameDamage, the problem > goes away. We are concerned about recreating layers being expensive and > OnDelegatedFrameDamage happening too much. Note that this would only trigger the > layer creation if the user is Alt+Tabbing (and has gotten to the window in > question). So the question might boil down to: will OnDelegatedFrameDamage > happen super frequently? In my testing it seemed to happen infrequently (like > once, when first painting a web contents). OnDelegatedFrameDamage is really meant for exactly one thing: the video detector on Chrome OS, that disables screen dimming and keyboard lighting. This will be called whenever the renderer sends a new frame, but the entire logic is really best effort only. Recreating layers can be expensive, we certainly don't want to do it every frame. In a way, what it sounds like is that you're duplicating the layer, and you would like to get further updates if the surface id given to the original layer changes. Maybe it would make more sense to observe that (i.e. calls to Layer::SetShowSurface), so that you can simply update the surface id in the duplicate layer. I'm even wondering whether it could make sense to make this a first class operation on ui::Layer - now that we use SurfaceLayers instead of DelegatedRendererLayer, it's a lot simpler to do... so maybe Layer could keep a list of "slave" layers (setup by RecreateLayer), so that when the "master" gets its surface updated, so do all the slaves? It would also avoid having to walk the hierarchy, or call to the delegates to do something special (e.g. DelegatedFrameHost::OnLayerRecreated), or the dance in LayerOwner::RecreateLayer to create a new layer, replace it in the tree and return the old one. It also seems more robust overall. Thoughts?
thanks for the quick response. On 2016/08/18 20:33:53, piman wrote: > OnDelegatedFrameDamage is really meant for exactly one thing: the video detector > on Chrome OS, that disables screen dimming and keyboard lighting. This will be > called whenever the renderer sends a new frame, but the entire logic is really > best effort only. What is the meaning of "frame" in this context? A frame of a video? An iframe? > > In a way, what it sounds like is that you're duplicating the layer, and you > would like to get further updates if the surface id given to the original layer > changes. Maybe it would make more sense to observe that (i.e. calls to > Layer::SetShowSurface), so that you can simply update the surface id in the > duplicate layer. I can try this, it did occur to me but I didn't know if it was advantageous to hooking where we already do. > I'm even wondering whether it could make sense to make this a > first class operation on ui::Layer - now that we use SurfaceLayers instead of > DelegatedRendererLayer, it's a lot simpler to do... so maybe Layer could keep a > list of "slave" layers (setup by RecreateLayer), so that when the "master" gets > its surface updated, so do all the slaves? If I understand it correctly, it seems like this is a less general version of the above suggestion (calling out to a generic observer in SetShowSurface). > It would also avoid having to walk the hierarchy, or call to the delegates to do > something special (e.g. DelegatedFrameHost::OnLayerRecreated), or the dance in > LayerOwner::RecreateLayer to create a new layer, replace it in the tree and > return the old one. It also seems more robust overall. Thoughts? Wouldn't it just change the steps of the dance a little? We'd still need to duplicate the layer tree, whether we took ownership of the new one or the old one.
On Thu, Aug 18, 2016 at 2:12 PM, <estade@chromium.org> wrote: > thanks for the quick response. > > On 2016/08/18 20:33:53, piman wrote: > > OnDelegatedFrameDamage is really meant for exactly one thing: the video > detector > > on Chrome OS, that disables screen dimming and keyboard lighting. This > will be > > called whenever the renderer sends a new frame, but the entire logic is > really > > best effort only. > > What is the meaning of "frame" in this context? A frame of a video? An > iframe? > A renderer frame - the output of the rendering of the page contents, in the form of a CompositorFrame which describes render passes and quads, + associated resources. Damage is the area that has changed since the last frame. > > In a way, what it sounds like is that you're duplicating the layer, and > you > > would like to get further updates if the surface id given to the original > layer > > changes. Maybe it would make more sense to observe that (i.e. calls to > > Layer::SetShowSurface), so that you can simply update the surface id in > the > > duplicate layer. > > I can try this, it did occur to me but I didn't know if it was > advantageous to > hooking where we already do. > > > I'm even wondering whether it could make sense to make this a > > first class operation on ui::Layer - now that we use SurfaceLayers > instead of > > DelegatedRendererLayer, it's a lot simpler to do... so maybe Layer could > keep > a > > list of "slave" layers (setup by RecreateLayer), so that when the > "master" > gets > > its surface updated, so do all the slaves? > > If I understand it correctly, it seems like this is a less general version > of > the above suggestion (calling out to a generic observer in SetShowSurface). > Maybe - but are higher level users really interested in micro-managing these surface ids and updates to them, or are they interested in duplicating a "live" layer? There are other uses of RecreateLayer - e.g. when moving a window from one screen to another one. That logic also currently punts on trying to update the surface id on the recreated layer, though it would benefit the same way your feature does. IOW there's definitely good opportunity for sharing that functionality, and once you do, I'm not sure there's any use left for the generic "observe surface id changes". > It would also avoid having to walk the hierarchy, or call to the > delegates to > do > > something special (e.g. DelegatedFrameHost::OnLayerRecreated), or the > dance in > > LayerOwner::RecreateLayer to create a new layer, replace it in the tree > and > > return the old one. It also seems more robust overall. Thoughts? > > Wouldn't it just change the steps of the dance a little? We'd still need to > duplicate the layer tree, whether we took ownership of the new one or the > old > one. > What happens in LayerOwner::RecreateLayer is: - we create a new layer - we copy some properties of the old layer into the new layer - we remove the old layer from the tree - we attach the new layer to the tree, restoring stacking information - we move all children of the old layer onto the new layer - we move the old layer's delegate onto the new layer - we call the delegate to fixup the new layer - we return the old layer What I'm suggesting is: - we create a new layer - we copy some (more) properties of the old layer into the new layer - we return the new layer If the callers want to do that over the hierarchy, sure, that doesn't change. > > https://codereview.chromium.org/2254733003/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
does the same issue happen when you drag a window to another display on chromeos?
On 2016/08/19 19:47:02, oshima wrote: > does the same issue happen when you drag a window to another display on > chromeos? how would I test? Is there a command line flag to create fake secondary displays?
On 2016/08/21 22:06:09, Evan Stade wrote: > On 2016/08/19 19:47:02, oshima wrote: > > does the same issue happen when you drag a window to another display on > > chromeos? > > how would I test? Is there a command line flag to create fake secondary > displays? You can use --ash-host-window-bounds=1000x600,0+600-800x500 to create two (or more) displays (root windows).
OK everyone, thanks for the feedback. The latest patch incorporates a lot of it. Now: a) ForwardingLayerDelegate observes its source layer b) ForwardingLayerDelegate handles recreating the output layer when the surface is set* c) (New) We also pass along paint calls from the source layer to the output layer. This fixes stale content in non-surface layers. For example, now the tab title/loading indicator will update in the preview. I did not implement Antoine's slave layer idea (although moving the observer notification to SetShowSurface was one of his recommendations), in part because I'm uncomfortable making substantial changes to Layer and in part because there's not a lot of time before branch. *this seemed like a good way to leverage DelegatedFrameHost::OnLayerRecreated to make sure there are two layers with the same surface info. This is the same thing we do when the preview is initially created. Compared to the original patch, it only recreates the subtree that needs recreating. re: does this happen with dragged windows --- I couldn't trigger it, but then again you can't drag minimized windows.
lgtm
https://codereview.chromium.org/2254733003/diff/100001/ash/common/wm/forwardi... File ash/common/wm/forwarding_layer_delegate.cc (right): https://codereview.chromium.org/2254733003/diff/100001/ash/common/wm/forwardi... ash/common/wm/forwarding_layer_delegate.cc:50: std::unique_ptr<ui::Layer> recreated = owner->RecreateLayer(); Don't you need to reset client_layer_ to recreated? When recreating the layer shouldn't you also recreate any children? https://codereview.chromium.org/2254733003/diff/100001/ui/compositor/layer_ob... File ui/compositor/layer_observer.h (right): https://codereview.chromium.org/2254733003/diff/100001/ui/compositor/layer_ob... ui/compositor/layer_observer.h:16: virtual void DidPaintLayer(Layer* layer, const gfx::Rect& region) = 0; Observers generally don't care about all functions. I don't think you should make any of these pure virtual. https://codereview.chromium.org/2254733003/diff/100001/ui/wm/core/window_util.cc File ui/wm/core/window_util.cc (left): https://codereview.chromium.org/2254733003/diff/100001/ui/wm/core/window_util... ui/wm/core/window_util.cc:94: if (factory) { optional: personally I would leave the {} here as the body spans multiple lines. But I realize the style guide is unclear on this.
https://codereview.chromium.org/2254733003/diff/100001/ash/common/wm/forwardi... File ash/common/wm/forwarding_layer_delegate.cc (right): https://codereview.chromium.org/2254733003/diff/100001/ash/common/wm/forwardi... ash/common/wm/forwarding_layer_delegate.cc:50: std::unique_ptr<ui::Layer> recreated = owner->RecreateLayer(); On 2016/08/23 17:15:07, sky wrote: > Don't you need to reset client_layer_ to recreated? yes indeed. > When recreating the layer > shouldn't you also recreate any children? good call. I think this works as is though because layers with surfaces don't generally have children. I added a CHECK and tried to trigger it without success, so instead of adding complexity to handle a case that doesn't seem to come up, I just left a DCHECK. I think the worst case scenario if a surface layer did have children would be a messed up preview (i.e. missing child layers). Relatedly, there's nothing to keep the layer trees in sync, in case the base window is adding or removing layers for example. Until it becomes apparent how this is a practical problem, I'd just as soon not deal with it. https://codereview.chromium.org/2254733003/diff/100001/ui/compositor/layer_ob... File ui/compositor/layer_observer.h (right): https://codereview.chromium.org/2254733003/diff/100001/ui/compositor/layer_ob... ui/compositor/layer_observer.h:16: virtual void DidPaintLayer(Layer* layer, const gfx::Rect& region) = 0; On 2016/08/23 17:15:07, sky wrote: > Observers generally don't care about all functions. I don't think you should > make any of these pure virtual. Done. https://codereview.chromium.org/2254733003/diff/100001/ui/wm/core/window_util.cc File ui/wm/core/window_util.cc (left): https://codereview.chromium.org/2254733003/diff/100001/ui/wm/core/window_util... ui/wm/core/window_util.cc:94: if (factory) { On 2016/08/23 17:15:07, sky wrote: > optional: personally I would leave the {} here as the body spans multiple lines. > But I realize the style guide is unclear on this. oops, this was not intentional
https://codereview.chromium.org/2254733003/diff/140001/ash/common/wm/forwardi... File ash/common/wm/forwarding_layer_delegate.cc (right): https://codereview.chromium.org/2254733003/diff/140001/ash/common/wm/forwardi... ash/common/wm/forwarding_layer_delegate.cc:50: // tend to have children anyway. I'm nervous about this DCHECK. It's seems entirely possible to hit it when dragging browser windows across displays or other complex objects. I'm ok if you want to punt on it, but I think this DHECK will hit, so it shouldn't be a DCHECK.
On 2016/08/30 00:03:13, sky wrote: > https://codereview.chromium.org/2254733003/diff/140001/ash/common/wm/forwardi... > File ash/common/wm/forwarding_layer_delegate.cc (right): > > https://codereview.chromium.org/2254733003/diff/140001/ash/common/wm/forwardi... > ash/common/wm/forwarding_layer_delegate.cc:50: // tend to have children anyway. > I'm nervous about this DCHECK. It's seems entirely possible to hit it when > dragging browser windows across displays or other complex objects. I'm ok if you > want to punt on it, but I think this DHECK will hit, so it shouldn't be a > DCHECK. removed dcheck, updated comments
Thanks, LGTM
The CQ bit was checked by estade@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org Link to the patchset: https://codereview.chromium.org/2254733003/#ps160001 (title: "ok no dcheck")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by estade@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org, piman@chromium.org Link to the patchset: https://codereview.chromium.org/2254733003/#ps200001 (title: "fix for other test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Add ui::LayerObserver and use it to update Alt+Tab previews as needed. This fixes the Google Maps app part of the bug, and is necessary but not sufficient to fix the minimized window/swapped-out renderer part of the bug. BUG=636594 ========== to ========== Add ui::LayerObserver and use it to update Alt+Tab previews as needed. This fixes the Google Maps app part of the bug, and is necessary but not sufficient to fix the minimized window/swapped-out renderer part of the bug. BUG=636594 Committed: https://crrev.com/696aa48c01ff79afbde347312f790fb54406d430 Cr-Commit-Position: refs/heads/master@{#416071} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/696aa48c01ff79afbde347312f790fb54406d430 Cr-Commit-Position: refs/heads/master@{#416071} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
