MenuToolbarController improvements

- Make it self-contained, instead of having its code
spread across ToolBarManager.
- Base it on svt::ToolboxController directly, since it
uses nothing from GenericToolbarController.
- Move the code from dtor to dispose method.
- Call ToolBox::SetItemDown to make the button work on
a second click.

Change-Id: I5b38ef532ba40ef0ac31bc53de5b6be4ccd1a118
Reviewed-on: https://gerrit.libreoffice.org/33982
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Maxim Monastirsky <momonasmon@gmail.com>

4 files changed, 83 insertions, 135 deletions

diff --git a/framework/inc/uielement/generictoolbarcontroller.hxx b/framework/inc/uielement/generictoolbarcontroller.hxx
index 88aa79832c11..c2711f1efc8c 100644
--- a/framework/inc/uielement/generictoolbarcontroller.hxx
+++ b/framework/inc/uielement/generictoolbarcontroller.hxx
@@ -67,26 +67,21 @@ class GenericToolbarController : public svt::ToolboxController
         OUString            m_aEnumCommand;
 };
 
-class MenuToolbarController : public GenericToolbarController
+class MenuToolbarController : public svt::ToolboxController
 {
     css::uno::Reference< css::container::XIndexAccess > m_xMenuDesc;
     VclPtr<PopupMenu>                                   pMenu;
     css::uno::Reference< css::lang::XComponent >        m_xMenuManager;
-    OUString                                            m_aModuleIdentifier;
-    public:
-        MenuToolbarController( const css::uno::Reference< css::uno::XComponentContext >& rxContext,
-                                  const css::uno::Reference< css::frame::XFrame >& rFrame,
-                                  ToolBox* pToolBar,
-                                  sal_uInt16 nID,
-                                  const OUString& aCommand,
-                                  const OUString& aModuleIdentifier,
-                                  const css::uno::Reference< css::container::XIndexAccess >& xMenuDesc );
 
-    virtual ~MenuToolbarController() override;
+public:
+    // XStatusListener
+    virtual void SAL_CALL statusChanged( const css::frame::FeatureStateEvent& ) override {}
+    // XComponent
+    virtual void SAL_CALL dispose() override;
+    // XInitialization
+    virtual void SAL_CALL initialize( const css::uno::Sequence< css::uno::Any >& rArgs ) override;
     // XToolbarController
-    virtual void SAL_CALL click() override;
     virtual css::uno::Reference< css::awt::XWindow > SAL_CALL createPopupWindow() override;
-
 };
 
 }
diff --git a/framework/inc/uielement/toolbarmanager.hxx b/framework/inc/uielement/toolbarmanager.hxx
index 1ecbdfa293d2..739d22f5647a 100644
--- a/framework/inc/uielement/toolbarmanager.hxx
+++ b/framework/inc/uielement/toolbarmanager.hxx
@@ -37,7 +37,6 @@
 #include <com/sun/star/frame/XToolbarController.hpp>
 #include <com/sun/star/lang/XComponent.hpp>
 #include <com/sun/star/ui/XImageManager.hpp>
-#include <com/sun/star/ui/XUIConfigurationManager.hpp>
 #include <com/sun/star/ui/ItemStyle.hpp>
 #include <com/sun/star/ui/XAcceleratorConfiguration.hpp>
 #include <com/sun/star/uno/XComponentContext.hpp>
@@ -153,8 +152,6 @@ class ToolBarManager : public ToolbarManager_Base
         typedef ::std::vector< css::uno::Reference< css::frame::XSubToolbarController > >             SubToolBarControllerVector;
         typedef std::unordered_map<OUString, SubToolBarControllerVector, OUStringHash>                                                SubToolBarToSubToolBarControllerMap;
 
-        typedef std::unordered_map< sal_uInt16, css::uno::Reference< css::container::XIndexAccess > > MenuDescriptionMap;
-
         bool m_bDisposed : 1,
              m_bAddedToTaskPaneList : 1,
              m_bFrameActionRegistered : 1,
