Merge lp:~stolowski/unity-scopes-shell/fix-1484299-vivid into lp:unity-scopes-shell/trunk-15.04

[Paweł Stołowski]

Fix crash when aggregator's settings view is opened shortly after a new child scope was installed. Fix invalidating of results in temp scopes.

Fix crash when aggregator's settings view is opened shortly after a new child scope was installed. There is a short window (5 seconds) where child scopes list can contain a just installed scope but scopes metadata hasn't been refreshed yet and doesn't know that scope id. If that situation is detected, mark settings as needing refreshing, and re-populate settings model after we have all the new metadata from the registry.

Also fixed invalidating of results in temp scopes (lines 71-74).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:254
http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-ci/3/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-armhf-ci/3/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-i386-ci/3

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-shell-trunk-15.04-ci/3/rebuild

review: Approve (continuous-integration)

[Marcus Tomlinson (marcustomlinson)]

Did you intend to leave all those qDebug()'s in the code?

review: Needs Information

[lp:~stolowski/unity-scopes-shell/fix-1484299-vivid]

255. By Paweł Stołowski on 2015-08-20

Removed debug messages

[Marcus Tomlinson (marcustomlinson)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:255
http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-ci/4/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-armhf-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-armhf-ci/4/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-trunk-15.04-vivid-i386-ci/4

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-shell-trunk-15.04-ci/4/rebuild

review: Approve (continuous-integration)

=== modified file 'src/Unity/scope.cpp'
--- src/Unity/scope.cpp	2015-07-08 09:29:51 +0000
+++ src/Unity/scope.cpp	2015-08-20 08:17:43 +0000
@@ -231,6 +231,11 @@
 
 void Scope::metadataRefreshed()
 {
+    // refresh Settings view if needed
+    if (require_child_scopes_refresh()) {
+        update_child_scopes();
+    }
+
     std::shared_ptr<scopes::ActivationResponse> response;
     response.swap(m_delayedActivation);
 
@@ -377,7 +382,7 @@
         bool containsDepartments = m_rootDepartment.get() != nullptr;
         // design decision - no navigation when doing searches
         containsDepartments &= m_searchQuery.isEmpty();
-        
+
         if (containsDepartments != m_hasNavigation) {
             m_hasNavigation = containsDepartments;
             Q_EMIT hasNavigationChanged();
@@ -862,6 +867,23 @@
     return m_settingsModel.data();
 }
 
+bool Scope::require_child_scopes_refresh() const
+{
+    if (m_settingsModel && m_scopesInstance)
+    {
+        return m_settingsModel->require_child_scopes_refresh();
+    }
+    return false;
+}
+
+void Scope::update_child_scopes()
+{
+    if (m_settingsModel && m_scopesInstance)
+    {
+        m_settingsModel->update_child_scopes(m_scopesInstance->getAllMetadata());
+    }
+}
+
 /*
 Filters* Scope::filters() const
 {

=== modified file 'src/Unity/scope.h'
--- src/Unity/scope.h	2015-07-08 08:25:28 +0000
+++ src/Unity/scope.h	2015-08-20 08:17:43 +0000
@@ -128,6 +128,8 @@
     unity::shell::scopes::ScopeInterface::Status status() const override;
     unity::shell::scopes::CategoriesInterface* categories() const override;
     unity::shell::scopes::SettingsModelInterface* settings() const override;
+    bool require_child_scopes_refresh() const;
+    void update_child_scopes();
     QString searchQuery() const override;
     QString noResultsHint() const override;
     QString formFactor() const override;

=== modified file 'src/Unity/scopes.cpp'
--- src/Unity/scopes.cpp	2015-07-06 17:14:14 +0000
+++ src/Unity/scopes.cpp	2015-08-20 08:17:43 +0000
@@ -577,6 +577,10 @@
     Q_FOREACH(Scope::Ptr scope, m_scopes) {
         scope->invalidateResults();
     }
+
+    Q_FOREACH(Scope::Ptr scope, m_tempScopes) {
+        scope->invalidateResults();
+    }
 }
 
 QVariant Scopes::data(const QModelIndex& index, int role) const

=== modified file 'src/Unity/settingsmodel.cpp'
--- src/Unity/settingsmodel.cpp	2015-06-11 16:10:09 +0000
+++ src/Unity/settingsmodel.cpp	2015-08-20 08:17:43 +0000
@@ -32,7 +32,8 @@
 SettingsModel::SettingsModel(const QDir& configDir, const QString& scopeId,
         const QVariant& settingsDefinitions, QObject* parent,
         int settingsTimeout)
-        : SettingsModelInterface(parent), m_scopeId(scopeId), m_settingsTimeout(settingsTimeout)
+        : SettingsModelInterface(parent), m_scopeId(scopeId), m_settingsTimeout(settingsTimeout),
+          m_requireChildScopesRefresh(false)
 {
     configDir.mkpath(scopeId);
     QDir databaseDir = configDir.filePath(scopeId);
@@ -207,6 +208,16 @@
         return;
     }
 
+    const bool reset = m_requireChildScopesRefresh;
+    if (reset) {
+        // Reset the settings model to fix LP: #1484299, where a new child scope just finished installing
+        // while settings view is created (and we crash); since this is really a corner case, just
+        // resetting the model is fine.
+        beginResetModel();
+    }
+
+    m_requireChildScopesRefresh = false;
+
     m_child_scopes_data.clear();
     m_child_scopes_data_by_id.clear();
     m_child_scopes_timers.clear();
@@ -214,6 +225,13 @@
     for (sc::ChildScope const& child_scope : m_child_scopes)
     {
         QString id = child_scope.id.c_str();
+        if (!scopes_metadata.contains(id)) {
+            // if a child scope was just added to the registry, then scopes_metadata may not contain it yet because of the
+            // scope registry refresh delay on scope add/removal (see LP: #1484299).
+            qWarning() << "SettingsModel::update_child_scopes(): no scope with id '" + id + "'";
+            m_requireChildScopesRefresh = true;
+            continue;
+        }
         QString displayName = QString::fromStdString(_("Display results from %1")).arg(QString(scopes_metadata[id]->display_name().c_str()));
 
         QSharedPointer<QTimer> timer(new QTimer());
@@ -231,6 +249,11 @@
         m_child_scopes_data << setting;
         m_child_scopes_data_by_id[id] = setting;
     }
+
+    if (reset) {
+        endResetModel();
+        Q_EMIT countChanged();
+    }
 }
 
 bool SettingsModel::setData(const QModelIndex &index, const QVariant &value,
@@ -290,6 +313,11 @@
     return m_data.size() + m_child_scopes_data.size();
 }
 
+bool SettingsModel::require_child_scopes_refresh() const
+{
+    return m_requireChildScopesRefresh;
+}
+
 void SettingsModel::settings_timeout()
 {
     QObject *timer = sender();

=== modified file 'src/Unity/settingsmodel.h'
--- src/Unity/settingsmodel.h	2015-01-22 08:21:33 +0000
+++ src/Unity/settingsmodel.h	2015-08-20 08:17:43 +0000
@@ -81,6 +81,8 @@
 
     void update_child_scopes(QMap<QString, unity::scopes::ScopeMetadata::SPtr> const& scopes_metadata);
 
+    bool require_child_scopes_refresh() const;
+
 Q_SIGNALS:
     void settingsChanged();
 
@@ -101,6 +103,7 @@
     QMap<QString, QSharedPointer<Data>> m_child_scopes_data_by_id;
     unity::scopes::ChildScopeList m_child_scopes;
     QMap<QString, QSharedPointer<QTimer>> m_child_scopes_timers;
+    bool m_requireChildScopesRefresh;
 };
 
 }