comphelper/source/misc/lok.cxx | 33 ++++++++++
desktop/source/lib/init.cxx | 1
include/comphelper/lok.hxx | 7 ++
include/sfx2/lokhelper.hxx | 2
sfx2/source/doc/guisaveas.cxx | 9 ++
sfx2/source/view/lokhelper.cxx | 10 +++
sw/qa/extras/tiledrendering/data/to-pdf.odt |binary
sw/qa/extras/tiledrendering/tiledrendering2.cxx | 39 ++++++++++++
sw/qa/extras/tiledrendering/tiledrenderingmodeltestbase.cxx | 6 +
vcl/source/app/svapp.cxx | 17 ++++-
10 files changed, 121 insertions(+), 3 deletions(-)
New commits:
commit ff46d82c5814e2bee68f4e1ddaa75c661dc7e421
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Jan 2 16:18:03 2025 +0100
Commit: Andras Timar <andras.ti...@collabora.com>
CommitDate: Mon Feb 10 16:00:18 2025 +0100
cool#10782 vcl lok: avoid changing the current view during Yield
Commit 520cc546e2940bdfbc45eab1e569bb06bab17a9c (cool#10782 sfx2 lok:
fix bad view id on PDF export, 2024-12-20) provided a specific fix for
one case where spinning the main loop can result in callbacks being
emitted on the wrong view.
Hunting down all the SfxViewShell::Current() calls and auditing them if
they are problematic in practice is a large task, so it's desirable to
handle this problem in a more generic way.
Fix the problem by pushing/popping the current LOK view in
Application::Reschedule(), which is what e.g. the framework/ status bar
code uses during PDF export. This keeps the original problem fixed but
is more generic.
This requires storing function pointers in comphelper/, since normally
vcl/ code can't call SfxLokHelper, since sfx2/ already depends on vcl/.
The fix can be tested by reverting the original fix:
git show 520cc546e2940bdfbc45eab1e569bb06bab17a9c -- sfx2|git apply -R
and then running the testcase:
make -C sw -sr CppunitTest_sw_tiledrendering
CPPUNIT_TEST_NAME=testPDFExportViewSwitch
which now passes even if the sfx2-level fix is not there. Keep that
specific fix, though: the less time we assume SfxViewShell::Current()
returns the correct view, the better.
Change-Id: Ic1811ff1e67f73fa5066af775d31589136b8502a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/179623
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
diff --git a/comphelper/source/misc/lok.cxx b/comphelper/source/misc/lok.cxx
index 88521ebb455e..cd1333711e4d 100644
--- a/comphelper/source/misc/lok.cxx
+++ b/comphelper/source/misc/lok.cxx
@@ -40,6 +40,9 @@ static Compat g_eCompatFlags(Compat::none);
static std::function<bool(void*)> g_pAnyInputCallback;
static void* g_pAnyInputCallbackData;
+static std::function<void(int)> g_pViewSetter;
+static std::function<int()> g_pViewGetter;
+
namespace
{
@@ -337,6 +340,36 @@ bool anyInput()
return bRet;
}
+void setViewSetter(std::function<void(int)> pViewSetter)
+{
+ g_pViewSetter = pViewSetter;
+}
+
+void setView(int nView)
+{
+ if (!g_pViewSetter)
+ {
+ return;
+ }
+
+ g_pViewSetter(nView);
+}
+
+void setViewGetter(std::function<int()> pViewGetter)
+{
+ g_pViewGetter = pViewGetter;
+}
+
+int getView()
+{
+ if (!g_pViewGetter)
+ {
+ return -1;
+ }
+
+ return g_pViewGetter();
+}
+
} // namespace
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index 06729ea3c92f..1b729d019273 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -7785,6 +7785,7 @@ static void lo_runLoop(LibreOfficeKit* /*pThis*/,
SolarMutexGuard aGuard;
vcl::lok::registerPollCallbacks(pPollCallback, pWakeCallback, pData);
+ SfxLokHelper::registerViewCallbacks();
Application::UpdateMainThread();
soffice_main();
}
diff --git a/include/comphelper/lok.hxx b/include/comphelper/lok.hxx
index d1aa6804f534..0565fa78d6da 100644
--- a/include/comphelper/lok.hxx
+++ b/include/comphelper/lok.hxx
@@ -137,6 +137,13 @@ COMPHELPER_DLLPUBLIC void setBlockedCommandList(const
char* blockedCommandList);
COMPHELPER_DLLPUBLIC void setAnyInputCallback(const
std::function<bool(void*)>& pAnyInputCallback,
void* pData);
COMPHELPER_DLLPUBLIC bool anyInput();
+
+// These allow setting callbacks, so that set/get of a LOK view is possible
even in code that is
+// below sfx2.
+COMPHELPER_DLLPUBLIC void setViewSetter(std::function<void(int)> pViewSetter);
+COMPHELPER_DLLPUBLIC void setView(int nView);
+COMPHELPER_DLLPUBLIC void setViewGetter(std::function<int()> pViewGetter);
+COMPHELPER_DLLPUBLIC int getView();
}
#endif // INCLUDED_COMPHELPER_LOK_HXX
diff --git a/include/sfx2/lokhelper.hxx b/include/sfx2/lokhelper.hxx
index ec73f7c597f0..2f87faf82e78 100644
--- a/include/sfx2/lokhelper.hxx
+++ b/include/sfx2/lokhelper.hxx
@@ -255,6 +255,8 @@ public:
static void getCommandValues(tools::JsonWriter& rJsonWriter,
std::string_view rCommand);
/// Parses key-value parameters of rCommand.
static std::map<OUString, OUString>
parseCommandParameters(std::u16string_view rCommand);
+ /// Registers function pointers in comphelper/ to set/get of the current
LOK view.
+ static void registerViewCallbacks();
private:
static int createView(SfxViewFrame& rViewFrame, ViewShellDocId docId);
diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx
index 5eec80fba576..0ea04c16238f 100644
--- a/sfx2/source/view/lokhelper.cxx
+++ b/sfx2/source/view/lokhelper.cxx
@@ -1105,6 +1105,16 @@ void
SfxLokHelper::notifyOtherViewsUpdatePerViewId(SfxViewShell const* pThisView
}
}
+void SfxLokHelper::registerViewCallbacks()
+{
+ comphelper::LibreOfficeKit::setViewSetter([](int nView) {
+ SfxLokHelper::setView(nView);
+ });
+ comphelper::LibreOfficeKit::setViewGetter([]() -> int {
+ return SfxLokHelper::getView();
+ });
+}
+
namespace
{
struct LOKAsyncEventData
diff --git a/sw/qa/extras/tiledrendering/tiledrendering2.cxx
b/sw/qa/extras/tiledrendering/tiledrendering2.cxx
index 0f9591d5afc3..753016cf8d58 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering2.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering2.cxx
@@ -283,6 +283,7 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest,
testPDFExportViewSwitch)
{
// Given a document with 2 views:
SwXTextDocument* pXTextDocument = createDoc("to-pdf.odt");
+ SfxLokHelper::registerViewCallbacks();
SwDoc* pDoc = pXTextDocument->GetDocShell()->GetDoc();
ViewCallback aView1;
SfxLokHelper::createView();
diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx
index fdfed6c05e5d..7e66af9cb7e8 100644
--- a/vcl/source/app/svapp.cxx
+++ b/vcl/source/app/svapp.cxx
@@ -401,7 +401,22 @@ bool Application::Reschedule( bool i_bAllEvents )
SAL_WARN("vcl.schedule", "Application::Reschedule(" << i_bAllEvents <<
")");
return false;
}
- return ImplYield(false, i_bAllEvents);
+ int nOldView = -1;
+ if (comphelper::LibreOfficeKit::isActive())
+ {
+ nOldView = comphelper::LibreOfficeKit::getView();
+ }
+ bool bRet = ImplYield(false, i_bAllEvents);
+ if (comphelper::LibreOfficeKit::isActive())
+ {
+ int nNewView = comphelper::LibreOfficeKit::getView();
+ if (nOldView != -1 && nNewView != -1 && nOldView != nNewView)
+ {
+ // Yield changed the current view, restore the old value.
+ comphelper::LibreOfficeKit::setView(nOldView);
+ }
+ }
+ return bRet;
}
bool Application::IsOnSystemEventLoop()
commit e16dd22337fd22520303c6f88f08681108b436f5
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Dec 20 12:56:15 2024 +0100
Commit: Andras Timar <andras.ti...@collabora.com>
CommitDate: Mon Feb 10 15:50:02 2025 +0100
cool#10782 sfx2 lok: fix bad view id on PDF export
Have 2 views, view 2 dispatches .uno:ExportDirectToPDF, and sometimes
view 1 gets the LOK_CALLBACK_EXPORT_FILE callback, which is incorrect.
What happens is that the command gets dispatched correctly, but during
save the progressbar gets updated in
framework::StatusIndicatorFactory::end(), which calls
Application::Reschedule(), which processes LOK jobs on the main loop,
which may switch back to view 1, so the callback is emitted on that
view.
Fix the problem by reducing the duration where we work with the "current
view". We know that initially the command dispatch has the correct
current view since commit ee7ca8e4ea8ed93655f99e77a9e77032ac830c46
(cool#7865 sfx2 lok: fix bad view id on async command dispatch,
2023-12-20), so fetch the current view before the actual filter call and
work with that view explicitly later.
This is also similar to what SfxObjectShell::ExecFile_Impl() does for
the bMailPrepareExport case, which also had trouble with GUIStoreModel()
spinning the main loop.
Change-Id: Id642056aa55831c54e88c61931753c03fa23b6b0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178915
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/sfx2/source/doc/guisaveas.cxx b/sfx2/source/doc/guisaveas.cxx
index f0d3000a0e0c..adfc6c8345fa 100644
--- a/sfx2/source/doc/guisaveas.cxx
+++ b/sfx2/source/doc/guisaveas.cxx
@@ -1825,6 +1825,11 @@ bool
SfxStoringHelper::FinishGUIStoreModel(::comphelper::SequenceAsHashMap::cons
}
OSL_ENSURE( aModelData.GetMediaDescr().find( u"Password"_ustr ) ==
aModelData.GetMediaDescr().end(), "The Password property of MediaDescriptor
should not be used here!" );
+
+ // Fetch the current view early, saving may spin the main loop, which may
change the current
+ // view.
+ SfxViewShell* pViewShell = SfxViewShell::Current();
+
if ( officecfg::Office::Common::Save::Document::EditProperty::get()
&& ( !aModelData.GetStorable()->hasLocation()
|| INetURLObject( aModelData.GetStorable()->getLocation() ) != aURL
) )
@@ -1966,10 +1971,10 @@ bool
SfxStoringHelper::FinishGUIStoreModel(::comphelper::SequenceAsHashMap::cons
if ( comphelper::LibreOfficeKit::isActive() )
{
- if ( SfxViewShell* pShell = SfxViewShell::Current() )
+ if ( pViewShell )
{
OUString sURL = aURL.GetMainURL(
INetURLObject::DecodeMechanism::NONE );
- pShell->libreOfficeKitViewCallback( LOK_CALLBACK_EXPORT_FILE,
sURL.toUtf8() );
+ pViewShell->libreOfficeKitViewCallback( LOK_CALLBACK_EXPORT_FILE,
sURL.toUtf8() );
}
}
diff --git a/sw/qa/extras/tiledrendering/data/to-pdf.odt
b/sw/qa/extras/tiledrendering/data/to-pdf.odt
new file mode 100644
index 000000000000..26e7b01dc980
Binary files /dev/null and b/sw/qa/extras/tiledrendering/data/to-pdf.odt differ
diff --git a/sw/qa/extras/tiledrendering/tiledrendering2.cxx
b/sw/qa/extras/tiledrendering/tiledrendering2.cxx
index 112d0ce90c1d..0f9591d5afc3 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering2.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering2.cxx
@@ -19,6 +19,7 @@
#include <sfx2/msgpool.hxx>
#include <vcl/scheduler.hxx>
#include <comphelper/propertyvalue.hxx>
+#include <comphelper/dispatchcommand.hxx>
#include <view.hxx>
#include <IDocumentLayoutAccess.hxx>
@@ -268,6 +269,43 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest,
testFormatInsertStartList)
// - Expected: Calibri
// - Actual : MS Sans Serif
}
+
+/// Job on the main loop that switches to the first view.
+class ViewSwitcher
+{
+public:
+ DECL_STATIC_LINK(ViewSwitcher, SwitchView, void*, void);
+};
+
+IMPL_STATIC_LINK_NOARG(ViewSwitcher, SwitchView, void*, void) {
SfxLokHelper::setView(0); }
+
+CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testPDFExportViewSwitch)
+{
+ // Given a document with 2 views:
+ SwXTextDocument* pXTextDocument = createDoc("to-pdf.odt");
+ SwDoc* pDoc = pXTextDocument->GetDocShell()->GetDoc();
+ ViewCallback aView1;
+ SfxLokHelper::createView();
+ ViewCallback aView2;
+ SwView* pView2 = pDoc->GetDocShell()->GetView();
+ uno::Reference<frame::XFrame> xFrame2 =
pView2->GetViewFrame().GetFrame().GetFrameInterface();
+
+ // When exporting to PDF on the second view and a job on the main loop
that switches to the
+ // first view:
+ uno::Sequence<beans::PropertyValue> aPropertyValues = {
+ comphelper::makePropertyValue("SynchronMode", false),
+ comphelper::makePropertyValue("URL", maTempFile.GetURL()),
+ };
+ comphelper::dispatchCommand(".uno:ExportDirectToPDF", xFrame2,
aPropertyValues);
+ Application::PostUserEvent(LINK(nullptr, ViewSwitcher, SwitchView),
nullptr);
+ Scheduler::ProcessEventsToIdle();
+
+ // Then make sure the callback is invoked exactly on the second view:
+ // Without the accompanying fix in place, this test failed, as the
callback was invoked on the
+ // first view.
+ CPPUNIT_ASSERT(aView1.m_aExportFile.isEmpty());
+ CPPUNIT_ASSERT(!aView2.m_aExportFile.isEmpty());
+}
}
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/qa/extras/tiledrendering/tiledrenderingmodeltestbase.cxx
b/sw/qa/extras/tiledrendering/tiledrenderingmodeltestbase.cxx
index a53770b06cc3..96bfd851ce0c 100644
--- a/sw/qa/extras/tiledrendering/tiledrenderingmodeltestbase.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrenderingmodeltestbase.cxx
@@ -294,6 +294,7 @@ public:
boost::property_tree::ptree m_aComment;
std::vector<OString> m_aStateChanges;
TestLokCallbackWrapper m_callbackWrapper;
+ OString m_aExportFile;
ViewCallback(SfxViewShell* pViewShell = nullptr,
std::function<void(ViewCallback&)> const& rBeforeInstallFunc
= {})
@@ -471,6 +472,11 @@ public:
m_aDocColor = aPayload;
break;
}
+ case LOK_CALLBACK_EXPORT_FILE:
+ {
+ m_aExportFile = aPayload;
+ break;
+ }
}
}
};
