[Fix] This patch resolves removing incorrect Activity from stack. 93/153993/9
authorPawel Kurowski <p.kurowski2@samsung.com>
Wed, 4 Oct 2017 16:10:13 +0000 (18:10 +0200)
committerPawel Kurowski <p.kurowski2@samsung.com>
Tue, 17 Oct 2017 13:26:12 +0000 (15:26 +0200)
void process() method in Activity.
void markAsCompleted() method in Activity.

Change-Id: I1de7f9a266b32919add25607e3a0ee55b5909d5c

23 files changed:
src/Activity.cpp
src/Activity.hpp
src/AppControlActivity.cpp
src/CallActivity.cpp
src/ChangeScanningMethodActivity.cpp
src/ChangeSoundProfileActivity.cpp
src/ChangeSpeedAutoScanActivity.cpp
src/ChangeValueActivity.cpp
src/GestureActivity.cpp
src/HardwareKeyActivity.cpp
src/MoveElementActivity.cpp
src/PowerKeyMenuActivity.cpp
src/QuickpanelActivity.cpp
src/ScreenshotActivity.cpp
src/SelectActivity.cpp
src/SwitchManager.cpp
src/TapActivity.cpp
src/ToggleAutoScanActivity.cpp
src/ToggleSoundFeedbackEnabledActivity.cpp
src/ToggleVoiceFeedbackEnabledActivity.cpp
src/VolumeControlActivity.cpp
tests/ui-scenarios/ActivityProcessingTests.cpp
tests/ui-scenarios/ConfigurationTests.hpp

index 5d62ef2..d8e585a 100644 (file)
@@ -4,12 +4,17 @@ Activity::Activity(const std::string &activityType)
        : completed(false), type(activityType)
 {}
 
-std::string Activity::getType()
+std::string Activity::getType() const
 {
        return type;
 }
 
-bool Activity::isCompleted()
+bool Activity::isCompleted() const
 {
        return completed;
 }
+
+void Activity::markAsCompleted()
+{
+       completed = true;
+}
index 812e853..b45ad8c 100644 (file)
@@ -12,14 +12,17 @@ public:
        Activity(const std::string &activityType);
        virtual ~Activity() = default;
 
-       std::string getType();
+       std::string getType() const;
 
        virtual void initialize()
        {}
 
-       virtual bool process() = 0;
+       virtual void process() = 0;
+
+       bool isCompleted() const;
+       void markAsCompleted();
+
 
-       virtual bool isCompleted();
 
 protected:
        bool completed;
index ed3253a..2f7e9da 100644 (file)
@@ -12,7 +12,8 @@
 #define RETURN_ON_ERROR(ret) do {                                              \
                if (ret != APP_CONTROL_ERROR_NONE) {                    \
                        CHECK_ERROR(ret);                                                       \
-                       return true;                                                            \
+                       markAsCompleted();                                                      \
+                       return;                                                                         \
                }                                                                                               \
        } while (false)
 
@@ -33,7 +34,7 @@ public:
                }
        }
 
-       bool process() override
+       void process() override
        {
                auto ret = app_control_create(&appControlHandle);
                RETURN_ON_ERROR(ret);
@@ -50,7 +51,7 @@ public:
                ret = app_control_send_launch_request(appControlHandle, NULL, NULL);
                RETURN_ON_ERROR(ret);
 
-               return true;
+               markAsCompleted();
        }
 protected:
        virtual int addExtraDataToCall()
index c62f175..4b40c0f 100644 (file)
@@ -18,11 +18,11 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                DBus::DBusClient dbus{dbusLocators::callmgr::BUS, dbusLocators::callmgr::OBJ_PATH, dbusLocators::callmgr::INTERFACE, DBus::ConnectionType::SYSTEM};
                dbus.method<int(int)>("AnswerCall").call(CM_TEL_CALL_ANSWER_TYPE_NORMAL);
-               return true;
+               markAsCompleted();
        }
 };
 
@@ -35,11 +35,11 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                DBus::DBusClient dbus{dbusLocators::callmgr::BUS, dbusLocators::callmgr::OBJ_PATH, dbusLocators::callmgr::INTERFACE, DBus::ConnectionType::SYSTEM};
                dbus.method<int()>("RejectCall").call();
-               return true;
+               markAsCompleted();
        }
 };
 
