D12919: [Plotter] Don't render if m_node is null

2018-05-17 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R296:9d7c4927e506: [Plotter] Dont render if m_node is 
null (authored by broulik).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12919?vs=34261=34356

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrolsaddons/plotter.cpp

To: broulik, #plasma, bshah, davidedmundson
Cc: davidedmundson, bshah, kde-frameworks-devel, michaelh, ngraham, bruns


D12919: [Plotter] Don't render if m_node is null

2018-05-17 Thread David Edmundson
davidedmundson accepted this revision.
davidedmundson added a comment.


  I think it makes sense in the following situation:
  
  a remove window event is queued 
  a render event is queued
  
  we'll have set node to null, and called disconnect, but we'll still be 
processing pending events.
  
  Ship it

REPOSITORY
  R296 KDeclarative

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

To: broulik, #plasma, bshah, davidedmundson
Cc: davidedmundson, bshah, kde-frameworks-devel, michaelh, ngraham, bruns


D12919: [Plotter] Don't render if m_node is null

2018-05-16 Thread Kai Uwe Broulik
broulik added a comment.


  > Do you know if that bug reporter had 
2617a2ab9d472a78559f11ee8d430d951e3335f2 
 
applied?
  
  Given it said Frameworks 5.46 I would assume so

REPOSITORY
  R296 KDeclarative

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

To: broulik, #plasma, bshah
Cc: davidedmundson, bshah, kde-frameworks-devel, michaelh, ngraham, bruns


D12919: [Plotter] Don't render if m_node is null

2018-05-16 Thread David Edmundson
davidedmundson added a comment.


  Have you seen 2617a2ab9d472a78559f11ee8d430d951e3335f2 

  
  The point we set m_node to null we disconnect the connection that calls 
render.
  
  Do you know if that bug reporter had 2617a2ab9d472a78559f11ee8d430d951e3335f2 
 
applied?
  
  but given we check m_window, checking m_node does makes sense for the same 
reasons

REPOSITORY
  R296 KDeclarative

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

To: broulik, #plasma, bshah
Cc: davidedmundson, bshah, kde-frameworks-devel, michaelh, ngraham, bruns


D12919: [Plotter] Don't render if m_node is null

2018-05-16 Thread Bhushan Shah
bshah accepted this revision.
bshah added a comment.
This revision is now accepted and ready to land.


  Sounds good to me.

REPOSITORY
  R296 KDeclarative

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

To: broulik, #plasma, bshah
Cc: bshah, kde-frameworks-devel, michaelh, ngraham, bruns


D12919: [Plotter] Don't render if m_node is null

2018-05-16 Thread Kai Uwe Broulik
broulik created this revision.
broulik added a reviewer: Plasma.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  `m_node` is deleted when window changes, we would access it inconditionally 
in `render`
  
  BUG: 394283

TEST PLAN
  Compiles.

REPOSITORY
  R296 KDeclarative

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrolsaddons/plotter.cpp

To: broulik, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns