Merge lp:~aacid/qmenumodel/aboutToShow into lp:qmenumodel

[Albert Astals Cid]

Call about to show for those items that have a qtubuntu-tag property

Silo available at https://bileto.ubuntu.com/#/ticket/2548

Goes together with
https://code.launchpad.net/~aacid/qtubuntu/aboutToShow/+merge/319080
https://code.launchpad.net/~aacid/unity8/aboutToShow/+merge/319079

[lp:~aacid/qmenumodel/aboutToShow]

130. By Albert Astals Cid on 2017-03-07

Increase version because of new api

[Charles Kerr (charlesk)]

comments inline

review: Needs Fixing

[Albert Astals Cid (aacid)]

I will add all those auto, but those are *totally* not on the style of the rest of the file, basically the file has 0 auto variables.

> You probably meant to use -1 here to use the default timeout? I'm not sure what specifying a timeout of 0msec would do but it's probably not good
No, I don't want the default timeout, i simply want no timeout, it either works or doesn't, i don't see why it's "probably not good" to use a 0 timeout

> Not a blocker, but want to confirm that you meant to not provide a callback here so that you can log any errors reported by g_dbus_connection_call_finish(), yes?
Yes, if you want I can add error reporting, but don't think it's going to give us much tbh.

[lp:~aacid/qmenumodel/aboutToShow]

131. By Albert Astals Cid on 2017-03-16

Review improvements

[Charles Kerr (charlesk)]

> I will add all those auto, but those are *totally* not on the style of the
> rest of the file, basically the file has 0 auto variables.

No worries, as I said those were trivial comments. Redundant type info
is bad, but inconsistency is also bad, pick your minor poison :)

> > You probably meant to use -1 here to use the default timeout? I'm not sure
> what specifying a timeout of 0msec would do but it's probably not good
> No, I don't want the default timeout, i simply want no timeout, it either
> works or doesn't, i don't see why it's "probably not good" to use a 0 timeout

Because the docs say use G_MAXINT for no timeout.

g_dbus_connection_call() is in gdbusconnection.c, which the call down a few steps.
The timeout is applied in g_dbus_connection_send_message_with_reply_unlocked(),
which reads:

  if (timeout_msec != G_MAXINT)
    {
      data->timeout_source = g_timeout_source_new (timeout_msec);
      g_task_attach_source (task, data->timeout_source,
                            (GSourceFunc) send_message_with_reply_timeout_cb);
      g_source_unref (data->timeout_source);
    }

I /think/ you are effectively asking the call to timeout asap.

> > Not a blocker, but want to confirm that you meant to not provide a callback
> here so that you can log any errors reported by
> g_dbus_connection_call_finish(), yes?
> Yes, if you want I can add error reporting, but don't think it's going to give
> us much tbh.

Agreed that it's minor.

Keeping as NF for the G_MAXINT issue

review: Needs Fixing

[lp:~aacid/qmenumodel/aboutToShow]

132. By Albert Astals Cid on 2017-03-16

0 -> G_MAXINT as found by Charles

[Charles Kerr (charlesk)]

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2017-01-10 06:41:53 +0000
+++ debian/changelog	2017-03-16 15:50:45 +0000
@@ -1,3 +1,10 @@
+qmenumodel (0.2.12) UNRELEASED; urgency=medium
+
+  [ Albert Astals Cid ]
+  * Add UnityMenuModel::aboutToShow ((LP: 1664578)
+
+ -- Albert Astals Cid <[e-mail]>  Tue, 07 Mar 2017 13:08:46 +0100
+
 qmenumodel (0.2.11+17.04.20170110.1-0ubuntu1) zesty; urgency=medium
 
   [ Nick Dedekind ]

=== modified file 'libqmenumodel/src/unitymenumodel.cpp'
--- libqmenumodel/src/unitymenumodel.cpp	2016-12-09 17:06:25 +0000
+++ libqmenumodel/src/unitymenumodel.cpp	2017-03-16 15:50:45 +0000
@@ -289,7 +289,7 @@
     priv = new UnityMenuModelPrivate(this);
 }
 
-UnityMenuModel::UnityMenuModel(const UnityMenuModelPrivate& other, QObject *parent):
+UnityMenuModel::UnityMenuModel(const UnityMenuModelPrivate& other, UnityMenuModel *parent):
     QAbstractListModel(parent)
 {
     priv = new UnityMenuModelPrivate(other, this);
@@ -729,6 +729,47 @@
     }
 }
 
+void UnityMenuModel::aboutToShow(int index)
+{
+    GSequenceIter *it = g_sequence_get_iter_at_pos (priv->items, index);
+    if (g_sequence_iter_is_end (it)) {
+        return;
+    }
+
+    auto item = static_cast<GtkMenuTrackerItem*>(g_sequence_get(it));
+    if (!item) {
+        return;
+    }
+
+    quint64 actionTag;
+    if (gtk_menu_tracker_item_get_attribute (item, "qtubuntu-tag", "t", &actionTag)) {
+        // Child UnityMenuModel have priv->connection null, so climb to the parent until we find a non null one
+        UnityMenuModelPrivate *privToUse = priv;
+        while (privToUse && !privToUse->connection) {
+            auto pModel = dynamic_cast<UnityMenuModel*>(privToUse->model->QObject::parent());
+            if (pModel) {
+                privToUse = pModel->priv;
+            } else {
+                privToUse = nullptr;
+            }
+        }
+        if (privToUse) {
+            g_dbus_connection_call (privToUse->connection,
+                                    privToUse->busName,
+                                    privToUse->menuObjectPath,
+                                    "qtubuntu.actions.extra",
+                                    "aboutToShow",
+                                    g_variant_new("(t)", actionTag),
+                                    nullptr,
+                                    G_DBUS_CALL_FLAGS_NO_AUTO_START,
+                                    G_MAXINT,
+                                    nullptr,
+                                    nullptr,
+                                    nullptr);
+        }
+    }
+}
+
 void UnityMenuModel::activateByVariantString(int index, const QString& parameter)
 {
     activate(index, Converter::toQVariantFromVariantString(parameter));

=== modified file 'libqmenumodel/src/unitymenumodel.h'
--- libqmenumodel/src/unitymenumodel.h	2016-10-18 09:55:14 +0000
+++ libqmenumodel/src/unitymenumodel.h	2017-03-16 15:50:45 +0000
@@ -63,6 +63,7 @@
     Q_INVOKABLE QVariant get(int row, const QByteArray &role);
 
     Q_INVOKABLE void activate(int index, const QVariant& parameter = QVariant());
+    Q_INVOKABLE void aboutToShow(int index);
     Q_INVOKABLE void activateByVariantString(int index, const QString& parameter = QString());
     Q_INVOKABLE void changeState(int index, const QVariant& parameter);
     Q_INVOKABLE void changeStateByVariantString(int index, const QString& parameter);
@@ -90,7 +91,7 @@
     class UnityMenuModelPrivate *priv;
     friend class UnityMenuModelPrivate;
 
-    UnityMenuModel(const UnityMenuModelPrivate& other, QObject *parent);
+    UnityMenuModel(const UnityMenuModelPrivate& other, UnityMenuModel *parent);
 };
 
 #endif