sfx2/source/view/viewsh.cxx | 36 +++++++++++-------------------------
1 file changed, 11 insertions(+), 25 deletions(-)
New commits:
commit 453178d33d76248ead90c3547b2f140d8e7bb998
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Oct 26 12:32:25 2021 +0200
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Wed Oct 27 08:23:02 2021 +0200
remove useless check from SfxViewShell::GetFirst()/GetNext()
This comes from https://bz.apache.org/ooo/show_bug.cgi?id=62084 ,
but it's unclear to me what the steps to reproduce actually mean.
If it's print preview, then I can't reproduce, and all tests work
fine too. This code is called very often from LOK code in a loop,
and this check makes it O(n^2), so remove it under the assumption
that the problem no longer exists, only keep an assert just in case.
Change-Id: I0e7ed03ef370aa32f2064c587b242e1ffaff73b2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124185
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sfx2/source/view/viewsh.cxx b/sfx2/source/view/viewsh.cxx
index ecfd3f97bba4..ee77dcadbde0 100644
--- a/sfx2/source/view/viewsh.cxx
+++ b/sfx2/source/view/viewsh.cxx
@@ -1323,24 +1323,20 @@ SfxViewShell* SfxViewShell::GetFirst
{
// search for a SfxViewShell of the specified type
std::vector<SfxViewShell*> &rShells = SfxGetpApp()->GetViewShells_Impl();
- auto &rFrames = SfxGetpApp()->GetViewFrames_Impl();
for (SfxViewShell* pShell : rShells)
{
if ( pShell )
{
+ // This code used to check that the frame exists in the other list,
+ // because of https://bz.apache.org/ooo/show_bug.cgi?id=62084,
with the explanation:
// sometimes dangling SfxViewShells exist that point to a dead
SfxViewFrame
// these ViewShells shouldn't be accessible anymore
// a destroyed ViewFrame is not in the ViewFrame array anymore, so
checking this array helps
- for (SfxViewFrame* pFrame : rFrames)
- {
- if ( pFrame == pShell->GetViewFrame() )
- {
- // only ViewShells with a valid ViewFrame will be returned
- if ( ( !bOnlyVisible || pFrame->IsVisible() ) &&
(!isViewShell || isViewShell(pShell)))
- return pShell;
- break;
- }
- }
+ // That doesn't seem to be needed anymore, but keep an assert,
just in case.
+ assert(std::find(SfxGetpApp()->GetViewFrames_Impl().begin(),
SfxGetpApp()->GetViewFrames_Impl().end(),
+ pShell->GetViewFrame()) !=
SfxGetpApp()->GetViewFrames_Impl().end());
+ if ( ( !bOnlyVisible || pShell->GetViewFrame()->IsVisible() ) &&
(!isViewShell || isViewShell(pShell)))
+ return pShell;
}
}
@@ -1358,7 +1354,6 @@ SfxViewShell* SfxViewShell::GetNext
)
{
std::vector<SfxViewShell*> &rShells = SfxGetpApp()->GetViewShells_Impl();
- auto &rFrames = SfxGetpApp()->GetViewFrames_Impl();
size_t nPos;
for ( nPos = 0; nPos < rShells.size(); ++nPos )
if ( rShells[nPos] == &rPrev )
@@ -1369,19 +1364,10 @@ SfxViewShell* SfxViewShell::GetNext
SfxViewShell *pShell = rShells[nPos];
if ( pShell )
{
- // sometimes dangling SfxViewShells exist that point to a dead
SfxViewFrame
- // these ViewShells shouldn't be accessible anymore
- // a destroyed ViewFrame is not in the ViewFrame array anymore, so
checking this array helps
- for (SfxViewFrame* pFrame : rFrames)
- {
- if ( pFrame == pShell->GetViewFrame() )
- {
- // only ViewShells with a valid ViewFrame will be returned
- if ( ( !bOnlyVisible || pFrame->IsVisible() ) &&
(!isViewShell || isViewShell(pShell)) )
- return pShell;
- break;
- }
- }
+ assert(std::find(SfxGetpApp()->GetViewFrames_Impl().begin(),
SfxGetpApp()->GetViewFrames_Impl().end(),
+ pShell->GetViewFrame()) !=
SfxGetpApp()->GetViewFrames_Impl().end());
+ if ( ( !bOnlyVisible || pShell->GetViewFrame()->IsVisible() ) &&
(!isViewShell || isViewShell(pShell)) )
+ return pShell;
}
}