@@ -52,10 +52,10 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                DBus::DBusClient dbus{dbusLocators::callmgr::BUS, dbusLocators::callmgr::OBJ_PATH, dbusLocators::callmgr::INTERFACE, DBus::ConnectionType::SYSTEM};
                dbus.method<int(unsigned, int)>("EndCall").call(0, CM_TEL_CALL_RELEASE_TYPE_ALL_CALLS);
-               return true;
+               markAsCompleted();
        }
 };
index fa4a118..967d992 100644 (file)
@@ -15,15 +15,17 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                auto method = Singleton<VConfInterface>::instance().get(VCONF_KEY_SCAN_METHOD, 0);
-               if (!method)
-                       return true;
+               if (!method) {
+                       markAsCompleted();
+                       return;
+               }
 
                method = method % 2 + 1;
                Singleton<VConfInterface>::instance().set(VCONF_KEY_SCAN_METHOD, method);
                Singleton<UniversalSwitch>::instance().getScreenScannerManager()->startAutoscanning();
-               return true;
+               markAsCompleted();
        }
 };
index b07e60b..7dbc729 100644 (file)
@@ -11,7 +11,7 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                auto sound = Singleton<VConfInterface>::instance().get(VCONF_KEY_SOUND_ENABLED, true) ? 1 : 0;
                auto vibration = Singleton<VConfInterface>::instance().get(VCONF_KEY_VIBRATION_ENABLED, false) ? 1 : 0;
@@ -28,7 +28,7 @@ public:
                        changeProfile(false, false);
                        break;
                }
-               return true;
+               markAsCompleted();
        }
 
        void changeProfile(bool sound, bool vibration)
index 4235beb..d8cd5cd 100644 (file)
@@ -24,13 +24,13 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                changeKeyValue([](double currentValue) {
                        auto newValue = currentValue - VCONF_KEY_AUTO_SCAN_INTERVAL_VALUE_INCREMENT;
                        return newValue < VCONF_KEY_AUTO_SCAN_INTERVAL_VALUE_MIN ? VCONF_KEY_AUTO_SCAN_INTERVAL_VALUE_MIN : newValue;
                });
-               return true;
+               markAsCompleted();
        }
 };
 
@@ -42,12 +42,12 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                changeKeyValue([](double currentValue) {
                        auto newValue = currentValue + VCONF_KEY_AUTO_SCAN_INTERVAL_VALUE_INCREMENT;
                        return newValue > VCONF_KEY_AUTO_SCAN_INTERVAL_VALUE_MAX ? VCONF_KEY_AUTO_SCAN_INTERVAL_VALUE_MAX : newValue;
                });
-               return true;
+               markAsCompleted();
        }
 };
index 58e0256..a375d6f 100644 (file)
@@ -12,11 +12,11 @@ public:
                : UIActivity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                ASSERT(uiElement, "process invoked before uiElement initialization");
                uiElement->increment();
-               return true;
+               markAsCompleted();
        }
 };
 
@@ -28,10 +28,10 @@ public:
                : UIActivity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                ASSERT(uiElement, "process invoked before uiElement initialization");
                uiElement->decrement();
-               return true;
+               markAsCompleted();
        }
 };
index d36548d..698c414 100644 (file)
@@ -25,13 +25,13 @@ public:
                : UIActivity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                auto point = uiElement->getScanningCoordinates();
                DBus::DBusClient dbus {dbusLocators::accessibilityEMod::BUS, dbusLocators::accessibilityEMod::OBJ_PATH, dbusLocators::accessibilityEMod::INTERFACE, DBus::ConnectionType::SYSTEM};
                SwipeType swipeType = DerivedType::swipeType;
                dbus.method<void(int, int, int)>("DispatchGestureEvent").call(swipeType, point.x, point.y);
-               return true;
+               markAsCompleted();
        }
 };
 
index b057022..16549a2 100644 (file)
@@ -12,10 +12,10 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                utils::EventGenerator::generateKeyPress(DerivedType::buttonCode, DerivedType::multiplicity);
-               return true;
+               markAsCompleted();
        }
 };
 
index 105e555..a5459de 100644 (file)
@@ -13,12 +13,12 @@ public:
        constexpr static const char *activityType = DerivedType::activityType;
        MoveElementActivity() : Activity(activityType) {}
 
