From 8d4414b58f7e24db7480ffe896324c235bd5ebd7 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen <richard.gustavsen@digia.com> Date: Fri, 11 Jan 2013 11:27:33 +0100 Subject: [PATCH] PageStack: add recursion guard We need to protect the page stack while it updates properties to ensure that the update is atomic. Otherwise the user can e.g push a new page upon receiving Page.onStatusChanged or Page.onCompleted, which will recursively start a new transition while we prepare for another. Calling push/pop while the transition runs is fine. Change-Id: Ib9453b1a61f8c8cee19f5e748bb75a1a30614119 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@digia.com> --- src/qtdesktop/PageStack.qml | 42 ++++++++++++++++++++++++++++++------- tests/manual/PageStack.qml | 7 ++++++- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/qtdesktop/PageStack.qml b/src/qtdesktop/PageStack.qml index 5b41f145c..7eade6dbb 100644 --- a/src/qtdesktop/PageStack.qml +++ b/src/qtdesktop/PageStack.qml @@ -547,6 +547,8 @@ Item { // Note: we support two different APIs in this function; The old meego API, and // the new "property list" API. Hence the reason for hiding the fact that you // can pass more arguments than shown in the signature: + if (__recursionGuard(true)) + return var properties = arguments[1] var immediate = arguments[2] var replace = arguments[3] @@ -584,6 +586,7 @@ Item { push: true } __performPageTransition(transition) + __recursionGuard(false) return __currentPage } @@ -625,6 +628,9 @@ Item { if (page === __currentPage) return + if (__recursionGuard(true)) + return + var outElement = JSArray.pop() var transitionElement = outElement var inElement = JSArray.current() @@ -654,12 +660,16 @@ Item { push: false } __performPageTransition(transition) + __recursionGuard(false) return outElement.page; } /*! Remove all pages from the stack. No animations will be applied. */ function clear() { - completeTransition() + if (__recursionGuard(true)) + return + if (__currentTransition) + __currentTransition.animation.complete() __currentPage = null var count = __depth for (var i=0; i<count; ++i) { @@ -667,6 +677,7 @@ Item { if (element.page) __cleanup(element); } + __recursionGuard(false) } /*! Search for a specific page inside the stack. \a func will @@ -712,8 +723,11 @@ Item { */ function completeTransition() { + if (__recursionGuard(true)) + return if (__currentTransition) __currentTransition.animation.complete() + __recursionGuard(false) } /********* DEPRECATED API *********/ @@ -735,6 +749,8 @@ Item { property int __depth: 0 /*! \internal Stores the transition info while a transition is ongoing */ property var __currentTransition: null + /*! \internal Stops the user from pushing pages while preparing a transition */ + property bool __guard: false /*! \internal */ Component.onCompleted: { @@ -744,10 +760,21 @@ Item { /*! \internal */ Component.onDestruction: { - completeTransition() + if (__currentTransition) + __currentTransition.animation.complete() __currentPage = null } + function __recursionGuard(use) + { + if (use && __guard) { + console.warn("Warning: PageStack: You cannot push/pop recursively!") + console.trace() + return true + } + __guard = use + } + /*! \internal */ function __loadElement(element) { @@ -858,12 +885,13 @@ Item { /*! \internal */ function __performPageTransition(transition) { - // Animate page in "outElement" out, and page in "inElement" in. Ensure entering page is - // loaded. Loading a page can result in the loaded page recursively pushing or popping - // other pages, so we need to check for this. - completeTransition() + // Animate page in "outElement" out, and page in "inElement" in. Set a guard to protect + // the user from pushing new pages on signals that will fire while preparing for the transition + // (e.g Page.onCompleted, Page.onStatusChanged, Page.onIndexChanged etc). Otherwise, we will enter + // this function several times, which causes the pages to be half-way updated. + if (__currentTransition) + __currentTransition.animation.complete() __loadElement(transition.inElement) - var currentElement = JSArray.current() transition.name = transition.replace ? "replaceAnimation" : (transition.push ? "pushAnimation" : "popAnimation") var enterPage = transition.inElement.page diff --git a/tests/manual/PageStack.qml b/tests/manual/PageStack.qml index 2ff8492c5..b8492a837 100644 --- a/tests/manual/PageStack.qml +++ b/tests/manual/PageStack.qml @@ -135,7 +135,8 @@ Window { Page { id: page Component.onDestruction: console.log("destroyed component page: " + index) - + property bool pushFromOnCompleted: false + Component.onCompleted: if (pushFromOnCompleted) pageStack.push(pageComponent) //pageTransition: rotateTransition Rectangle { @@ -178,6 +179,10 @@ Window { text: "Push component page with destroyOnPop == false" onClicked: pageStack.push({page:pageComponent, destroyOnPop:false}) } + Button { + text: "Push from Page.onCompleted" + onClicked: pageStack.push({page:pageComponent, properties:{pushFromOnCompleted:true}}) + } Button { text: "Pop" onClicked: pageStack.pop() -- GitLab