sw: fix missing cache invalidation when switching between images

It is possible to disable toolbar buttons from UNO API client code by
registering a dispatch provider interceptor and then returning an empty
reference when the UNO command associated with that toolbar button is queried
for a dispatch. Such querying of a dispatch happens when changing context (e.g.
text -> image selection), but not when switching between two images.

A benefit of the current approach is that once a button is disabled this way,
it remains disabled without re-querying the dispatch provider, which helps
performance. A downside is that in case the dispatch provider intercepts the
command based on the current selection (e.g. currently selected image), then
switching to an other image won't re-query the dispatch provider, for at least
two reasons:

- SfxBindings::Register_Impl() is only called when the dispatcher is an
internal one (e.g. not implemented in Java), so there is no listener that would
re-query the state on selection change

- even if we re-query the dispatch provider, the actual toolbar button won't be
updated if the initial dispatch was an empty reference, since updating works by
registering a status listener on the returned dispatch object in
svt::ToolboxController::bindListener()

Fix the problem by explicitly calling contextChanged() on the current frame
when switching between images (but not changing context), similar to how
SvtCommandOptions_Impl::Notify() invalidates registered dispatch objects when
the configuration (on what commands are disabled) changes.

This only helps with images and OLE objects, other object types are kept
unchanged for now.

Change-Id: I7f33dd2804067acf5cb0ca836f6a2a69fa950a8b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138724
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

4 files changed, 199 insertions, 1 deletions

diff --git a/sw/CppunitTest_sw_uibase_uiview.mk b/sw/CppunitTest_sw_uibase_uiview.mk
index 5e73ce9c266f..14fbfd9b4c6f 100644
--- a/sw/CppunitTest_sw_uibase_uiview.mk
+++ b/sw/CppunitTest_sw_uibase_uiview.mk
@@ -63,7 +63,8 @@ $(eval $(call gb_CppunitTest_use_custom_headers,sw_uibase_uiview,\
     officecfg/registry \
 ))
 
