GNOME Bugzilla – Bug 697300
screenShield: Ensure we destroy background container widgets
Last modified: 2013-04-05 22:24:18 UTC
We may accidentally leak a widget when monitors change while locked otherwise. This is especially bad because we put a box-shadow on this widget.
Created attachment 240673 [details] [review] screenShield: Ensure we destroy background container widgets
Review of attachment 240673 [details] [review]: I don't think this is really right. The only backgrounds in the group should be the ones managed by the bg manager, and it should clean up after itself. Looking I do see a problem in the bg manager code, but I'm not immediately sure how we get from there to here. I'll post a patch for that problem.
Created attachment 240780 [details] [review] background: don't leak background objects during quick changes We currently let some backgrounds "fall through the cracks" if a bunch of change notifications come in at once. This commit fixes that.
(In reply to comment #2) > Review of attachment 240673 [details] [review]: > > I don't think this is really right. The only backgrounds in the group should > be the ones managed by the bg manager, and it should clean up after itself. There are no backgrounds in the group, only container widgets that the ScreenShield itself creates in _createBackground that the BgManager uses as a container. I don't think the BgManager should destroy the container.
(In reply to comment #4) > There are no backgrounds in the group, only container widgets that the > ScreenShield itself creates in _createBackground that the BgManager uses as a > container. Ah right, so to be clear we don't have: new Background.BackgroundManager({ container: this._backgroundGroup, ...}); but instead new Background.BackgroundManager({ container: widget, ...}); this._backgroundGroup.add_child(widget); > I don't think the BgManager should destroy the container. yup, right.
Review of attachment 240673 [details] [review]: +
Comment on attachment 240780 [details] [review] background: don't leak background objects during quick changes This is unrelated, so i'll post it to a different bug
Attachment 240673 [details] pushed as 1ee88a2 - screenShield: Ensure we destroy background container widgets