on xenial help sign in
Merge pull request #257 from ubports/xenial_-_tlwm-no-wraparound

Remove logic to wrap around from maxWindowId to 1

commitfd0f7e8e09b892db0fe46ebc9cec83cbb6037b87 authorMarius Gripsgard <marius@ubports.com> dateThu 2 Jan 2020, 21:45 (7 years ago) parentefd3bd112e32a5
3 files changed+1−85
plugins/WindowManager/TopLevelWindowModel.cpp+1−42 plugins/WindowManager/TopLevelWindowModel.h−14 tests/qmltests/Stage/tst_QMLTopLevelWindowModel.qml−29
plugins/WindowManager/TopLevelWindowModel.cpp+1−42
@@ -40,7 +40,6 @@ namespace unityapi = unity::shell::application;
4040
4141TopLevelWindowModel::TopLevelWindowModel()
4242 : m_nullWindow(createNullWindow()),
43− m_maxWindowId(std::numeric_limits<int>::max()),
4443 m_surfaceManagerBusy(false)
4544{
4645 connect(m_nullWindow, &Window::focusedChanged, this, [this] {
@@ -351,7 +350,7 @@ void TopLevelWindowModel::onSurfaceDestroyed(unityapi::MirSurfaceInterface *surf
351350
352351Window *TopLevelWindowModel::createWindow(unityapi::MirSurfaceInterface *surface)
353352{
354− int id = generateId();
353+ int id = m_nextId.fetchAndAddAcquire(1);
355354 return createWindowWithId(surface, id);
356355}
357356
@@ -526,32 +525,6 @@ int TopLevelWindowModel::findIndexOf(const unityapi::MirSurfaceInterface *surfac
526525 return -1;
527526}
528527
529−// Finds a unique integer identifier not greater than maxWindowId.
530−int TopLevelWindowModel::generateId()
531−{
532− int candidateId = m_nextId.fetchAndAddAcquire(1);
533−
534− // If the user opens two windows at the exact same time after opening
535− // maxWindowId windows, it is potentially possible that we issue the same
536− // ID for both. This is a risk we can accept.
537− if (candidateId > getMaxWindowId()) {
538− int originalCandidateId = candidateId;
539− candidateId = 1;
540−
541− while (indexForId(candidateId) != -1) {
542− candidateId += 1;
543−
544− if (candidateId == originalCandidateId) {
545− // The user has maxWindowId windows open somehow.
546− qFatal("TopLevelWindowModel: Ran out of window IDs!");
547− }
548− }
549−
550− m_nextId.fetchAndStoreAcquire(candidateId + 1);
551− }
552− return candidateId;
553−}
554−
555528QString TopLevelWindowModel::toString()
556529{
557530 QString str;
@@ -795,20 +768,6 @@ void TopLevelWindowModel::setRootFocus(bool focus)
795768 }
796769}
797770
798−void TopLevelWindowModel::setMaxWindowId(int newMaxWindowId)
799−{
800− DEBUG_MSG << "(" << newMaxWindowId << ")";
801− if (m_maxWindowId != newMaxWindowId) {
802− m_maxWindowId = newMaxWindowId;
803− Q_EMIT maxWindowIdChanged();
804− }
805−}
806−
807−int TopLevelWindowModel::getMaxWindowId()
808−{
809− return m_maxWindowId;
810−}
811−
812771// Pending Activation will block refocus of previous focused window
813772// this is needed since surface activation with miral is async,
814773// and activation of placeholder is sync. This causes a race condition
plugins/WindowManager/TopLevelWindowModel.h−14
@@ -90,14 +90,6 @@ class WINDOWMANAGERQML_EXPORT TopLevelWindowModel : public QAbstractListModel
9090 */
9191 Q_PROPERTY(int nextId READ nextId)
9292
93− /**
94− The maximum window ID that may be issued by the model.
95− Will only leave the model in a consistent state if changed before
96− any windows are added.
97− Don't change this unless you are the tests or want to break everything.
98− */
99− Q_PROPERTY(int maxWindowId READ getMaxWindowId WRITE setMaxWindowId)
100−
10193
10294 /**
10395 * @brief Sets whether a user Window or "nothing" should be focused
@@ -208,9 +200,6 @@ public:
208200 void setRootFocus(bool focus);
209201 bool rootFocus();
210202
211− void setMaxWindowId(int newMaxWindowId);
212− int getMaxWindowId();
213−
214203Q_SIGNALS:
215204 void countChanged();
216205 void inputMethodSurfaceChanged(unity::shell::application::MirSurfaceInterface* inputMethodSurface);
@@ -229,8 +218,6 @@ Q_SIGNALS:
229218
230219 void rootFocusChanged();
231220
232− void maxWindowIdChanged();
233−
234221private Q_SLOTS:
235222 void onSurfaceCreated(unity::shell::application::MirSurfaceInterface *surface);
236223 void onSurfacesRaised(const QVector<unity::shell::application::MirSurfaceInterface*> &surfaces);
@@ -297,7 +284,6 @@ private:
297284 bool m_pendingActivation;
298285
299286 QAtomicInteger<int> m_nextId{1};
300− int m_maxWindowId;
301287
302288 unity::shell::application::ApplicationManagerInterface* m_applicationManager{nullptr};
303289 unity::shell::application::SurfaceManagerInterface *m_surfaceManager{nullptr};
tests/qmltests/Stage/tst_QMLTopLevelWindowModel.qml−29
@@ -168,34 +168,5 @@ Item {
168168 tlwm.rootFocus = true;
169169 compare(dialerApp.focused, true);
170170 }
171−
172− /*
173− Ensure that we can handle issuing windows up to maxWindowId,
174− including reusing IDs once we're exhausted.
175− */
176− function test_issueAllTheIds()
177− {
178− tlwm.maxWindowId = 5;
179−
180− // Start an app ourselves so we can close it
181− var nextAppId = ApplicationManager.availableApplications[5];
182− var app = ApplicationManager.startApplication(nextAppId);
183− var appWindowId = tlwm.idAt(0);
184−
185− // Add four more apps to bring us to five
186− addApps(4);
187−
188− ApplicationManager.stopApplication(app.appId);
189− // It should be gone soon
190− tryCompareFunction(function() { return tlwm.indexForId(appWindowId); }, -1);
191−
192− // And now we can start a new one
193− var appAgain = ApplicationManager.startApplication(nextAppId);
194− verify(appAgain);
195−
196− // The new app should have taken the ID of the closed app since
197− // we're under contention.
198− tryCompareFunction(function() { return tlwm.indexForId(appWindowId); }, 0);
199− }
200171 }
201172}