-$(eval $(call gb_CppunitTest_use_configuration,sw_uibase_uiview))
+$(eval $(call gb_CppunitTest_use_instdir_configuration,sw_uibase_uiview))
+$(eval $(call gb_CppunitTest_use_common_configuration,sw_uibase_uiview))
 
 $(eval $(call gb_CppunitTest_use_uiconfigs,sw_uibase_uiview, \
     modules/swriter \
diff --git a/sw/inc/view.hxx b/sw/inc/view.hxx
index d4d8ec608d8f..11ea11d2ec57 100644
--- a/sw/inc/view.hxx
+++ b/sw/inc/view.hxx
@@ -212,6 +212,7 @@ class SW_DLLPUBLIC SwView: public SfxViewShell
     std::unique_ptr<SwDrawBase>    m_pDrawActual;
 
     const SwFrameFormat      *m_pLastTableFormat;
+    const SwFrameFormat* m_pLastFlyFormat;
 
     std::unique_ptr<SwFormatClipboard> m_pFormatClipboard; //holds data for format paintbrush
 
diff --git a/sw/qa/uibase/uiview/uiview.cxx b/sw/qa/uibase/uiview/uiview.cxx
index 880485487207..6b608e0ae309 100644
--- a/sw/qa/uibase/uiview/uiview.cxx
+++ b/sw/qa/uibase/uiview/uiview.cxx
@@ -13,6 +13,7 @@
 #include <osl/file.hxx>
 #include <comphelper/propertyvalue.hxx>
 #include <comphelper/scopeguard.hxx>
+#include <vcl/scheduler.hxx>
 
 #include <com/sun/star/frame/XDispatchHelper.hpp>
 #include <com/sun/star/frame/XDispatchProvider.hpp>
@@ -20,11 +21,13 @@
 #include <com/sun/star/frame/XStorable2.hpp>
 #include <com/sun/star/lang/XMultiServiceFactory.hpp>
 #include <com/sun/star/packages/zip/ZipFileAccess.hpp>
+#include <com/sun/star/view/XSelectionSupplier.hpp>
 
 #include <unotxdoc.hxx>
 #include <docsh.hxx>
 #include <wrtsh.hxx>
 #include <swmodule.hxx>
+#include <view.hxx>
 
 constexpr OUStringLiteral DATA_DIRECTORY = u"/sw/qa/uibase/uiview/data/";
 
@@ -144,6 +147,171 @@ CPPUNIT_TEST_FIXTURE(SwUibaseUiviewTest, testKeepRatio)
     assertXPathContent(pXmlDoc, "//config:config-item[@config:name='KeepRatio']", "true");
 }
 
+namespace
+{
+/// Interception implementation that disables .uno:Zoom on Image1, but not on Image2.
+struct ImageInterceptor : public cppu::WeakImplHelper<frame::XDispatchProviderInterceptor>
+{
+    uno::Reference<view::XSelectionSupplier> m_xSelectionSupplier;
+    uno::Reference<frame::XDispatchProvider> m_xMaster;
+    uno::Reference<frame::XDispatchProvider> m_xSlave;
+    int m_nEnabled = 0;
+    int m_nDisabled = 0;
+
+public:
+    ImageInterceptor(const uno::Reference<lang::XComponent>& xComponent);
+
+    // XDispatchProviderInterceptor
+    uno::Reference<frame::XDispatchProvider> SAL_CALL getMasterDispatchProvider() override;
+    uno::Reference<frame::XDispatchProvider> SAL_CALL getSlaveDispatchProvider() override;
+    void SAL_CALL setMasterDispatchProvider(
+        const uno::Reference<frame::XDispatchProvider>& xNewSupplier) override;
+    void SAL_CALL
+    setSlaveDispatchProvider(const uno::Reference<frame::XDispatchProvider>& xNewSupplier) override;
+
+    // XDispatchProvider
+    uno::Reference<frame::XDispatch> SAL_CALL queryDispatch(const util::URL& rURL,
+                                                            const OUString& rTargetFrameName,
+                                                            sal_Int32 SearchFlags) override;
+    uno::Sequence<uno::Reference<frame::XDispatch>> SAL_CALL
+    queryDispatches(const uno::Sequence<frame::DispatchDescriptor>& rRequests) override;
+};
+}
+
+ImageInterceptor::ImageInterceptor(const uno::Reference<lang::XComponent>& xComponent)
+{
+    uno::Reference<frame::XModel2> xModel(xComponent, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xModel.is());
+    m_xSelectionSupplier.set(xModel->getCurrentController(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(m_xSelectionSupplier.is());
+}
+
+uno::Reference<frame::XDispatchProvider> ImageInterceptor::getMasterDispatchProvider()
+{
+    return m_xMaster;
+}
+
+uno::Reference<frame::XDispatchProvider> ImageInterceptor::getSlaveDispatchProvider()
+{
+    return m_xSlave;
+}
+
+void ImageInterceptor::setMasterDispatchProvider(
+    const uno::Reference<frame::XDispatchProvider>& xNewSupplier)
+{
+    m_xMaster = xNewSupplier;
+}
+
+void ImageInterceptor::setSlaveDispatchProvider(
+    const uno::Reference<frame::XDispatchProvider>& xNewSupplier)
+{
+    m_xSlave = xNewSupplier;
+}
+
+uno::Reference<frame::XDispatch> ImageInterceptor::queryDispatch(const util::URL& rURL,
+                                                                 const OUString& rTargetFrameName,
+                                                                 sal_Int32 nSearchFlags)
+{
+    // Disable the UNO command based on the currently selected image, i.e. this can't be cached when
+    // a different image is selected. Originally this was .uno:SetBorderStyle, but let's pick a
+    // command which is active when running cppunit tests:
+    if (rURL.Complete == ".uno:Zoom")
+    {
+        uno::Reference<container::XNamed> xImage;
+        m_xSelectionSupplier->getSelection() >>= xImage;
+        if (xImage.is() && xImage->getName() == "Image1")
+        {
+            ++m_nDisabled;
+            return {};
+        }
+
+        ++m_nEnabled;
+    }
+
+    return m_xSlave->queryDispatch(rURL, rTargetFrameName, nSearchFlags);
+}
+
+uno::Sequence<uno::Reference<frame::XDispatch>>
+ImageInterceptor::queryDispatches(const uno::Sequence<frame::DispatchDescriptor>& /*rRequests*/)
+{
+    return {};
+}
+
+CPPUNIT_TEST_FIXTURE(SwUibaseUiviewTest, testSwitchBetweenImages)
+{
+    // Given a document with 2 images, and an interceptor catching an UNO command that specific to
+    // the current selection:
+    SwDoc* pDoc = createSwDoc();
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    for (int i = 0; i < 2; ++i)
+    {
+        uno::Reference<beans::XPropertySet> xTextGraphic(
+            xMSF->createInstance("com.sun.star.text.TextGraphicObject"), uno::UNO_QUERY);
+        xTextGraphic->setPropertyValue("AnchorType",
+                                       uno::Any(text::TextContentAnchorType_AS_CHARACTER));
+        xTextGraphic->setPropertyValue("Size", uno::Any(awt::Size(5000, 5000)));
+        uno::Reference<text::XTextContent> xTextContent(xTextGraphic, uno::UNO_QUERY);
+        xText->insertTextContent(xCursor, xTextContent, false);
+    }
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    uno::Reference<frame::XModel> xModel(mxComponent, uno::UNO_QUERY);
+    uno::Reference<frame::XDispatchProviderInterception> xRegistration(
+        xModel->getCurrentController()->getFrame(), uno::UNO_QUERY);
+    rtl::Reference pInterceptor(new ImageInterceptor(mxComponent));
+
+    xRegistration->registerDispatchProviderInterceptor(pInterceptor);
+    pInterceptor->m_nEnabled = 0;
+    pInterceptor->m_nDisabled = 0;
+
+    // When selecting the first image:
+    uno::Reference<text::XTextGraphicObjectsSupplier> xGraphicObjectsSupplier(mxComponent,
+                                                                              uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xGraphicObjects(
+        xGraphicObjectsSupplier->getGraphicObjects(), uno::UNO_QUERY);
+    pInterceptor->m_xSelectionSupplier->select(xGraphicObjects->getByIndex(0));
+    Scheduler::ProcessEventsToIdle();
+    SwView* pView = pDoc->GetDocShell()->GetView();
+    pView->StopShellTimer();
+
+    // Then make sure the UNO command is disabled:
+    CPPUNIT_ASSERT_EQUAL(0, pInterceptor->m_nEnabled);
+    CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nDisabled);
+
+    // Given a clean state:
+    pInterceptor->m_nEnabled = 0;
+    pInterceptor->m_nDisabled = 0;
+
+    // When selecting the second image:
+    pInterceptor->m_xSelectionSupplier->select(xGraphicObjects->getByIndex(1));
+    Scheduler::ProcessEventsToIdle();
+    pView->StopShellTimer();
+
+    // Then make sure the UNO command is enabled:
+    CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nEnabled);
+    CPPUNIT_ASSERT_EQUAL(0, pInterceptor->m_nDisabled);
+
+    // Given a clean state:
+    pInterceptor->m_nEnabled = 0;
+    pInterceptor->m_nDisabled = 0;
+
+    // When selecting the first image, again (this time not changing the selection type):
+    pInterceptor->m_xSelectionSupplier->select(xGraphicObjects->getByIndex(0));
+    Scheduler::ProcessEventsToIdle();
+    pView->StopShellTimer();
+
+    // Then make sure the UNO command is disabled:
+    CPPUNIT_ASSERT_EQUAL(0, pInterceptor->m_nEnabled);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected greater or equal than: 1
+    // - Actual  : 0
+    // i.e. selecting the first image didn't result in a disabled UNO command.
+    CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nDisabled);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/uibase/uiview/view.cxx b/sw/source/uibase/uiview/view.cxx
index b99baff1988f..30a57840d9c6 100644
--- a/sw/source/uibase/uiview/view.cxx
+++ b/sw/source/uibase/uiview/view.cxx
@@ -269,6 +269,19 @@ void SwView::SelectShell()
     SelectionType nNewSelectionType = m_pWrtShell->GetSelectionType()
                                 & ~SelectionType::TableCell;
 
+    // Determine if a different fly frame was selected.
+    bool bUpdateFly = false;
+    const SwFrameFormat* pCurFlyFormat = nullptr;
+    if (m_nSelectionType & SelectionType::Ole || m_nSelectionType & SelectionType::Graphic)
+    {
+        pCurFlyFormat = m_pWrtShell->GetFlyFrameFormat();
+    }
+    if (pCurFlyFormat && pCurFlyFormat != m_pLastFlyFormat)
+    {
+        bUpdateFly = true;
+    }
+    m_pLastFlyFormat = pCurFlyFormat;
+
     if ( m_pFormShell && m_pFormShell->IsActiveControl() )
         nNewSelectionType |= SelectionType::FormControl;
 
@@ -279,6 +292,20 @@ void SwView::SelectShell()
              m_nSelectionType & SelectionType::Graphic )
             // For graphs and OLE the verb can be modified of course!
             ImpSetVerb( nNewSelectionType );
+
+        if (bUpdateFly)
+        {
+            SfxViewFrame* pViewFrame = GetViewFrame();
+            if (pViewFrame)
+            {
+                uno::Reference<frame::XFrame> xFrame = pViewFrame->GetFrame().GetFrameInterface();
+                if (xFrame.is())
+                {
+                    // Invalidate cached dispatch objects.
+                    xFrame->contextChanged();
+                }
+            }
+        }
     }
     else
     {
@@ -729,6 +756,7 @@ SwView::SwView( SfxViewFrame *_pFrame, SfxViewShell* pOldSh )
                             GetViewFrame()->GetBindings(),
                             WB_VSCROLL |  WB_EXTRAFIELD | WB_BORDER )),
     m_pLastTableFormat(nullptr),
+    m_pLastFlyFormat(nullptr),
     m_pFormatClipboard(new SwFormatClipboard()),
     m_nSelectionType( SelectionType::All ),
     m_nPageCnt(0),