-       bool process() override
+       void process() override
        {
                auto s = Singleton<UniversalSwitch>::instance().getScreenScannerManager();
                if (s) DerivedType::move(s);
                else ERROR("ScreenScannerManager not found");
-               return true;
+               markAsCompleted();
        }
 };
 
index 6c66d10..dd82d62 100644 (file)
@@ -14,25 +14,26 @@ public:
        PowerKeyMenuActivity()
                : Activity(activityType) {}
 
-       bool process() override
+       void process() override
        {
                auto bundleHandle = std::unique_ptr<bundle, int(*)(bundle *)>(bundle_create(), bundle_free);
                if (!bundleHandle) {
-                       ERROR("bundle_create failure");
-                       return true;
+                       markAsCompleted();
+                       return;
                }
 
                auto ret = bundle_add(bundleHandle.get(), POPUP_CONTENT, POPUP_NAME_POWERKEY);
                if (ret < 0) {
                        ERROR("Failed to add bundle (%s,%s), error: %d", POPUP_CONTENT, POPUP_NAME_POWERKEY, ret);
-                       return true;
+                       markAsCompleted();
+                       return;
                }
 
                ret = syspopup_launch(const_cast<char *>(POPUP_POWERKEY), bundleHandle.get());
                if (ret < 0)
                        ERROR("Failed to launch popup, error: %d", ret);
 
-               return true;
+               markAsCompleted();
        }
 
 private:
index 57d9a66..ebf9542 100644 (file)
@@ -15,10 +15,10 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                DerivedType::changeQuickpanelState();
-               return true;
+               markAsCompleted();
        }
 };
 
index 0bcc063..a2dc6bf 100644 (file)
@@ -23,7 +23,7 @@ class ScreenshotActivity : public UIActivity, private RegisterActivity<Screensho
 {
 public:
        ScreenshotActivity();
-       bool process() override;
+       void process() override;
 
        constexpr static const char *activityType = "SCREENSHOT";
 
@@ -37,7 +37,7 @@ private:
 
 ScreenshotActivity::ScreenshotActivity() : UIActivity(activityType) {}
 
-bool ScreenshotActivity::process()
+void ScreenshotActivity::process()
 {
        char *path = nullptr;
        storage_get_directory(STORAGE_TYPE_INTERNAL, STORAGE_DIRECTORY_IMAGES, &path);
@@ -46,7 +46,8 @@ bool ScreenshotActivity::process()
 
        if (directory.empty()) {
                ERROR("No storage path");
-               return true;
+               markAsCompleted();
+               return;
        }
 
        auto filename = directory + "/" + generateFileName();
@@ -57,11 +58,9 @@ bool ScreenshotActivity::process()
                else
                        ERROR("No main window");
 
-               completed = true;
+               markAsCompleted();
                return ecore::TimerRepetitionPolicy::cancel;
        });
-
-       return false;
 }
 
 
index 7608d29..42b3f02 100644 (file)
@@ -37,7 +37,7 @@ public:
        ~SelectActivity();
        SelectActivity(SelectActivity &&) = delete;
 
-       bool process() override;
+       void process() override;
        void update(const std::shared_ptr<UIElement> &elem) override;
        void update(const std::shared_ptr<MenuItem> &menuitem) override;
 
@@ -91,11 +91,9 @@ SelectActivity::~SelectActivity()
        removeMenu();
 }
 
-bool SelectActivity::process()
+void SelectActivity::process()
 {
        DEBUG("Select - process called");
-       if (completed)
-               return true;
 
        if (timer.isSet()) {
                timer.reset();
@@ -107,8 +105,6 @@ bool SelectActivity::process()
 
        if (screenScannerManager)
                screenScannerManager->acceptAutoscanning();
-
-       return false;
 }
 
 void SelectActivity::update(const std::shared_ptr<UIElement> &elem)
@@ -252,7 +248,7 @@ void SelectActivity::navigateThroughSubMenuOrCreateActivityChangeRequest(MenuIte
 
        if (!menuItem->isRepeatable()) {
                removeMenu();
-               completed = true;
+               markAsCompleted();
        }
 
        notify(std::make_shared<ActivityChangeRequest>(menuItem->getActivityType(), realUiElement));
@@ -272,7 +268,7 @@ void SelectActivity::navigateBack()
        removeMenu();
        nestedMenusLabels.pop_back();
        if (nestedMenusLabels.empty())
-               completed = true;
+               markAsCompleted();
        else
                refreshMenu();
 }
@@ -280,7 +276,7 @@ void SelectActivity::navigateBack()
 void SelectActivity::sendTapActivityChangeRequest()
 {
        ASSERT(realUiElement, "realUiElement is NULL");
-       completed = true;
+       markAsCompleted();
        notify(std::make_shared<ActivityChangeRequest>("TAP", realUiElement));
 }
 
index dd8fb7a..35c5142 100644 (file)
@@ -215,11 +215,6 @@ void SwitchManager::changeActivity(const std::shared_ptr<ActivityChangeRequest>
                return;
        }
 
-       bool completed = activities.top()->process();
-
-       if (completed) {
-               DEBUG("Pop from stack: %s", activities.top()->getType().c_str());
-               activities.pop();
-       }
+       activities.top()->process();
        //TODO: check if activity change request observers are properly detached from activity object removed from activity stack
 }
