Merge lp:~alecu/unity-scope-click/simplify-bridge-take2 into lp:unity-scope-click/devel

[Alejandro J. Cura]

Remove unused code in the qtbridge

This branch gets rid of HeapAllocatedObject and Environment, since they have no unit tests and are no longer being used.

[dobey (dobey)]

Noice!

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:209
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/438/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/339
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/336
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/336/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/438/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:210
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/448/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/349/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/346/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/448/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:212
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/449/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/350
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/347
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/347/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/449/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:212
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/7/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-amd64-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-armhf-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-i386-ci/7/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/7/rebuild

review: Needs Fixing (continuous-integration)

[lp:~alecu/unity-scope-click/simplify-bridge-take2]

213. By Alejandro J. Cura on 2014-04-25

merged with lp:unity-scope-click/devel

214. By Alejandro J. Cura on 2014-04-25

Add fix for new usage of enter_with_task

[dobey (dobey)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:214
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/10/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-armhf-ci/10/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-i386-ci/10

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/10/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'scope/click/download-manager.cpp'
--- scope/click/download-manager.cpp	2014-03-22 00:08:31 +0000
+++ scope/click/download-manager.cpp	2014-04-25 18:38:37 +0000
@@ -335,7 +335,7 @@
 void click::Downloader::startDownload(std::string url, std::string package_name,
                                       const std::function<void (std::pair<std::string, click::InstallError >)>& callback)
 {
-    qt::core::world::enter_with_task([this, callback, url, package_name] (qt::core::world::Environment& /*env*/)
+    qt::core::world::enter_with_task([this, callback, url, package_name] ()
     {
         auto& dm = downloadManagerInstance(networkAccessManager);
 

=== modified file 'scope/click/preview.cpp'
--- scope/click/preview.cpp	2014-04-24 17:11:54 +0000
+++ scope/click/preview.cpp	2014-04-25 18:38:37 +0000
@@ -99,7 +99,7 @@
         // I think this should not be required when we switch the click::Index over
         // to using the Qt bridge. With that, the qt dependency becomes an implementation detail
         // and code using it does not need to worry about threading/event loop topics.
-        qt::core::world::enter_with_task([this, details_callback, reviews_callback, app_name](qt::core::world::Environment&)
+        qt::core::world::enter_with_task([this, details_callback, reviews_callback, app_name]()
             {
                 index_operation = index->get_details(app_name, [this, app_name, details_callback, reviews_callback](PackageDetails details, click::Index::Error error){
                     if(error == click::Index::Error::NoError) {
@@ -346,7 +346,7 @@
     std::future<bool> manifest_future = manifest_promise.get_future();
     std::string app_name = result["name"].get_string();
     if (!app_name.empty()) {
-        qt::core::world::enter_with_task([&](qt::core::world::Environment& /*env*/) {
+        qt::core::world::enter_with_task([&]() {
             click::Interface().get_manifest_for_app(app_name,
                 [&](Manifest manifest, ManifestError error) {
                     qDebug() << "Got manifest for:" << app_name.c_str();
@@ -415,7 +415,7 @@
     // this can happen if the app was just installed and we have its http uri from the Result.
     if (!app_url.startsWith("application:///")) {
         const std::string name = result["name"].get_string();
-        auto ft = qt::core::world::enter_with_task([this, name, callback] (qt::core::world::Environment& /*env*/)
+        auto ft = qt::core::world::enter_with_task([this, name, callback] ()
         {
             click::Interface().get_dotdesktop_filename(name,
                                           [callback] (std::string val, click::ManifestError error) {
@@ -577,7 +577,7 @@
     package.title = result.title();
     package.name = result["name"].get_string();
     package.version = result["version"].get_string();
-    qt::core::world::enter_with_task([this, package] (qt::core::world::Environment& /*env*/)
+    qt::core::world::enter_with_task([this, package] ()
     {
         click::PackageManager manager;
         manager.uninstall(package, [&](int code, std::string stderr_content) {

=== modified file 'scope/click/qtbridge.cpp'
--- scope/click/qtbridge.cpp	2014-02-07 11:14:48 +0000
+++ scope/click/qtbridge.cpp	2014-04-25 18:38:37 +0000
@@ -39,10 +39,6 @@
 {
 namespace world
 {
-Environment::Environment(QObject *parent) : QObject(parent)
-{
-}
-
 namespace detail
 {
 QEvent::Type qt_core_world_task_event_type()
@@ -51,28 +47,20 @@
     return event_type;
 }
 
-class Environment : public qt::core::world::Environment
-{
-public:
-    Environment(QObject* parent) : qt::core::world::Environment(parent)
-    {
-    }
-};
-
 class TaskEvent : public QEvent
 {
 public:
-    TaskEvent(const std::function<void(qt::core::world::Environment&)>& task)
+    TaskEvent(const std::function<void()>& task)
         : QEvent(qt_core_world_task_event_type()),
           task(task)
     {
     }
 
-    void run(qt::core::world::Environment& env)
+    void run()
     {
         try
         {
-            task(env);
+            task();
             promise.set_value();
         } catch(...)
         {
@@ -86,7 +74,7 @@
     }
 
 private:
-    std::function<void(qt::core::world::Environment&)> task;
+    std::function<void()> task;
     std::promise<void> promise;
 };
 
@@ -125,12 +113,6 @@
     return instance;
 }
 
-qt::core::world::Environment* environment()
-{
-    static detail::Environment* env = new detail::Environment(coreApplicationInstance());
-    return env;
-}
-
 bool TaskHandler::event(QEvent *e)
 {
     if (e->type() != qt_core_world_task_event_type())
@@ -139,7 +121,7 @@
     auto te = dynamic_cast<TaskEvent*>(e);
     if (te)
     {
-        te->run(*environment());
+        te->run();
         return true;
     }
 
@@ -160,9 +142,6 @@
     detail::task_handler()->moveToThread(
                 detail::coreApplicationInstance()->thread());
 
-    detail::environment()->moveToThread(
-                detail::coreApplicationInstance()->thread());
-
     // Signal to other worlds that we are good to go.
     ready();
 
@@ -174,14 +153,14 @@
 
 void destroy()
 {
-    enter_with_task([](qt::core::world::Environment&)
+    enter_with_task([]()
     {
         // We make sure that all tasks have completed before quitting the app.
         QEventLoopLocker locker;
     }).wait_for(std::chrono::seconds{1});
 }
 
-std::future<void> enter_with_task(const std::function<void(qt::core::world::Environment&)>& task)
+std::future<void> enter_with_task(const std::function<void()>& task)
 {
     QCoreApplication* instance = QCoreApplication::instance();
 

=== modified file 'scope/click/qtbridge.h'
--- scope/click/qtbridge.h	2014-02-24 12:41:57 +0000
+++ scope/click/qtbridge.h	2014-04-25 18:38:37 +0000
@@ -32,134 +32,6 @@
 {
 namespace world
 {
-// Forward declaration to allow for friend declaration in HeapAllocatedObject.
-class Environment;
-}
-}
-
-/**
- * @brief Models an object allocated on the heap in qt world, but pulled over
- * to this world to be passed on to tasks. Please note that compilation will
- * abort if QObject is not a base of T.
- *
- * One other important thing to note: The destrucotor will throw a std::runtime_error
- * if a HeapAllocatedObject has not been free'd in Qt world and does _not_ have a parent
- * assigned. In that case, the object would be leaked.
- *
- */
-template<typename T>
-class HeapAllocatedObject
-{
-public:
-    static_assert(
-            std::is_base_of<QObject, T>::value,
-            "HeapAllocatedObject<T> is only required to transport QObject"
-            " subclasses over the world boundary.");
-
-    /** Constructs an empty, invalid instance. */
-    HeapAllocatedObject() = default;
-
-    /** HeapAllocatedObjects, i.e., their shared state, can be copied. */
-    HeapAllocatedObject(const HeapAllocatedObject<T>& rhs) = default;
-
-    /** HeapAllocatedObjects, i.e., their shared state can be assigned. */
-    HeapAllocatedObject& operator=(const HeapAllocatedObject<T>& rhs) = default;
-
-    /** HeapAllocatedObjects, i.e., their shared state can be compared for equality. */
-    inline bool operator==(const HeapAllocatedObject<T>& rhs) const
-    {
-        return state == rhs.state || state->instance == rhs.state->instance;
-    }
-
-    /** Check if this object contains a valid instance. */
-    inline operator bool() const
-    {
-        return state && (state->instance != nullptr);
-    }
-
-private:
-    friend class qt::core::world::Environment;
-
-    struct Private
-    {
-        Private(T* instance) : instance(instance)
-        {
-        }
-
-        ~Private()
-        {
-            if (instance != nullptr && instance->parent() == nullptr)
-            {
-                std::cerr << "HeapAllocatedObject::Private::~Private: Giving up ownership "
-                             "on a QObject instance without a parent: "
-                          << std::string(qPrintable(instance->metaObject()->className()))
-                          << std::endl;
-                ::abort();
-            }
-        }
-
-        T* instance{nullptr};
-    };
-
-    HeapAllocatedObject(T* instance) : state(std::make_shared<Private>(instance))
-    {
-    }
-
-    std::shared_ptr<Private> state;
-};
-
-namespace core
-{
-namespace world
-{
-/**
- * @brief The Environment class models the environment in the Qt world
- * that tasks operate under.
- */
-class Environment : public QObject
-{
-    Q_OBJECT
-
-public:
-    /**
-     * @brief Allocates subclasses of a QObject in the Qt world to make sure
-     * thread assumptions are satisfied.
-     *
-     * @tparam Args Construction argument types to be passed to the class's c'tor.
-     * @param args Construction arguments to be passed to the class's c'tor.
-     */
-    template<typename T, typename... Args>
-    qt::HeapAllocatedObject<T> allocate(Args... args)
-    {
-        return qt::HeapAllocatedObject<T>{new T(args...)};
-    }
-
-    /**
-     * @brief Frees a previously allocated object in the Qt world.
-     * @param object The object to be freed.
-     */
-    template<typename T>
-    void free(const qt::HeapAllocatedObject<T>& object)
-    {
-        delete object.state->instance;
-        object.state->instance = nullptr;
-    }
-
-    /**
-     * @brief Provides access to the instance contained in the object handle.
-     * @param object The object containing the instance to be accessed.
-     * @return A pointer to an instance of T, or nullptr.
-     */
-    template<typename T>
-    T* resolve(const HeapAllocatedObject<T>& object)
-    {
-        return object.state->instance;
-    }
-
-protected:
-    Environment(QObject* parent);
-};
-
 /**
  * @brief Sets up the Qt core world and executes its event loop. Blocks until destroy() is called.
  * @param argc Number of arguments in argv.
@@ -179,7 +51,7 @@
  * @param task The task to be executed in the Qt core world.
  * @return A std::future that can be waited for to synchronize to the world's internal event loop.
  */
-std::future<void> enter_with_task(const std::function<void(Environment&)>& task);
+std::future<void> enter_with_task(const std::function<void()>& task);
 
 
 /**
@@ -188,16 +60,16 @@
  * @return A std::future that can be waited for to get hold of the result of the task.
  */
 template<typename T>
-inline std::future<T> enter_with_task_and_expect_result(const std::function<T(Environment&)>& task)
+inline std::future<T> enter_with_task_and_expect_result(const std::function<T()>& task)
 {
     std::shared_ptr<std::promise<T>> promise = std::make_shared<std::promise<T>>();
     std::future<T> future = promise->get_future();
 
-    auto t = [promise, task](Environment& env)
+    auto t = [promise, task]()
     {
         try
         {
-            promise->set_value(task(env));
+            promise->set_value(task());
         } catch(...)
         {
             promise->set_exception(std::current_exception());

=== modified file 'scope/click/query.cpp'
--- scope/click/query.cpp	2014-04-16 20:26:26 +0000
+++ scope/click/query.cpp	2014-04-25 18:38:37 +0000
@@ -169,10 +169,10 @@
     return searchReply->register_category(id, title, icon, renderer_template);
 }
 
-void click::Query::run_under_qt(const std::function<void (qt::core::world::Environment &)> &task)
+void click::Query::run_under_qt(const std::function<void ()> &task)
 {
-    qt::core::world::enter_with_task([task](qt::core::world::Environment& env) {
-        task(env);
+    qt::core::world::enter_with_task([task]() {
+        task();
     });
 }
 
@@ -188,7 +188,7 @@
         return;
     }
 
-    run_under_qt([=](qt::core::world::Environment& /*env*/)
+    run_under_qt([=]()
     {
         qDebug() << "starting search of" << QString::fromStdString(impl->query);
 

=== modified file 'scope/click/query.h'
--- scope/click/query.h	2014-04-12 19:19:36 +0000
+++ scope/click/query.h	2014-04-25 18:38:37 +0000
@@ -38,17 +38,6 @@
 #include <QSharedPointer>
 #include <set>
 
-namespace qt
-{
-namespace core
-{
-namespace world
-{
-class Environment;
-}
-}
-}
-
 namespace click
 {
 
@@ -98,7 +87,7 @@
                                                std::string const& title,
                                                std::string const& icon,
                                                scopes::CategoryRenderer const& renderer_template);
-    virtual void run_under_qt(const std::function<void(qt::core::world::Environment&)>& task);
+    virtual void run_under_qt(const std::function<void()> &task);
 
 private:
     struct Private;

=== modified file 'scope/tests/test_query.cpp'
--- scope/tests/test_query.cpp	2014-04-14 20:48:43 +0000
+++ scope/tests/test_query.cpp	2014-04-25 18:38:37 +0000
@@ -76,15 +76,6 @@
                       std::function<void(click::PackageList)>));
 };
 
-class MockEnvironment : public qt::core::world::Environment
-{
-public:
-    MockEnvironment(QObject* parent) : qt::core::world::Environment(parent)
-    {
-    }
-};
-
-
 class MockQueryBase : public click::Query {
 public:
     MockQueryBase(const std::string query, click::Index& index,
@@ -93,10 +84,9 @@
 
     }
 
-    void run_under_qt(const std::function<void (qt::core::world::Environment &)> &task) {
+    void run_under_qt(const std::function<void()> &task) {
         // when testing, do not actually run under qt
-        MockEnvironment env(nullptr);
-        task(env);
+        task();
     }
 };