From 41cf9c7438ccc1e254b977520d6cea96e20b654f Mon Sep 17 00:00:00 2001 From: Aleksey Pivovarov Date: Thu, 15 Mar 2018 20:00:24 +0300 Subject: [PATCH] vcs: fix batch file change operations handling --- .../openapi/vcs/ex/LineStatusTracker.kt | 14 ++++ .../vcs/ex/PartialLocalLineStatusTracker.kt | 27 ------- .../vcs/impl/LineStatusTrackerManager.kt | 53 +++++++++++++- .../vcs/BaseLineStatusTrackerManagerTest.kt | 13 ++++ .../vcs/LineStatusTrackerManagerTest.kt | 72 +++++++++++++++++++ 5 files changed, 150 insertions(+), 29 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.kt b/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.kt index dd8f3417d49e..e47d6ebd8735 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.kt +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.kt @@ -16,6 +16,7 @@ package com.intellij.openapi.vcs.ex import com.intellij.diff.util.DiffUtil +import com.intellij.diff.util.Side import com.intellij.ide.GeneralSettings import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.actionSystem.IdeActions @@ -30,6 +31,7 @@ import com.intellij.openapi.fileTypes.FileType import com.intellij.openapi.project.Project import com.intellij.openapi.vcs.changes.VcsDirtyScopeManager import com.intellij.openapi.vfs.VirtualFile +import org.jetbrains.annotations.CalledInAny import org.jetbrains.annotations.CalledInAwt import java.awt.Graphics import java.awt.Point @@ -131,4 +133,16 @@ abstract class LineStatusTracker constructor(override val project: Pr } } } + + @CalledInAny + internal fun freeze() { + documentTracker.freeze(Side.LEFT) + documentTracker.freeze(Side.RIGHT) + } + + @CalledInAwt + internal fun unfreeze() { + documentTracker.unfreeze(Side.LEFT) + documentTracker.unfreeze(Side.RIGHT) + } } diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/PartialLocalLineStatusTracker.kt b/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/PartialLocalLineStatusTracker.kt index 8bafea109bd8..ec9e20c4a175 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/PartialLocalLineStatusTracker.kt +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/PartialLocalLineStatusTracker.kt @@ -16,7 +16,6 @@ package com.intellij.openapi.vcs.ex import com.intellij.diff.util.Side -import com.intellij.ide.file.BatchFileChangeListener import com.intellij.openapi.Disposable import com.intellij.openapi.actionSystem.DefaultActionGroup import com.intellij.openapi.actionSystem.Separator @@ -54,7 +53,6 @@ import java.awt.Graphics import java.awt.Point import java.lang.ref.WeakReference import java.util.* -import java.util.concurrent.atomic.AtomicInteger import javax.swing.JComponent import javax.swing.JPanel import kotlin.collections.HashSet @@ -77,7 +75,6 @@ class PartialLocalLineStatusTracker(project: Project, private var lastKnownTrackerChangeListId: String? = null private val affectedChangeLists = HashSet() - private val batchChangeTaskCounter: AtomicInteger = AtomicInteger() private var hasUndoInCommand: Boolean = false private var shouldInitializeWithExcludedFromCommit: Boolean = false @@ -88,9 +85,6 @@ class PartialLocalLineStatusTracker(project: Project, defaultMarker = ChangeListMarker(changeListManager.defaultChangeList) affectedChangeLists.add(defaultMarker.changelistId) - val connection = application.messageBus.connect(disposable) - connection.subscribe(BatchFileChangeListener.TOPIC, MyBatchFileChangeListener()) - document.addDocumentListener(MyUndoDocumentListener(), disposable) CommandProcessor.getInstance().addCommandListener(MyUndoCommandListener(), disposable) Disposer.register(disposable, Disposable { dropExistingUndoActions() }) @@ -302,27 +296,6 @@ class PartialLocalLineStatusTracker(project: Project, undoableActions.add(action) } - private inner class MyBatchFileChangeListener : BatchFileChangeListener { - override fun batchChangeStarted(eventProject: Project, activityName: String?) { - if (eventProject != project) return - if (batchChangeTaskCounter.getAndIncrement() == 0) { - documentTracker.freeze(Side.LEFT) - documentTracker.freeze(Side.RIGHT) - } - } - - override fun batchChangeCompleted(eventProject: Project) { - if (eventProject != project) return - application.invokeLater( - { - if (batchChangeTaskCounter.decrementAndGet() == 0) { - documentTracker.unfreeze(Side.LEFT) - documentTracker.unfreeze(Side.RIGHT) - } - }, ModalityState.any()) - } - } - private inner class PartialDocumentTrackerHandler : LineStatusTrackerBase.MyDocumentTrackerHandler() { override fun onRangeAdded(block: Block) { super.onRangeAdded(block) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.kt b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.kt index 07b9e691143e..47d27811cfde 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.kt +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.kt @@ -18,6 +18,7 @@ package com.intellij.openapi.vcs.impl import com.google.common.collect.HashMultiset import com.google.common.collect.Multiset import com.intellij.icons.AllIcons +import com.intellij.ide.file.BatchFileChangeListener import com.intellij.notification.Notification import com.intellij.notification.NotificationAction import com.intellij.notification.NotificationType @@ -89,6 +90,7 @@ class LineStatusTrackerManager( private var partialChangeListsEnabled = VcsApplicationSettings.getInstance().ENABLE_PARTIAL_CHANGELISTS && Registry.`is`("vcs.enable.partial.changelists") private val documentsInDefaultChangeList = HashSet() + private var batchChangeTaskCounter: Int = 0 private val filesWithDamagedInactiveRanges = HashSet() private val fileStatesAwaitingRefresh = HashMap() @@ -115,8 +117,11 @@ class LineStatusTrackerManager( application.addApplicationListener(MyApplicationListener(), disposable) - val busConnection = project.messageBus.connect(disposable) - busConnection.subscribe(LineStatusTrackerSettingListener.TOPIC, MyLineStatusTrackerSettingListener()) + val projectConnection = project.messageBus.connect(disposable) + projectConnection.subscribe(LineStatusTrackerSettingListener.TOPIC, MyLineStatusTrackerSettingListener()) + + val appConnection = application.messageBus.connect(disposable) + appConnection.subscribe(BatchFileChangeListener.TOPIC, MyBatchFileChangeListener()) val fsManager = FileStatusManager.getInstance(project) fsManager.addFileStatusListener(MyFileStatusListener(), disposable) @@ -417,6 +422,10 @@ class LineStatusTrackerManager( refreshTracker(tracker) eventDispatcher.multicaster.onTrackerAdded(tracker) + if (batchChangeTaskCounter > 0) { + tracker.freeze() + } + log("Tracker installed", virtualFile) return tracker } @@ -748,6 +757,46 @@ class LineStatusTrackerManager( } } + private inner class MyBatchFileChangeListener : BatchFileChangeListener { + override fun batchChangeStarted(eventProject: Project, activityName: String?) { + if (eventProject != project) return + runReadAction { + synchronized(LOCK) { + if (batchChangeTaskCounter == 0) { + for (data in trackers.values) { + try { + data.tracker.freeze() + } + catch (e: Throwable) { + LOG.error(e) + } + } + } + batchChangeTaskCounter++ + } + } + } + + override fun batchChangeCompleted(eventProject: Project) { + if (eventProject != project) return + runInEdt(ModalityState.any()) { + synchronized(LOCK) { + batchChangeTaskCounter-- + if (batchChangeTaskCounter == 0) { + for (data in trackers.values) { + try { + data.tracker.unfreeze() + } + catch (e: Throwable) { + LOG.error(e) + } + } + } + } + } + } + } + private fun shouldBeUpdated(oldInfo: ContentInfo?, newInfo: ContentInfo): Boolean { if (oldInfo == null) return true diff --git a/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BaseLineStatusTrackerManagerTest.kt b/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BaseLineStatusTrackerManagerTest.kt index 04a55cccb91c..de7b6c6ecea0 100644 --- a/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BaseLineStatusTrackerManagerTest.kt +++ b/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BaseLineStatusTrackerManagerTest.kt @@ -1,12 +1,14 @@ // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.openapi.vcs +import com.intellij.ide.file.BatchFileChangeListener import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.command.CommandProcessor import com.intellij.openapi.editor.Document import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.progress.ProgressIndicator +import com.intellij.openapi.progress.util.BackgroundTaskUtil import com.intellij.openapi.project.Project import com.intellij.openapi.vcs.BaseLineStatusTrackerTestCase.Companion.parseInput import com.intellij.openapi.vcs.changes.* @@ -199,6 +201,17 @@ abstract class BaseLineStatusTrackerManagerTest : LightPlatformTestCase() { } + fun runBatchFileChangeOperation(task: () -> Unit) { + BackgroundTaskUtil.syncPublisher(BatchFileChangeListener.TOPIC).batchChangeStarted(ourProject, "Update") + try { + task() + } + finally { + BackgroundTaskUtil.syncPublisher(BatchFileChangeListener.TOPIC).batchChangeCompleted(ourProject) + } + } + + protected fun createChangelist(listName: String) { assertDoesntContain(changeListsNames(), listName) clm.addChangeList(listName, null) diff --git a/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/LineStatusTrackerManagerTest.kt b/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/LineStatusTrackerManagerTest.kt index ba2fe3807815..1735f8883502 100644 --- a/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/LineStatusTrackerManagerTest.kt +++ b/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/LineStatusTrackerManagerTest.kt @@ -332,4 +332,76 @@ class LineStatusTrackerManagerTest : BaseLineStatusTrackerManagerTest() { tracker.assertAffectedChangeLists("Default") } } + + fun `test bulk refresh freezes tracker - inner operation`() { + val file = addLocalFile(FILE_1, "a_b_c_d_e2") + setBaseVersion(FILE_1, "a_b_c_d_e") + refreshCLM() + + file.withOpenedEditor { + val tracker = file.tracker as PartialLocalLineStatusTracker + lstm.waitUntilBaseContentsLoaded() + assertTrue(tracker.isValid()) + + runBatchFileChangeOperation { + assertFalse(tracker.isValid()) + } + assertTrue(tracker.isValid()) + } + assertNull(file.tracker) + } + + fun `test bulk refresh freezes tracker - outer operation`() { + val file = addLocalFile(FILE_1, "a_b_c_d_e2") + setBaseVersion(FILE_1, "a_b_c_d_e") + refreshCLM() + + runBatchFileChangeOperation { + file.withOpenedEditor { + val tracker = file.tracker as PartialLocalLineStatusTracker + lstm.waitUntilBaseContentsLoaded() + assertFalse(tracker.isValid()) + } + } + assertNull(file.tracker) + } + + fun `test bulk refresh freezes tracker - interleaving operation`() { + val file = addLocalFile(FILE_1, "a_b_c_d_e2") + setBaseVersion(FILE_1, "a_b_c_d_e") + refreshCLM() + + runBatchFileChangeOperation { + lstm.requestTrackerFor(file.document, this) + lstm.waitUntilBaseContentsLoaded() + + val tracker = file.tracker as PartialLocalLineStatusTracker + assertFalse(tracker.isValid()) + } + val tracker = file.tracker as PartialLocalLineStatusTracker + assertTrue(tracker.isValid()) + lstm.releaseTrackerFor(file.document, this) + assertNull(file.tracker) + } + + fun `test bulk refresh freezes tracker - multiple operations`() { + val file = addLocalFile(FILE_1, "a_b_c_d_e2") + setBaseVersion(FILE_1, "a_b_c_d_e") + refreshCLM() + + file.withOpenedEditor { + val tracker = file.tracker as PartialLocalLineStatusTracker + lstm.waitUntilBaseContentsLoaded() + assertTrue(tracker.isValid()) + + runBatchFileChangeOperation { + runBatchFileChangeOperation { + assertFalse(tracker.isValid()) + } + assertFalse(tracker.isValid()) + } + assertTrue(tracker.isValid()) + } + assertNull(file.tracker) + } } \ No newline at end of file