diff options
author | Luboš Luňák <l.lunak@collabora.com> | 2021-10-26 12:41:34 +0200 |
---|---|---|
committer | Luboš Luňák <l.lunak@collabora.com> | 2021-10-27 08:23:16 +0200 |
commit | c0138475cdbd378832c69b3592c21c84f7eaa2c0 (patch) | |
tree | e42c627cb4eddcf4abaa3c2508d4686ab458f597 | |
parent | 6af4b4c8dcfde5c8e258fc356cbe2d6cf94490c8 (diff) |
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/+/124208
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
-rw-r--r-- | sfx2/source/view/viewsh.cxx | 36 |
1 files changed, 11 insertions, 25 deletions
diff --git a/sfx2/source/view/viewsh.cxx b/sfx2/source/view/viewsh.cxx index fab9d5af2b13..325c9ffa71cc 100644 --- a/sfx2/source/view/viewsh.cxx +++ b/sfx2/source/view/viewsh.cxx @@ -1332,24 +1332,20 @@ SfxViewShell* SfxViewShell::GetFirst { // search for a SfxViewShell of the specified type SfxViewShellArr_Impl &rShells = SfxGetpApp()->GetViewShells_Impl(); - SfxViewFrameArr_Impl &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; } } @@ -1367,7 +1363,6 @@ SfxViewShell* SfxViewShell::GetNext ) { SfxViewShellArr_Impl &rShells = SfxGetpApp()->GetViewShells_Impl(); - SfxViewFrameArr_Impl &rFrames = SfxGetpApp()->GetViewFrames_Impl(); size_t nPos; for ( nPos = 0; nPos < rShells.size(); ++nPos ) if ( rShells[nPos] == &rPrev ) @@ -1378,19 +1373,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; } } |