ActionManager: Centralize ActionContainer update

Each ActionContainer used to schedule an invokeMethod on the event loop
when it changes, which creates an unnecessary amount of individual
events, especially during startup.

Instead schedule that update through the ActionManager, which then can
schedule a single event that handles all updates up to that time.

Change-Id: Ia193606adc4cfda88df8e45f2027662ca16f2ad3
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/coreplugin/actionmanager/actioncontainer.cpp

@@ -172,8 +172,11 @@ namespace Internal {
     \internal
 */
 
-ActionContainerPrivate::ActionContainerPrivate(Id id)
+ActionContainerPrivate::ActionContainerPrivate(Id id, ActionManagerPrivate *actionManagerPrivate)
-    : m_onAllDisabledBehavior(Disable), m_id(id), m_updateRequested(false)
+    : m_onAllDisabledBehavior(Disable)
+    , m_id(id)
+    , m_actionManagerPrivate(actionManagerPrivate)
+    , m_updateRequested(false)
 {
     appendGroup(Constants::G_DEFAULT_ONE);
     appendGroup(Constants::G_DEFAULT_TWO);
@@ -382,16 +385,7 @@ bool ActionContainerPrivate::canAddAction(Command *action)
 
 void ActionContainerPrivate::scheduleUpdate()
 {
-    if (m_updateRequested)
+    m_actionManagerPrivate->scheduleContainerUpdate(this);
-        return;
-    m_updateRequested = true;
-    QMetaObject::invokeMethod(this, &ActionContainerPrivate::update, Qt::QueuedConnection);
-}
-
-void ActionContainerPrivate::update()
-{
-    updateInternal();
-    m_updateRequested = false;
 }
 
 // ---------- MenuActionContainer ------------
@@ -401,9 +395,9 @@ void ActionContainerPrivate::update()
     \internal
 */
 
-MenuActionContainer::MenuActionContainer(Id id)
+MenuActionContainer::MenuActionContainer(Id id, ActionManagerPrivate *actionManagerPrivate)
-    : ActionContainerPrivate(id),
+    : ActionContainerPrivate(id, actionManagerPrivate)
-      m_menu(new QMenu)
+    , m_menu(new QMenu)
 {
     m_menu->setObjectName(id.toString());
     m_menu->menuAction()->setMenuRole(QAction::NoRole);
@@ -450,7 +444,7 @@ void MenuActionContainer::removeMenu(ActionContainer *container)
     m_menu->removeAction(menu->menuAction());
 }
 
-bool MenuActionContainer::updateInternal()
+bool MenuActionContainer::update()
 {
     if (onAllDisabledBehavior() == Show)
         return true;
@@ -470,7 +464,7 @@ bool MenuActionContainer::updateInternal()
                     qWarning("%s", warning.constData());
                     continue;
                 }
-                if (container->updateInternal()) {
+                if (container->update()) {
                     hasitems = true;
                     break;
                 }
@@ -520,8 +514,9 @@ bool MenuActionContainer::canBeAddedToContainer(ActionContainerPrivate *containe
     \internal
 */
 
-MenuBarActionContainer::MenuBarActionContainer(Id id)
+MenuBarActionContainer::MenuBarActionContainer(Id id, ActionManagerPrivate *actionManagerPrivate)
-    : ActionContainerPrivate(id), m_menuBar(nullptr)
+    : ActionContainerPrivate(id, actionManagerPrivate)
+    , m_menuBar(nullptr)
 {
     setOnAllDisabledBehavior(Show);
 }
@@ -566,7 +561,7 @@ void MenuBarActionContainer::removeMenu(ActionContainer *container)
     m_menuBar->removeAction(menu->menuAction());
 }
 
-bool MenuBarActionContainer::updateInternal()
+bool MenuBarActionContainer::update()
 {
     if (onAllDisabledBehavior() == Show)
         return true;
@@ -597,9 +592,12 @@ bool MenuBarActionContainer::canBeAddedToContainer(ActionContainerPrivate *) con
 
 const char ID_PREFIX[] = "io.qt.qtcreator.";
 
-TouchBarActionContainer::TouchBarActionContainer(Id id, const QIcon &icon, const QString &text)
+TouchBarActionContainer::TouchBarActionContainer(Id id,
-    : ActionContainerPrivate(id),
+                                                 ActionManagerPrivate *actionManagerPrivate,
-      m_touchBar(std::make_unique<TouchBar>(id.withPrefix(ID_PREFIX).name(), icon, text))
+                                                 const QIcon &icon,
+                                                 const QString &text)
+    : ActionContainerPrivate(id, actionManagerPrivate)
+    , m_touchBar(std::make_unique<TouchBar>(id.withPrefix(ID_PREFIX).name(), icon, text))
 {
 }
 
@@ -653,7 +651,7 @@ bool TouchBarActionContainer::canBeAddedToContainer(ActionContainerPrivate *cont
     return qobject_cast<TouchBarActionContainer *>(container);
 }
 
-bool TouchBarActionContainer::updateInternal()
+bool TouchBarActionContainer::update()
 {
     return false;
 }

src/plugins/coreplugin/actionmanager/actioncontainer_p.h

@@ -25,7 +25,7 @@ class ActionContainerPrivate : public ActionContainer
     Q_OBJECT
 
 public:
-    ActionContainerPrivate(Utils::Id id);
+    ActionContainerPrivate(Utils::Id id, ActionManagerPrivate *actionManagerPrivate);
     ~ActionContainerPrivate() override = default;
 
     void setOnAllDisabledBehavior(OnAllDisabledBehavior behavior) final;
@@ -55,7 +55,7 @@ public:
     virtual void removeAction(Command *command) = 0;
     virtual void removeMenu(ActionContainer *container) = 0;
 
-    virtual bool updateInternal() = 0;
+    virtual bool update() = 0;
 
 protected:
     static bool canAddAction(Command *action);
@@ -67,7 +67,6 @@ protected:
 
 private:
     void scheduleUpdate();
-    void update();
     void itemDestroyed(QObject *sender);
 
     QList<Group>::const_iterator findGroup(Utils::Id groupId) const;
@@ -75,6 +74,7 @@ private:
 
     OnAllDisabledBehavior m_onAllDisabledBehavior;
     Utils::Id m_id;
+    ActionManagerPrivate *m_actionManagerPrivate = nullptr;
     bool m_updateRequested;
 };
 
@@ -83,7 +83,7 @@ class MenuActionContainer : public ActionContainerPrivate
     Q_OBJECT
 
 public:
-    explicit MenuActionContainer(Utils::Id id);
+    explicit MenuActionContainer(Utils::Id id, ActionManagerPrivate *actionManagerPrivate);
     ~MenuActionContainer() override;
 
     QMenu *menu() const override;
@@ -98,7 +98,7 @@ public:
 
 protected:
     bool canBeAddedToContainer(ActionContainerPrivate *container) const override;
-    bool updateInternal() override;
+    bool update() override;
 
 private:
     QPointer<QMenu> m_menu;
@@ -109,7 +109,7 @@ class MenuBarActionContainer : public ActionContainerPrivate
     Q_OBJECT
 
 public:
-    explicit MenuBarActionContainer(Utils::Id id);
+    explicit MenuBarActionContainer(Utils::Id id, ActionManagerPrivate *actionManagerPrivate);
 
     void setMenuBar(QMenuBar *menuBar);
     QMenuBar *menuBar() const override;
@@ -124,7 +124,7 @@ public:
 
 protected:
     bool canBeAddedToContainer(ActionContainerPrivate *container) const override;
-    bool updateInternal() override;
+    bool update() override;
 
 private:
     QMenuBar *m_menuBar;
@@ -135,7 +135,10 @@ class TouchBarActionContainer : public ActionContainerPrivate
     Q_OBJECT
 
 public:
-    TouchBarActionContainer(Utils::Id id, const QIcon &icon, const QString &text);
+    TouchBarActionContainer(Utils::Id id,
+                            ActionManagerPrivate *actionManagerPrivate,
+                            const QIcon &icon,
+                            const QString &text);
     ~TouchBarActionContainer() override;
 
     Utils::TouchBar *touchBar() const override;
@@ -150,7 +153,7 @@ public:
     void removeMenu(ActionContainer *container) override;
 
     bool canBeAddedToContainer(ActionContainerPrivate *container) const override;
-    bool updateInternal() override;
+    bool update() override;
 
 private:
     std::unique_ptr<Utils::TouchBar> m_touchBar;

src/plugins/coreplugin/actionmanager/actionmanager.cpp

@@ -188,7 +188,7 @@ ActionContainer *ActionManager::createMenu(Id id)
     if (it !=  d->m_idContainerMap.constEnd())
         return it.value();
 
-    auto mc = new MenuActionContainer(id);
+    auto mc = new MenuActionContainer(id, d);
 
     d->m_idContainerMap.insert(id, mc);
     connect(mc, &QObject::destroyed, d, &ActionManagerPrivate::containerDestroyed);
@@ -213,7 +213,7 @@ ActionContainer *ActionManager::createMenuBar(Id id)
     auto mb = new QMenuBar; // No parent (System menu bar on macOS)
     mb->setObjectName(id.toString());
 
-    auto mbc = new MenuBarActionContainer(id);
+    auto mbc = new MenuBarActionContainer(id, d);
     mbc->setMenuBar(mb);
 
     d->m_idContainerMap.insert(id, mbc);
@@ -241,7 +241,7 @@ ActionContainer *ActionManager::createTouchBar(Id id, const QIcon &icon, const Q
     ActionContainer * const c = d->m_idContainerMap.value(id);
     if (c)
         return c;
-    auto ac = new TouchBarActionContainer(id, icon, text);
+    auto ac = new TouchBarActionContainer(id, d, icon, text);
     d->m_idContainerMap.insert(id, ac);
     connect(ac, &QObject::destroyed, d, &ActionManagerPrivate::containerDestroyed);
     return ac;
@@ -449,6 +449,7 @@ void ActionManagerPrivate::containerDestroyed(QObject *sender)
 {
     auto container = static_cast<ActionContainerPrivate *>(sender);
     m_idContainerMap.remove(m_idContainerMap.key(container));
+    m_scheduledContainerUpdates.remove(container);
 }
 
 Command *ActionManagerPrivate::overridableAction(Id id)
@@ -488,6 +489,23 @@ void ActionManagerPrivate::readUserSettings(Id id, Command *cmd)
     settings->endGroup();
 }
 
+void ActionManagerPrivate::scheduleContainerUpdate(ActionContainerPrivate *actionContainer)
+{
+    const bool needsSchedule = m_scheduledContainerUpdates.isEmpty();
+    m_scheduledContainerUpdates.insert(actionContainer);
+    if (needsSchedule)
+        QMetaObject::invokeMethod(this,
+                                  &ActionManagerPrivate::updateContainer,
+                                  Qt::QueuedConnection);
+}
+
+void ActionManagerPrivate::updateContainer()
+{
+    for (ActionContainerPrivate *c : std::as_const(m_scheduledContainerUpdates))
+        c->update();
+    m_scheduledContainerUpdates.clear();
+}
+
 void ActionManagerPrivate::saveSettings(Command *cmd)
 {
     const Key id = cmd->id().toKey();

src/plugins/coreplugin/actionmanager/actionmanager_p.h

@@ -43,11 +43,14 @@ public:
 
     static void readUserSettings(Utils::Id id, Command *cmd);
 
+    void scheduleContainerUpdate(ActionContainerPrivate *actionContainer);
+    void updateContainer();
     void containerDestroyed(QObject *sender);
 
     IdCmdMap m_idCmdMap;
 
     IdContainerMap m_idContainerMap;
+    QSet<ActionContainerPrivate *> m_scheduledContainerUpdates;
 
     Context m_context;