tdf#130187: Don't crash on exiting print preview with Notebookbar

Crash caused by this sequence (tested in Writer):

1. Closing print preview, frame is attached to controller;
2. This calls SfxNotebookBar::StateMethod
3. There notebookbar's listener is added to list of the controller's
   context change event listeners
4. Then in SwPagePreview::~SwPagePreview, notebookbar's listener is
   added to that list again
5. ContextChangeEventMultiplexer::addContextChangeEventListener
   detects second addition, and throws an unhandled exception.

I don't know why starting listening is needed in SwPagePreview dtor;
unfortunately commit d05b7b32d9ecb6fcb4a268eb68cdcee09bafa6dd doesn't
say much about context and reasons.

ControlListener is renamed to ControlListenerForCurrentController to
emphasize that it operates on the current controller of notebookbar's
frame; and its bListen parameter meaning was reverted: previously its
"true" value awkwardly meant "stop listening". All direct operations
with listener of notebookbar are replaced with calls to notebookbar's
methods.

In ContextChangeEventMultiplexer::addContextChangeEventListener,
uno::UNO_QUERY_THROW was replaced with uno::UNO_QUERY, because not
only chart controller may appear here, and it's not an error: e.g.
SfxBaseController doesn't implement lang::XServiceInfo.

Regression after commit d05b7b32d9ecb6fcb4a268eb68cdcee09bafa6dd.

Change-Id: Ief1aed188d8f02a6cfe3ea25f4d082dfdf449f32
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86257
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87582
(cherry picked from commit fb2382ad4f33b885650ad5156ccaec4e90661806)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/88229
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice@googlemail.com>
Tested-by: Christian Lohmaier <lohmaier+LibreOffice@googlemail.com>

5 files changed, 36 insertions, 26 deletions

diff --git a/include/vcl/notebookbar.hxx b/include/vcl/notebookbar.hxx
index af904ca2ae5f..457a64b9a206 100644
--- a/include/vcl/notebookbar.hxx
+++ b/include/vcl/notebookbar.hxx
@@ -14,6 +14,7 @@
 #include <vcl/ctrl.hxx>
 #include <vcl/NotebookBarAddonsMerger.hxx>
 #include <vcl/settings.hxx>
+#include <set>
 #include <vector>
 
 namespace com { namespace sun { namespace star { namespace ui { class XContextChangeEventListener; } } } }
@@ -39,17 +40,17 @@ public:
 
     void SetSystemWindow(SystemWindow* pSystemWindow);
 
-    const css::uno::Reference<css::ui::XContextChangeEventListener>& getContextChangeEventListener() const { return m_pEventListener; }
-
     void StateChanged(const StateChangedType nStateChange ) override;
 
     void DataChanged(const DataChangedEvent& rDCEvt) override;
 
-    void ControlListener(bool bListen);
+    void ControlListenerForCurrentController(bool bListen);
+    void StopListeningAllControllers();
 
 private:
     VclPtr<SystemWindow> m_pSystemWindow;
     css::uno::Reference<css::ui::XContextChangeEventListener> m_pEventListener;
+    std::set<css::uno::Reference<css::frame::XController>> m_alisteningControllers;
     std::vector<NotebookbarContextControl*> m_pContextContainers;
     css::uno::Reference<css::frame::XFrame> mxFrame;
 