@@ -176,14 +173,11 @@ class ToolBarManager : public ToolbarManager_Base
         css::uno::Reference< css::frame::XUIControllerFactory >      m_xToolbarControllerFactory;
         css::uno::Reference< css::ui::XImageManager >                m_xModuleImageManager;
         css::uno::Reference< css::ui::XImageManager >                m_xDocImageManager;
-        css::uno::Reference< css::ui::XUIConfigurationManager >      m_xUICfgMgr;
-        css::uno::Reference< css::ui::XUIConfigurationManager >      m_xDocUICfgMgr;
 
         CommandToInfoMap                                             m_aCommandMap;
         SubToolBarToSubToolBarControllerMap                          m_aSubToolBarControllerMap;
         Timer                                                        m_aAsyncUpdateControllersTimer;
         OUString                                                     m_sIconTheme;
-        MenuDescriptionMap                                           m_aMenuMap;
 };
 
 }
diff --git a/framework/source/uielement/generictoolbarcontroller.cxx b/framework/source/uielement/generictoolbarcontroller.cxx
index 56d24bf95724..2d8205f6fdcd 100644
--- a/framework/source/uielement/generictoolbarcontroller.cxx
+++ b/framework/source/uielement/generictoolbarcontroller.cxx
@@ -27,6 +27,8 @@
 #include <com/sun/star/frame/status/ItemStatus.hpp>
 #include <com/sun/star/frame/status/ItemState.hpp>
 #include <com/sun/star/frame/status/Visibility.hpp>
+#include <com/sun/star/ui/XUIConfigurationManagerSupplier.hpp>
+#include <com/sun/star/ui/theModuleUIConfigurationManagerSupplier.hpp>
 
 #include <comphelper/processfactory.hxx>
 #include <svtools/toolboxcontroller.hxx>
@@ -278,21 +280,7 @@ IMPL_STATIC_LINK( GenericToolbarController, ExecuteHdl_Impl, void*, p, void )
    delete pExecuteInfo;
 }
 
-MenuToolbarController::MenuToolbarController( const Reference< XComponentContext >& rxContext,
-                                              const Reference< XFrame >& rFrame,
-                                              ToolBox* pToolBar,
-                                              sal_uInt16   nID,
-                                              const OUString& aCommand,
-                                              const OUString& aModuleIdentifier,
-                                              const Reference< XIndexAccess >& xMenuDesc )
-    : GenericToolbarController( rxContext, rFrame, pToolBar, nID, aCommand ),
-      m_xMenuDesc( xMenuDesc ),
-      pMenu( nullptr ),
-      m_aModuleIdentifier( aModuleIdentifier )
-{
-}
-
-MenuToolbarController::~MenuToolbarController()
+void MenuToolbarController::dispose()
 {
     try
     {
@@ -300,39 +288,60 @@ MenuToolbarController::~MenuToolbarController()
             m_xMenuManager->dispose();
     }
     catch( const Exception& ) {}
-    if ( pMenu )
-    {
-        pMenu.disposeAndClear();
-    }
+
+    m_xMenuManager.clear();
+    m_xMenuDesc.clear();
+    pMenu.disposeAndClear();
 }
 
-class Toolbarmenu : public ::PopupMenu
+void MenuToolbarController::initialize( const css::uno::Sequence< css::uno::Any >& rArgs )
 {
-    public:
-    Toolbarmenu();
-    virtual ~Toolbarmenu() override;
-    virtual void dispose() override;
-};
+    ToolboxController::initialize( rArgs );
 
-Toolbarmenu::Toolbarmenu()
-{
-    SAL_INFO("fwk.uielement", "constructing Toolbarmenu " << this);
-}
+    css::uno::Reference< css::container::XIndexAccess > xMenuContainer;
+    try
+    {
+        css::uno::Reference< css::frame::XController > xController( m_xFrame->getController() );
+        css::uno::Reference< css::ui::XUIConfigurationManagerSupplier > xSupplier( xController->getModel(), css::uno::UNO_QUERY_THROW );
+        css::uno::Reference< css::ui::XUIConfigurationManager > xConfigManager( xSupplier->getUIConfigurationManager() );
+        xMenuContainer.set( xConfigManager->getSettings( m_aCommandURL, false ) );
+    }
+    catch( const css::uno::Exception& )
+    {}
 
