Merge lp:~ken-vandine/ubuntu-system-settings/lp1365646 into lp:ubuntu-system-settings

Proposed by Ken VanDine on 2014-09-10
Status: Merged
Approved by: Sebastien Bacher on 2014-09-10
Approved revision: 1005
Merged at revision: 1020
Proposed branch: lp:~ken-vandine/ubuntu-system-settings/lp1365646
Merge into: lp:ubuntu-system-settings
Diff against target: 26 lines (+3/-2)
2 files modified
plugins/system-update/PageComponent.qml (+1/-1)
plugins/system-update/system_update.cpp (+2/-1)
To merge this branch: bzr merge lp:~ken-vandine/ubuntu-system-settings/lp1365646
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing on 2014-09-10
Sebastien Bacher (community) 2014-09-10 Approve on 2014-09-10
Review via email:

Commit message

Don't call downloadUpdate for system updates that are already downloading (LP: #1365646)

Description of the change

Don't call downloadUpdate for system updates that are already downloading (LP: #1365646)

To post a comment you must log in.
1005. By Ken VanDine on 2014-09-10

fixed the height calculation for the ListView, it was causing a crash onModelChanged

Sebastien Bacher (seb128) wrote :

Thanks, could you explain why do you need to check the download mode there and what's the difference in the height computation/why the old value was leading to a segfault?

review: Needs Information
Ken VanDine (ken-vandine) wrote :

We don't want to tell system-image-dbus to download the system update if auto_download isn't enabled.

I'm not certain why using contentItem to get the height was causing the crash, it seemed to only happen when the system update was already downloaded, so got added to the model while the click updates were getting added. Note the system update gets prepended to the model, so maybe it has something to do with the reordering of the rows.

Sebastien Bacher (seb128) wrote :

thanks for the details, seems reasonable changes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/system-update/PageComponent.qml'
2--- plugins/system-update/PageComponent.qml 2014-09-09 16:39:35 +0000
3+++ plugins/system-update/PageComponent.qml 2014-09-10 19:35:29 +0000
4@@ -283,7 +283,7 @@
5 right: parent.right
6 }
7 model: updateManager.model
8- height: contentItem.childrenRect.height
9+ height: childrenRect.height
10 interactive: false
12 delegate: ListItem.Subtitled {
14=== modified file 'plugins/system-update/system_update.cpp'
15--- plugins/system-update/system_update.cpp 2014-08-27 22:00:30 +0000
16+++ plugins/system-update/system_update.cpp 2014-09-10 19:35:29 +0000
17@@ -206,7 +206,8 @@
18 } else {
19 Q_EMIT updateNotFound();
20 }
21- if (downloading) {
23+ if (!downloading && (m_downloadMode == 0)) {
24 downloadUpdate();
25 update->setSelected(true);
26 }


People subscribed via source and target branches