[Differential] [Request, 75 lines] D2742: Simplify Panel lastSpacer + fix crash

[davidedmundson (David Edmundson)]

davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  The panel contains an invisible Item lastSpacer, who will fill the width
  of the layout if no other applet has Layout.fillWidth/Height set,
  keeping all applets left aligned.
  
  Previous this worked by reparenting the invisible item in/out of the
  layout to the root item. This seems overkill when we can just toggle
  visibility. I think the code existed from before the panel was managed
  by a Layout.
  
  Worse, the current code leads to a potential crash on teardown.
  The root gets deleted, starting a foreach(child, children)
  {child->delete}
  This kills the task bar, which means we reparent the spacer to the
  layout. The layout then finishes killing the children and kills the
  spacer. We then get back to the root objects dtor which is now deleting
  a previously deleted object. See bug report.
  
  This patch also fixes the fact that addApplet/removeApplet had a
  slightly different check to see if the spacer is needed or not, fixing a
  hypothetical bug if you had a Hidden applet with fillWidth: true.
  
  BUG: 357317

TEST PLAN
  In both horizontal and vertical:
  
  - have a panel without a task manager
  - add a task manager
  - remove a task manager

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D2742

AFFECTED FILES
  containments/panel/contents/code/LayoutManager.js
  containments/panel/contents/ui/main.qml

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: davidedmundson, #plasma
Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160911/186091f8/attachment.html>