-Toolbarmenu::~Toolbarmenu()
-{
-    disposeOnce();
-}
+    if ( !xMenuContainer.is() )
+    {
+        try
+        {
+            css::uno::Reference< css::ui::XModuleUIConfigurationManagerSupplier > xSupplier(
+                css::ui::theModuleUIConfigurationManagerSupplier::get( m_xContext ) );
+            css::uno::Reference< css::ui::XUIConfigurationManager > xConfigManager(
+                xSupplier->getUIConfigurationManager( m_sModuleName ) );
+            xMenuContainer.set( xConfigManager->getSettings( m_aCommandURL, false ) );
+        }
+        catch( const css::uno::Exception& )
+        {}
+    }
 
-void Toolbarmenu::dispose()
-{
-    SAL_INFO("fwk.uielement", "destructing Toolbarmenu " << this);
-    ::PopupMenu::dispose();
-}
+    if ( xMenuContainer.is() && xMenuContainer->getCount() )
+    {
+        Sequence< PropertyValue > aProps;
+        // drop down menu info is currently the first ( and only ) menu in the menusettings container
+        xMenuContainer->getByIndex(0) >>= aProps;
+        for ( const auto& aProp : aProps )
+        {
+            if ( aProp.Name == "ItemDescriptorContainer" )
+            {
+                aProp.Value >>= m_xMenuDesc;
+                break;
+            }
+        }
 
-void SAL_CALL MenuToolbarController::click()
-{
-    createPopupWindow();
+        ToolBox* pToolBox = nullptr;
+        sal_uInt16 nId = 0;
+        if ( getToolboxId( nId, &pToolBox ) )
+            pToolBox->SetItemBits( nId, pToolBox->GetItemBits( nId ) | ToolBoxItemBits::DROPDOWNONLY );
+    }
 }
 
 Reference< XWindow > SAL_CALL
@@ -340,24 +349,21 @@ MenuToolbarController::createPopupWindow()
 {
     if ( !pMenu )
     {
-        Reference< XDispatchProvider > xDispatch;
-        Reference< XURLTransformer > xURLTransformer = URLTransformer::create( m_xContext );
-        pMenu = VclPtr<Toolbarmenu>::Create();
-        m_xMenuManager.set( new MenuBarManager( m_xContext, m_xFrame, xURLTransformer, xDispatch, m_aModuleIdentifier, pMenu, false, false ) );
-        if (m_xMenuManager.is())
-        {
-            MenuBarManager& rMgr = dynamic_cast<MenuBarManager&>(*m_xMenuManager.get());
-            rMgr.SetItemContainer(m_xMenuDesc);
-        }
+        pMenu = VclPtr<PopupMenu>::Create();
+        css::uno::Reference< css::frame::XDispatchProvider > xDispatchProvider( m_xFrame, css::uno::UNO_QUERY );
+        sal_uInt16 m_nMenuId = 1;
+        MenuBarManager::FillMenu( m_nMenuId, pMenu, m_sModuleName, m_xMenuDesc, xDispatchProvider );
+        m_xMenuManager.set( new MenuBarManager( m_xContext, m_xFrame, m_xUrlTransformer, xDispatchProvider, m_sModuleName, pMenu, false, false ) );
     }
 
-    if ( !pMenu || !m_pToolbar )
+    ToolBox* pToolBox = nullptr;
+    sal_uInt16 nId = 0;
+    if ( !getToolboxId( nId, &pToolBox ) )
         return nullptr;
 
-    OSL_ENSURE ( pMenu->GetItemCount(), "Empty PopupMenu!" );
-
-    ::Rectangle aRect( m_pToolbar->GetItemRect( m_nID ) );
-    pMenu->Execute( m_pToolbar, aRect, PopupMenuFlags::ExecuteDown );
+    pToolBox->SetItemDown( m_nToolBoxId, true );
+    pMenu->Execute( pToolBox, pToolBox->GetItemRect( nId ), PopupMenuFlags::ExecuteDown );
+    pToolBox->SetItemDown( m_nToolBoxId, false );
 
     return nullptr;
 }