index 043f130..2fc616a 100644 (file)
@@ -12,15 +12,16 @@ public:
                : UIActivity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                if (!uiElement) {
                        ERROR("process invoked before uiElement initialization");
-                       return true;
+                       markAsCompleted();
+                       return;
                }
 
                uiElement->activate();
-               return true;
+               markAsCompleted();
        }
 
 };
index 506cdb8..5aaa0ab 100644 (file)
@@ -11,10 +11,10 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                Singleton<VConfInterface>::instance().set(VCONF_KEY_AUTO_SCAN_ENABLED, true);
-               return true;
+               markAsCompleted();
        }
 };
 
@@ -26,9 +26,9 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                Singleton<VConfInterface>::instance().set(VCONF_KEY_AUTO_SCAN_ENABLED, false);
-               return true;
+               markAsCompleted();
        }
 };
index 59ce737..ecc79d3 100644 (file)
@@ -11,10 +11,10 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                auto isEnabled = Singleton<VConfInterface>::instance().get(VCONF_KEY_FEEDBACK_SOUND_ENABLED, false);
                Singleton<VConfInterface>::instance().set(VCONF_KEY_FEEDBACK_SOUND_ENABLED, !isEnabled);
-               return true;
+               markAsCompleted();
        }
 };
index 697323c..89d9d93 100644 (file)
@@ -13,10 +13,10 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                auto isEnabled = Singleton<VConfInterface>::instance().get(VCONF_KEY_FEEDBACK_VOICE_ENABLED, true);
                Singleton<VConfInterface>::instance().set(VCONF_KEY_FEEDBACK_VOICE_ENABLED, !isEnabled);
-               return true;
+               markAsCompleted();
        }
 };
index 345497c..f2d8e99 100644 (file)
@@ -5,11 +5,12 @@
 
 #include <sound_manager.h>
 
-#define RETURN_ON_ERROR(status) do {                                                                                           \
-               if (status != SOUND_MANAGER_ERROR_NONE) {                                                                       \
-                       ERROR("Sound manager error %d", status);                                                                \
-                       return true;                                                                                                                    \
-               }                                                                                                                                                       \
+#define RETURN_ON_ERROR(status) do {                                   \
+               if (status != SOUND_MANAGER_ERROR_NONE) {               \
+                       ERROR("Sound manager error %d", status);        \
+                       markAsCompleted();                                                      \
+                       return;                                                                         \
+               }                                                                                               \
        } while (false)
 
 #define RETURN_DEFAULT_SOUND_TYPE_ON_ERROR(status) do {                                                                \
@@ -31,7 +32,7 @@ public:
                : Activity(activityType)
        {}
 
-       bool process() override
+       void process() override
        {
                int volume = 0;
                int maxVolume = 0;
@@ -44,7 +45,7 @@ public:
                volume = utils::clamp(volume + DerivedType::step, minVolume, maxVolume);
                RETURN_ON_ERROR(sound_manager_set_volume(soundType, volume));
 
-               return true;
+               markAsCompleted();
        }
 
        virtual sound_type_e getSoundType() const
index a19a696..2579f96 100644 (file)
@@ -37,8 +37,10 @@ TEST_F(ActivityProcessingFixture, ActivityReady)
                EXPECT_EQ(activity->getType(), "TWO_STEP_PROCESS_ACTIVITY");
 
        simulateKeyPress(keyMappedToTwoStepProcessActivity_);