diff --git a/sc/source/ui/view/prevwsh.cxx b/sc/source/ui/view/prevwsh.cxx
index 493ce87228ce..fe3688abdd43 100644
--- a/sc/source/ui/view/prevwsh.cxx
+++ b/sc/source/ui/view/prevwsh.cxx
@@ -160,7 +160,7 @@ ScPreviewShell::ScPreviewShell( SfxViewFrame* pViewFrame,
 
     auto& pNotebookBar = pViewFrame->GetWindow().GetSystemWindow()->GetNotebookBar();
     if (pNotebookBar)
-        pNotebookBar->ControlListener(true);
+        pNotebookBar->ControlListenerForCurrentController(false); // stop listening
 
     if ( auto pTabViewShell = dynamic_cast<ScTabViewShell*>( pOldSh) )
     {
@@ -189,7 +189,7 @@ ScPreviewShell::~ScPreviewShell()
         mpFrameWindow->SetCloseHdl(Link<SystemWindow&,void>()); // Remove close handler.
 
     if (auto& pBar = GetViewFrame()->GetWindow().GetSystemWindow()->GetNotebookBar())
-        pBar->ControlListener(false);
+        pBar->ControlListenerForCurrentController(true); // let it start listening now
 
     // #108333#; notify Accessibility that Shell is dying and before destroy all
     BroadcastAccessibility( SfxHint( SfxHintId::Dying ) );
diff --git a/sfx2/source/notebookbar/SfxNotebookBar.cxx b/sfx2/source/notebookbar/SfxNotebookBar.cxx
index 538886f202a1..a2d7f1f39b17 100644
--- a/sfx2/source/notebookbar/SfxNotebookBar.cxx
+++ b/sfx2/source/notebookbar/SfxNotebookBar.cxx
@@ -400,15 +400,7 @@ bool SfxNotebookBar::StateMethod(SystemWindow* pSysWindow,
 
             if(pView)
             {
-                Reference<XContextChangeEventMultiplexer> xMultiplexer
-                            = ContextChangeEventMultiplexer::get( xContext );
-
-                if(xFrame.is())
-                {
-                    xMultiplexer->addContextChangeEventListener(
-                                        pNotebookBar->getContextChangeEventListener(),
-                                        xFrame->getController());
-                }
+                pNotebookBar->ControlListenerForCurrentController(true);
             }
         }
 
@@ -430,10 +422,9 @@ void SfxNotebookBar::RemoveListeners(SystemWindow const * pSysWindow)
                         = ContextChangeEventMultiplexer::get(
                                 ::comphelper::getProcessComponentContext());
 
-    if (pSysWindow->GetNotebookBar())
+    if (auto pNotebookBar = pSysWindow->GetNotebookBar())
     {
-        xMultiplexer->removeAllContextChangeEventListeners(
-                           pSysWindow->GetNotebookBar()->getContextChangeEventListener());
+        pNotebookBar->StopListeningAllControllers();
     }
 }
 
diff --git a/sw/source/uibase/uiview/pview.cxx b/sw/source/uibase/uiview/pview.cxx
index e40ee2807de4..3e93705806f3 100644
--- a/sw/source/uibase/uiview/pview.cxx
+++ b/sw/source/uibase/uiview/pview.cxx
@@ -1160,7 +1160,7 @@ SwPagePreview::SwPagePreview(SfxViewFrame *pViewFrame, SfxViewShell* pOldSh):
     SfxShell::BroadcastContextForActivation(true);
     //removelisteners for notebookbar
     if (auto& pBar = SfxViewFrame::Current()->GetWindow().GetSystemWindow()->GetNotebookBar())
-        pBar->ControlListener(true);
+        pBar->ControlListenerForCurrentController(false);
 
     SfxObjectShell* pObjShell = pViewFrame->GetObjectShell();
     if ( !pOldSh )
@@ -1228,7 +1228,7 @@ SwPagePreview::~SwPagePreview()
     m_pViewWin.disposeAndClear();
     if (SfxViewFrame* pCurrent = SfxViewFrame::Current())
         if (auto& pBar = pCurrent->GetWindow().GetSystemWindow()->GetNotebookBar())
-            pBar->ControlListener(false);
+            pBar->ControlListenerForCurrentController(true); // start listening now
     m_pScrollFill.disposeAndClear();
     m_pHScrollbar.disposeAndClear();
     m_pVScrollbar.disposeAndClear();
diff --git a/vcl/source/control/notebookbar.cxx b/vcl/source/control/notebookbar.cxx
index b178962f5d11..bcf2f7563ff5 100644
--- a/vcl/source/control/notebookbar.cxx
+++ b/vcl/source/control/notebookbar.cxx
@@ -97,6 +97,7 @@ void NotebookBar::dispose()
         m_pSystemWindow->GetTaskPaneList()->RemoveWindow(this);
     m_pSystemWindow.clear();
     disposeBuilder();
+    assert(m_alisteningControllers.empty());
     m_pEventListener.clear();
     Control::dispose();
 }
@@ -180,24 +181,41 @@ void SAL_CALL NotebookBarContextChangeEventListener::notifyContextChangeEvent(co
     }
 }
 
-void NotebookBar::ControlListener(bool bListen)
+void NotebookBar::ControlListenerForCurrentController(bool bListen)
 {
+    auto xController = mxFrame->getController();
     if(bListen)
     {
-        // remove listeners
-        css::uno::Reference<css::ui::XContextChangeEventMultiplexer> xMultiplexer (css::ui::ContextChangeEventMultiplexer::get(
+        // add listeners
+        if (m_alisteningControllers.count(xController) == 0)
+        {
+            auto xMultiplexer(css::ui::ContextChangeEventMultiplexer::get(
                 ::comphelper::getProcessComponentContext()));
-        xMultiplexer->removeContextChangeEventListener(getContextChangeEventListener(),mxFrame->getController());
+            xMultiplexer->addContextChangeEventListener(m_pEventListener, xController);
+            m_alisteningControllers.insert(xController);
+        }
     }
     else
     {
-        // add listeners
-        css::uno::Reference<css::ui::XContextChangeEventMultiplexer> xMultiplexer (css::ui::ContextChangeEventMultiplexer::get(
+        // remove listeners
+        if (m_alisteningControllers.count(xController))
+        {
+            auto xMultiplexer(css::ui::ContextChangeEventMultiplexer::get(
                 ::comphelper::getProcessComponentContext()));
-        xMultiplexer->addContextChangeEventListener(getContextChangeEventListener(),mxFrame->getController());
+            xMultiplexer->removeContextChangeEventListener(m_pEventListener, xController);
+            m_alisteningControllers.erase(xController);
+        }
     }
 }
 
+void NotebookBar::StopListeningAllControllers()
+{
+    auto xMultiplexer(
+        css::ui::ContextChangeEventMultiplexer::get(comphelper::getProcessComponentContext()));
+    xMultiplexer->removeAllContextChangeEventListeners(m_pEventListener);
+    m_alisteningControllers.clear();
+}
+
 void SAL_CALL NotebookBarContextChangeEventListener::disposing(const ::css::lang::EventObject&)
 {
     mpParent.clear();