Changeset View
Standalone View
shell/shellcorona.cpp
Show First 20 Lines • Show All 1899 Lines • ▼ Show 20 Line(s) | 1895 | { | |||
---|---|---|---|---|---|
1900 | 1900 | | |||
1901 | m_activityController->stopActivity(m_activityController->currentActivity()); | 1901 | m_activityController->stopActivity(m_activityController->currentActivity()); | ||
1902 | } | 1902 | } | ||
1903 | 1903 | | |||
1904 | void ShellCorona::insertContainment(const QString &activity, int screenNum, Plasma::Containment *containment) | 1904 | void ShellCorona::insertContainment(const QString &activity, int screenNum, Plasma::Containment *containment) | ||
1905 | { | 1905 | { | ||
1906 | Plasma::Containment *cont = nullptr; | 1906 | Plasma::Containment *cont = nullptr; | ||
1907 | for (Plasma::Containment *c : m_desktopContainments.value(activity)) { | 1907 | for (Plasma::Containment *c : m_desktopContainments.value(activity)) { | ||
1908 | if (c->screen() == screenNum) { | 1908 | //using lastScreen() instead of screen() catches also containments of activities that aren't the current one, so not assigned to a screen right now | ||
1909 | if (c->lastScreen() == screenNum) { | ||||
davidedmundson: > catches also containments of other activities
No it doesn't - you're looping through… | |||||
yes it does. mart: yes it does.
what i mean is:
insertContainment() called with an activity that is not the… | |||||
1909 | cont = c; | 1910 | cont = c; | ||
1910 | if (containment == cont) { | 1911 | if (containment == cont) { | ||
1911 | return; | 1912 | return; | ||
1912 | } | 1913 | } | ||
1913 | break; | 1914 | break; | ||
1914 | } | 1915 | } | ||
1915 | } | 1916 | } | ||
1916 | 1917 | | |||
1917 | Q_ASSERT(!m_desktopContainments.value(activity).values().contains(containment)); | 1918 | Q_ASSERT(!m_desktopContainments.value(activity).values().contains(containment)); | ||
1918 | 1919 | | |||
1919 | if (cont) { | 1920 | if (cont) { | ||
1920 | disconnect(cont, SIGNAL(destroyed(QObject*)), | | |||
1921 | this, SLOT(desktopContainmentDestroyed(QObject*))); | | |||
1922 | cont->destroy(); | 1921 | cont->destroy(); | ||
davidedmundson: Why are you doing this? | |||||
cont is being deleted there, i'm making sure it's not in m_desktopContainments anymore for not even an instant. mart: cont is being deleted there, i'm making sure it's not in m_desktopContainments anymore for not… | |||||
So to make it clear what this code is actually doing: we're disconnecting from a method that automatically does cleanup What is the point of that? davidedmundson: So to make it clear what this code is actually doing:
we're disconnecting from a method that… | |||||
i could just remove that disconnect, the thing is, i'm always hesitant to change things in ways that may have had a reason. mart: i could just remove that disconnect, the thing is, i'm always hesitant to change things in ways… | |||||
apparently if it's managed just by the signal it seems to be too "late", as the containments don't seem to be cleaned in a single startup from the config file and i sometimes get crashes when i change activity mart: apparently if it's managed just by the signal it seems to be too "late", as the containments… | |||||
1923 | } | 1922 | } | ||
1924 | m_desktopContainments[activity].insert(containment); | 1923 | m_desktopContainments[activity].insert(containment); | ||
1925 | 1924 | | |||
1926 | //when a containment gets deleted update our map of containments | 1925 | //when a containment gets deleted update our map of containments | ||
1927 | connect(containment, SIGNAL(destroyed(QObject*)), this, SLOT(desktopContainmentDestroyed(QObject*))); | 1926 | connect(containment, SIGNAL(destroyed(QObject*)), this, SLOT(desktopContainmentDestroyed(QObject*))); | ||
1928 | } | 1927 | } | ||
1929 | 1928 | | |||
1930 | void ShellCorona::desktopContainmentDestroyed(QObject *obj) | 1929 | void ShellCorona::desktopContainmentDestroyed(QObject *obj) | ||
1931 | { | 1930 | { | ||
1932 | // when QObject::destroyed arrives, ~Plasma::Containment has run, | 1931 | // when QObject::destroyed arrives, ~Plasma::Containment has run, | ||
1933 | // members of Containment are not accessible anymore, | 1932 | // members of Containment are not accessible anymore, | ||
1934 | // so keep ugly bookeeping by hand | 1933 | // so keep ugly bookeeping by hand | ||
1935 | auto containment = static_cast<Plasma::Containment*>(obj); | 1934 | auto containment = static_cast<Plasma::Containment*>(obj); | ||
1936 | for (auto a : m_desktopContainments) { | 1935 | //explicitly specify the range by reference, as we need to remove stuff from the sets | ||
1936 | for (QSet<Plasma::Containment *> &a : m_desktopContainments) { | ||||
1937 | QMutableSetIterator<Plasma::Containment *> it(a); | 1937 | QMutableSetIterator<Plasma::Containment *> it(a); | ||
1938 | while (it.hasNext()) { | 1938 | while (it.hasNext()) { | ||
1939 | it.next(); | 1939 | it.next(); | ||
1940 | if (it.value() == containment) { | 1940 | if (it.value() == containment) { | ||
1941 | it.remove(); | 1941 | it.remove(); | ||
1942 | return; | 1942 | return; | ||
1943 | } | 1943 | } | ||
1944 | } | 1944 | } | ||
▲ Show 20 Lines • Show All 150 Lines • Show Last 20 Lines |
No it doesn't - you're looping through m_desktopContainments.value(activity) so only things of the same activity.
Either your comment is nonsense, or your code is, or both.