-       auto isEmpty = switchManager_->getActivities().empty();
-       EXPECT_EQ(isEmpty, true);
+       activity = switchManager_->getActivities().top();
+       EXPECT_NE(activity, nullptr);
+       if (activity)
+               EXPECT_TRUE(activity->isCompleted());
 }
 
 /*
@@ -48,30 +50,45 @@ TEST_F(ActivityProcessingFixture, ActivityReady)
 TEST_F(ActivityProcessingFixture, ActivityAbandoned)
 {
        initialEnvironmentExpect();
-       auto activity = switchManager_->getActivities().top();
-       EXPECT_NE(activity, nullptr);
-       if (activity)
-               EXPECT_EQ(activity->getType(), "TWO_STEP_PROCESS_ACTIVITY");
+
+       auto twoStepProcessActivity = switchManager_->getActivities().top();
+       EXPECT_NE(twoStepProcessActivity, nullptr);
+       if (!twoStepProcessActivity)
+               return;
+
+       EXPECT_EQ(twoStepProcessActivity->getType(), "TWO_STEP_PROCESS_ACTIVITY");
+       EXPECT_FALSE(twoStepProcessActivity->isCompleted());
 
        simulateKeyPress(keyMappedToThreeStepProcessActivity_);
+
        EXPECT_EQ(switchManager_->getActivities().size(), 2u);
-       activity = switchManager_->getActivities().top();
-       EXPECT_NE(activity, nullptr);
-       if (activity)
-               EXPECT_EQ(activity->getType(), "THREE_STEP_PROCESS_ACTIVITY");
+
+       auto threeStepProcessActivity = switchManager_->getActivities().top();
+       EXPECT_NE(threeStepProcessActivity, nullptr);
+       if (!threeStepProcessActivity)
+               return;
+
+       EXPECT_EQ(threeStepProcessActivity->getType(), "THREE_STEP_PROCESS_ACTIVITY");
+       EXPECT_FALSE(twoStepProcessActivity->isCompleted());
+       EXPECT_FALSE(threeStepProcessActivity->isCompleted());
 
        simulateKeyPress(keyMappedToThreeStepProcessActivity_);
        EXPECT_EQ(switchManager_->getActivities().size(), 2u);
-       activity = switchManager_->getActivities().top();
+       EXPECT_EQ(threeStepProcessActivity, switchManager_->getActivities().top());
+       EXPECT_FALSE(twoStepProcessActivity->isCompleted());
+       EXPECT_FALSE(threeStepProcessActivity->isCompleted());
 
-       EXPECT_NE(activity, nullptr);
-       if (activity)
-               EXPECT_EQ(activity->getType(), "THREE_STEP_PROCESS_ACTIVITY");
 
        simulateKeyPress(keyMappedToThreeStepProcessActivity_);
+
+       EXPECT_EQ(switchManager_->getActivities().size(), 2u);
+       EXPECT_EQ(threeStepProcessActivity, switchManager_->getActivities().top());
+       EXPECT_FALSE(twoStepProcessActivity->isCompleted());
+       EXPECT_TRUE(threeStepProcessActivity->isCompleted());
+
+       simulateKeyPress(keyMappedToTwoStepProcessActivity_);
+
        EXPECT_EQ(switchManager_->getActivities().size(), 1u);
-       activity = switchManager_->getActivities().top();
-       EXPECT_NE(activity, nullptr);
-       if (activity)
-               EXPECT_EQ(activity->getType(), "TWO_STEP_PROCESS_ACTIVITY");
+       EXPECT_EQ(twoStepProcessActivity, switchManager_->getActivities().top());
+       EXPECT_TRUE(twoStepProcessActivity->isCompleted());
 }
index 21272c5..f0b96f2 100644 (file)
@@ -33,9 +33,10 @@ public:
                static constexpr const char *activityType = "TWO_STEP_PROCESS_ACTIVITY";
                TwoStepProcessActivity(): Activity(activityType) {}
 
-               bool process() override
+               void process() override
                {
-                       return ++state > 1;
+                       if (++state > 1)
+                               markAsCompleted();
                };
                int state = 0;
        };
@@ -46,9 +47,10 @@ public:
                static constexpr const char *activityType = "THREE_STEP_PROCESS_ACTIVITY";
                ThreeStepProcessActivity(): Activity(activityType) {}
 
-               bool process() override
+               void process() override
                {
-                       return ++state > 2;
+                       if (++state > 2)
+                               markAsCompleted();
                };
                int state = 0;
        };