diff --git a/framework/source/uielement/toolbarmanager.cxx b/framework/source/uielement/toolbarmanager.cxx
index b6ce88cf2249..7331d246018b 100644
--- a/framework/source/uielement/toolbarmanager.cxx
+++ b/framework/source/uielement/toolbarmanager.cxx
@@ -750,22 +750,19 @@ void ToolBarManager::CreateControllers()
 
                     xController = xStatusListener;
                 }
+                else if ( aCommandURL.startsWith( "private:resource/menubar/" ) )
+                {
+                    xController.set( new MenuToolbarController );
+                }
                 else
                 {
-                    MenuDescriptionMap::iterator it = m_aMenuMap.find( nId );
-                    if ( it == m_aMenuMap.end() )
-                    {
-                        xController.set(
-                            new GenericToolbarController( m_xContext, m_xFrame, m_pToolBar, nId, aCommandURL ));
+                    xController.set(
+                        new GenericToolbarController( m_xContext, m_xFrame, m_pToolBar, nId, aCommandURL ));
 
-                        // Accessibility support: Set toggle button role for specific commands
-                        sal_Int32 nProps = vcl::CommandInfoProvider::Instance().GetPropertiesForCommand(aCommandURL, m_xFrame);
-                        if ( nProps & UICOMMANDDESCRIPTION_PROPERTIES_TOGGLEBUTTON )
-                            m_pToolBar->SetItemBits( nId, m_pToolBar->GetItemBits( nId ) | ToolBoxItemBits::CHECKABLE );
-                    }
-                    else
-                        xController.set(
-                            new MenuToolbarController( m_xContext, m_xFrame, m_pToolBar, nId, aCommandURL, m_aModuleIdentifier, m_aMenuMap[ nId ] ));
+                    // Accessibility support: Set toggle button role for specific commands
+                    sal_Int32 nProps = vcl::CommandInfoProvider::Instance().GetPropertiesForCommand(aCommandURL, m_xFrame);
+                    if ( nProps & UICOMMANDDESCRIPTION_PROPERTIES_TOGGLEBUTTON )
+                        m_pToolBar->SetItemBits( nId, m_pToolBar->GetItemBits( nId ) | ToolBoxItemBits::CHECKABLE );
                 }
             }
             else if ( pController )
