From 554f0340ed983977e326fe8cdfa5508cd7b1a201 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 15 Mar 2017 19:45:50 +0100 Subject: [PATCH] remove modal invokeLater from bg document commit (IDEA-169201) --- .../core/MockDocumentCommitProcessor.java | 4 +-- .../psi/impl/DocumentCommitProcessor.java | 9 ++--- .../psi/impl/DocumentCommitThread.java | 36 +++++++++---------- .../psi/impl/PsiDocumentManagerBase.java | 4 +-- .../psi/impl/PsiDocumentManagerImpl.java | 3 +- .../psi/impl/PsiDocumentManagerImplTest.java | 19 ++++++++++ 6 files changed, 48 insertions(+), 27 deletions(-) diff --git a/platform/core-impl/src/com/intellij/core/MockDocumentCommitProcessor.java b/platform/core-impl/src/com/intellij/core/MockDocumentCommitProcessor.java index aef33bbd9aef..92784387ab48 100644 --- a/platform/core-impl/src/com/intellij/core/MockDocumentCommitProcessor.java +++ b/platform/core-impl/src/com/intellij/core/MockDocumentCommitProcessor.java @@ -15,7 +15,7 @@ */ package com.intellij.core; -import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.application.TransactionId; import com.intellij.openapi.editor.Document; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiFile; @@ -35,6 +35,6 @@ class MockDocumentCommitProcessor implements DocumentCommitProcessor { public void commitAsynchronously(@NotNull Project project, @NotNull Document document, @NonNls @NotNull Object reason, - @NotNull ModalityState currentModalityState) { + @NotNull TransactionId context) { } } diff --git a/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitProcessor.java b/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitProcessor.java index ca04c2cfaeb4..1fd9254724bb 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitProcessor.java +++ b/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitProcessor.java @@ -15,17 +15,18 @@ */ package com.intellij.psi.impl; -import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.application.TransactionId; import com.intellij.openapi.editor.Document; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiFile; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public interface DocumentCommitProcessor { void commitSynchronously(@NotNull Document document, @NotNull Project project, @NotNull PsiFile psiFile); void commitAsynchronously(@NotNull final Project project, - @NotNull final Document document, - @NonNls @NotNull Object reason, - @NotNull ModalityState currentModalityState); + @NotNull final Document document, + @NonNls @NotNull Object reason, + @Nullable TransactionId context); } diff --git a/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java b/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java index 7c87ef4ccdd0..309c06cbbb84 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java +++ b/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java @@ -152,25 +152,25 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi public void commitAsynchronously(@NotNull final Project project, @NotNull final Document document, @NonNls @NotNull Object reason, - @NotNull ModalityState currentModalityState) { + @Nullable TransactionId context) { assert !isDisposed : "already disposed"; if (!project.isInitialized()) return; PsiFile psiFile = PsiDocumentManager.getInstance(project).getCachedPsiFile(document); if (psiFile == null) return; - doQueue(project, document, getAllFileNodes(psiFile), reason, currentModalityState, + doQueue(project, document, getAllFileNodes(psiFile), reason, context, PsiDocumentManager.getInstance(project).getLastCommittedText(document)); } private void doQueue(@NotNull Project project, - @NotNull Document document, - @NotNull List> oldFileNodes, - @NotNull Object reason, - @NotNull ModalityState currentModalityState, - @NotNull CharSequence lastCommittedText) { + @NotNull Document document, + @NotNull List> oldFileNodes, + @NotNull Object reason, + @Nullable TransactionId context, + @NotNull CharSequence lastCommittedText) { synchronized (lock) { if (!project.isInitialized()) return; // check the project is disposed under lock. - CommitTask newTask = createNewTaskAndCancelSimilar(project, document, oldFileNodes, reason, currentModalityState, + CommitTask newTask = createNewTaskAndCancelSimilar(project, document, oldFileNodes, reason, context, lastCommittedText); documentsToCommit.offer(newTask); @@ -185,13 +185,13 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi @NotNull Document document, @NotNull List> oldFileNodes, @NotNull Object reason, - @NotNull ModalityState currentModalityState, + @Nullable TransactionId context, @NotNull CharSequence lastCommittedText) { synchronized (lock) { for (Pair pair : oldFileNodes) { assert pair.first.getProject() == project; } - CommitTask newTask = new CommitTask(project, document, oldFileNodes, createProgressIndicator(), reason, currentModalityState, + CommitTask newTask = new CommitTask(project, document, oldFileNodes, createProgressIndicator(), reason, context, lastCommittedText); cancelAndRemoveFromDocsToCommit(newTask, reason); cancelAndRemoveCurrentTask(newTask, reason); @@ -380,7 +380,7 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi if (success) { assert !myApplication.isDispatchThread(); TransactionGuardImpl guard = (TransactionGuardImpl)TransactionGuard.getInstance(); - guard.submitTransaction(project, guard.getModalityTransaction(task.myCreationModalityState), finishRunnable); + guard.submitTransaction(project, task.myCreationContext, finishRunnable); } } } @@ -408,7 +408,7 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi List> oldFileNodes = file == null ? null : getAllFileNodes(file); if (oldFileNodes != null) { doQueue(finalProject, finalDocument, oldFileNodes, "re-added on failure: " + finalFailureReason, - finalTask.myCreationModalityState, + finalTask.myCreationContext, lastCommittedText); } }); @@ -442,7 +442,7 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi CommitTask task; synchronized (lock) { // synchronized to ensure no new similar tasks can start before we hold the document's lock - task = createNewTaskAndCancelSimilar(project, document, allFileNodes, SYNC_COMMIT_REASON, ModalityState.current(), + task = createNewTaskAndCancelSimilar(project, document, allFileNodes, SYNC_COMMIT_REASON, TransactionGuard.getInstance().getContextTransaction(), PsiDocumentManager.getInstance(project).getLastCommittedText(document)); documentLock.lock(); } @@ -541,7 +541,7 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi throw new PsiInvalidElementAccessException(file, "File " + file + " invalidated during sync commit"); } commitAsynchronously(project, document, "File " + file + " invalidated during background commit; task: "+task, - task.myCreationModalityState); + task.myCreationContext); } } } @@ -605,7 +605,7 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi } else { // add document back to the queue - commitAsynchronously(project, document, "Re-added back", task.myCreationModalityState); + commitAsynchronously(project, document, "Re-added back", task.myCreationContext); } }; } @@ -655,7 +655,7 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi // when failed it's canceled @NotNull final ProgressIndicator indicator; // progress to commit this doc under. @NotNull final Object reason; - @NotNull final ModalityState myCreationModalityState; + @Nullable final TransactionId myCreationContext; private final CharSequence myLastCommittedText; @NotNull final List> myOldFileNodes; @@ -664,13 +664,13 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi @NotNull final List> oldFileNodes, @NotNull ProgressIndicator indicator, @NotNull Object reason, - @NotNull ModalityState currentModalityState, + @Nullable TransactionId context, @NotNull CharSequence lastCommittedText) { this.document = document; this.project = project; this.indicator = indicator; this.reason = reason; - myCreationModalityState = currentModalityState; + myCreationContext = context; myLastCommittedText = lastCommittedText; myOldFileNodes = oldFileNodes; modificationSequence = ((DocumentEx)document).getModificationSequence(); diff --git a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java index 54d78819c4ec..0a13546a7dcc 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java @@ -544,7 +544,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen } actions.add(action); - ModalityState current = ModalityState.current(); + TransactionId current = TransactionGuard.getInstance().getContextTransaction(); if (current != ModalityState.NON_MODAL) { // re-add all uncommitted documents into the queue with this new modality // because this client obviously expects them to commit even inside modal dialog @@ -846,7 +846,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen commitDocument(document); } else if (!((DocumentEx)document).isInBulkUpdate() && myPerformBackgroundCommit) { - myDocumentCommitProcessor.commitAsynchronously(myProject, document, event, ApplicationManager.getApplication().getCurrentModalityState()); + myDocumentCommitProcessor.commitAsynchronously(myProject, document, event, TransactionGuard.getInstance().getContextTransaction()); } } else { diff --git a/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java b/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java index ccbefe64ab09..6ad90f8e91be 100644 --- a/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java +++ b/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java @@ -22,6 +22,7 @@ import com.intellij.injected.editor.DocumentWindowImpl; import com.intellij.injected.editor.EditorWindowImpl; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.components.SettingsSavingComponent; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.EditorFactory; @@ -78,7 +79,7 @@ public class PsiDocumentManagerImpl extends PsiDocumentManagerBase implements Se connection.subscribe(DocumentBulkUpdateListener.TOPIC, new DocumentBulkUpdateListener.Adapter() { @Override public void updateFinished(@NotNull Document doc) { - documentCommitThread.commitAsynchronously(project, doc, "Bulk update finished", ApplicationManager.getApplication().getDefaultModalityState()); + documentCommitThread.commitAsynchronously(project, doc, "Bulk update finished", TransactionGuard.getInstance().getContextTransaction()); } }); Disposer.register(project, () -> ((DocumentCommitThread)myDocumentCommitThread).cancelTasksOnProjectDispose(project)); diff --git a/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java b/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java index b10c3cbad19c..3ae54d80c957 100644 --- a/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java @@ -21,6 +21,7 @@ import com.intellij.mock.MockDocument; import com.intellij.mock.MockPsiFile; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.application.impl.LaterInvocator; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Document; @@ -563,6 +564,24 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase { assertTrue(getPsiDocumentManager().isCommitted(document)); } + public void testBackgroundCommitInDialogInTransaction() throws IOException { + VirtualFile vFile = getVirtualFile(createTempFile("a.txt", "abc")); + PsiFile psiFile = findFile(vFile); + Document document = getDocument(psiFile); + + TransactionGuard.submitTransaction(myProject, () -> { + WriteCommandAction.runWriteCommandAction(myProject, () -> { + document.insertString(0, "x"); + LaterInvocator.enterModal(new Object()); + assertFalse(getPsiDocumentManager().isCommitted(document)); + }); + + waitTenSecondsForCommit(document); + assertTrue(getPsiDocumentManager().isCommitted(document)); + }); + UIUtil.dispatchAllInvocationEvents(); + } + public void testChangeDocumentThenEnterModalDialogThenCallPerformWhenAllCommittedShouldFireWhileInsideModal() throws IOException { VirtualFile vFile = getVirtualFile(createTempFile("a.txt", "abc")); PsiFile psiFile = findFile(vFile);