@@ -943,8 +940,8 @@ void ToolBarManager::FillToolbar( const Reference< XIndexAccess >& rItemContaine
             Reference< XUIConfigurationManagerSupplier > xSupplier( xModel, UNO_QUERY );
             if ( xSupplier.is() )
             {
-                m_xDocUICfgMgr.set( xSupplier->getUIConfigurationManager(), UNO_QUERY );
-                m_xDocImageManager.set( m_xDocUICfgMgr->getImageManager(), UNO_QUERY );
+                Reference< XUIConfigurationManager > xDocUICfgMgr( xSupplier->getUIConfigurationManager(), UNO_QUERY );
+                m_xDocImageManager.set( xDocUICfgMgr->getImageManager(), UNO_QUERY );
                 m_xDocImageManager->addConfigurationListener(
                                         Reference< XUIConfigurationListener >(
                                             static_cast< OWeakObject* >( this ), UNO_QUERY ));
@@ -964,8 +961,8 @@ void ToolBarManager::FillToolbar( const Reference< XIndexAccess >& rItemContaine
     {
         Reference< XModuleUIConfigurationManagerSupplier > xModuleCfgMgrSupplier =
             theModuleUIConfigurationManagerSupplier::get( m_xContext );
-        m_xUICfgMgr = xModuleCfgMgrSupplier->getUIConfigurationManager( m_aModuleIdentifier );
-        m_xModuleImageManager.set( m_xUICfgMgr->getImageManager(), UNO_QUERY );
+        Reference< XUIConfigurationManager > xUICfgMgr = xModuleCfgMgrSupplier->getUIConfigurationManager( m_aModuleIdentifier );
+        m_xModuleImageManager.set( xUICfgMgr->getImageManager(), UNO_QUERY );
         m_xModuleImageManager->addConfigurationListener( Reference< XUIConfigurationListener >(
                                                             static_cast< OWeakObject* >( this ), UNO_QUERY ));
     }
@@ -977,8 +974,6 @@ void ToolBarManager::FillToolbar( const Reference< XIndexAccess >& rItemContaine
     m_aControllerMap.clear();
     m_aCommandMap.clear();
 
-    m_aMenuMap.clear();
-
     CommandInfo aCmdInfo;
     for ( sal_Int32 n = 0; n < rItemContainer->getCount(); n++ )
     {
@@ -989,7 +984,6 @@ void ToolBarManager::FillToolbar( const Reference< XIndexAccess >& rItemContaine
         sal_uInt16                  nType( css::ui::ItemType::DEFAULT );
         sal_uInt32                  nStyle( 0 );
 
-        Reference< XIndexAccess >   aMenuDesc;
         try
         {
             if ( rItemContainer->getByIndex( n ) >>= aProp )
@@ -998,43 +992,7 @@ void ToolBarManager::FillToolbar( const Reference< XIndexAccess >& rItemContaine
                 for ( int i = 0; i < aProp.getLength(); i++ )
                 {
                     if ( aProp[i].Name == ITEM_DESCRIPTOR_COMMANDURL )
-                    {
                         aProp[i].Value >>= aCommandURL;
-                        if ( aCommandURL.startsWith("private:resource/menubar/") )
-                        {
-                            try
-                            {
-                                Reference< XIndexAccess > xMenuContainer;
-                                if ( m_xDocUICfgMgr.is() &&
-                                     m_xDocUICfgMgr->hasSettings( aCommandURL ) )
-                                    xMenuContainer  = m_xDocUICfgMgr->getSettings( aCommandURL, false );
-                                if ( !xMenuContainer.is() &&
-                                     m_xUICfgMgr.is() &&
-                                     m_xUICfgMgr->hasSettings( aCommandURL ) )
-                                    xMenuContainer = m_xUICfgMgr->getSettings( aCommandURL, false );
-                                if ( xMenuContainer.is() && xMenuContainer->getCount() )
-                                {
-                                    Sequence< PropertyValue > aProps;
-                                    // drop down menu info is currently
-                                    // the first ( and only ) menu
-                                    // in the menusettings container
-                                    xMenuContainer->getByIndex(0) >>= aProps;
-                                    for ( sal_Int32 index=0; index<aProps.getLength(); ++index )
-                                    {
-                                        if ( aProps[ index ].Name == "ItemDescriptorContainer" )
-
-                                        {
-                                            aProps[ index ].Value >>= aMenuDesc;
-                                            break;
-                                        }
-                                    }
-                                }
-                            }
-                            catch (const Exception&)
-                            {
-                            }
-                        }
-                    }
                     else if ( aProp[i].Name == "Label" )
                         aProp[i].Value >>= aLabel;
                     else if ( aProp[i].Name == "Tooltip" )
@@ -1058,11 +1016,6 @@ void ToolBarManager::FillToolbar( const Reference< XIndexAccess >& rItemContaine
                     OUString aString(vcl::CommandInfoProvider::Instance().GetLabelForCommand(aCommandURL, m_xFrame));
 
                     ToolBoxItemBits nItemBits = ConvertStyleToToolboxItemBits( nStyle );
-                    if ( aMenuDesc.is() )
-                    {
-                        m_aMenuMap[ nId ] = aMenuDesc;
-                        nItemBits |= ToolBoxItemBits::DROPDOWNONLY;
-                    }
                     m_pToolBar->InsertItem( nId, aString, nItemBits );
                     m_pToolBar->SetItemCommand( nId, aCommandURL );
                     if ( !aTooltip.isEmpty() )