From a45dc2c31a565da848c19e8045cd1f2e4a0faa73 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Mon, 19 Apr 2021 16:49:29 +0200 Subject: [PATCH] refactor indicator pushState: introduce dedicated internal State class to ease/localize storing additional info, store "isIndeterminate" flag GitOrigin-RevId: e9d93cbe3b0c640bbdeb263129ce9c570dfea09e --- .../openapi/progress/ProgressIndicator.java | 6 ++ .../util/AbstractProgressIndicatorBase.java | 95 +++++++++---------- .../progress/impl/ProgressIndicatorTest.java | 10 ++ 3 files changed, 61 insertions(+), 50 deletions(-) diff --git a/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicator.java b/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicator.java index ac6f9d0a6e05..0e11f10117f2 100644 --- a/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicator.java +++ b/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicator.java @@ -117,8 +117,14 @@ public interface ProgressIndicator { */ void setFraction(double fraction); + /** + * Stores {@link #getText()}, {@link #getText2()}, {@link #isIndeterminate()} and {@link #getFraction()} to the temporary stack, to be restored later via {@link #popState()} + */ void pushState(); + /** + * Restores {@link #getText()}, {@link #getText2()}, {@link #isIndeterminate()} and {@link #getFraction()} from the temporary stack, stored earlier by {@link #pushState()} + */ void popState(); /** diff --git a/platform/core-impl/src/com/intellij/openapi/progress/util/AbstractProgressIndicatorBase.java b/platform/core-impl/src/com/intellij/openapi/progress/util/AbstractProgressIndicatorBase.java index 845c27ff500a..4ae2a3c8dab6 100644 --- a/platform/core-impl/src/com/intellij/openapi/progress/util/AbstractProgressIndicatorBase.java +++ b/platform/core-impl/src/com/intellij/openapi/progress/util/AbstractProgressIndicatorBase.java @@ -22,7 +22,6 @@ import com.intellij.util.DeprecatedMethodException; import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.Stack; -import it.unimi.dsi.fastutil.doubles.DoubleArrayList; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -47,9 +46,24 @@ public class AbstractProgressIndicatorBase extends UserDataHolderBase implements // false by default - do not attempt to use such a relatively heavy code on start-up private volatile boolean myShouldStartActivity = SystemInfoRt.isMac && Boolean.parseBoolean(System.getProperty("idea.mac.prevent.app.nap", "true")); - private Stack<@NlsContexts.ProgressText String> myTextStack; // guarded by this - private DoubleArrayList myFractionStack; // guarded by this - private Stack<@NlsContexts.ProgressDetails String> myText2Stack; // guarded by this + private static class State { + private final @NlsContexts.ProgressText String myText; + private final @NlsContexts.ProgressDetails String myText2; + private final double myFraction; + private final boolean myIndeterminate; + + private State(@NlsContexts.ProgressText String text, + @NlsContexts.ProgressDetails String text2, + double fraction, + boolean indeterminate) { + myText = text; + myText2 = text2; + myFraction = fraction; + myIndeterminate = indeterminate; + } + } + + private Stack myStateStack; // guarded by this private ProgressIndicator myModalityProgress; private volatile ModalityState myModalityState = ModalityState.NON_MODAL; @@ -181,42 +195,39 @@ public class AbstractProgressIndicatorBase extends UserDataHolderBase implements @Override public void setFraction(final double fraction) { - if (isIndeterminate()) { - StackTraceElement[] trace = new Throwable().getStackTrace(); - StackTraceElement first = ContainerUtil.find(trace, - element -> !element.getClassName().startsWith("com.intellij.openapi.progress.util")); - @NonNls String message = "This progress indicator is indeterminate, this may lead to visual inconsistency. " + - "Please call setIndeterminate(false) before you start progress."; - if (first != null) { - message += "\n" + first; + synchronized (getLock()) { + if (isIndeterminate()) { + StackTraceElement[] trace = new Throwable().getStackTrace(); + StackTraceElement first = ContainerUtil.find(trace, + element -> !element.getClassName().startsWith("com.intellij.openapi.progress.util")); + @NonNls String message = "This progress indicator is indeterminate, this may lead to visual inconsistency. " + + "Please call setIndeterminate(false) before you start progress."; + if (first != null) { + message += "\n" + first; + } + LOG.warn(message); + setIndeterminate(false); } - LOG.warn(message); - setIndeterminate(false); + myFraction = fraction; } - myFraction = fraction; } @Override public void pushState() { synchronized (getLock()) { - getTextStack().push(myText); - getFractionStack().add(myFraction); - getText2Stack().push(myText2); + getStateStack().push(new State(getText(), getText2(), getFraction(), isIndeterminate())); } } @Override public void popState() { synchronized (getLock()) { - LOG.assertTrue(!myTextStack.isEmpty()); - String oldText = myTextStack.pop(); - String oldText2 = myText2Stack.pop(); - setText(oldText); - setText2(oldText2); - - double oldFraction = myFractionStack.removeDouble(myFractionStack.size() - 1); + State state = myStateStack.pop(); + setText(state.myText); + setText2(state.myText2); + setIndeterminate(state.myIndeterminate); if (!isIndeterminate()) { - setFraction(oldFraction); + setFraction(state.myFraction); } } } @@ -276,7 +287,10 @@ public class AbstractProgressIndicatorBase extends UserDataHolderBase implements @Override public void setIndeterminate(final boolean indeterminate) { - myIndeterminate = indeterminate; + // avoid race with popState() + synchronized (getLock()) { + myIndeterminate = indeterminate; + } } @@ -311,9 +325,7 @@ public class AbstractProgressIndicatorBase extends UserDataHolderBase implements if (indicator instanceof AbstractProgressIndicatorBase) { AbstractProgressIndicatorBase stacked = (AbstractProgressIndicatorBase)indicator; - myTextStack = stacked.myTextStack == null ? null : new Stack<>(stacked.getTextStack()); - myText2Stack = stacked.myText2Stack == null ? null : new Stack<>(stacked.getText2Stack()); - myFractionStack = stacked.myFractionStack == null ? null : new DoubleArrayList(stacked.getFractionStack().toDoubleArray()); + myStateStack = stacked.myStateStack == null ? null : new Stack<>(stacked.getStateStack()); } dontStartActivity(); } @@ -324,26 +336,9 @@ public class AbstractProgressIndicatorBase extends UserDataHolderBase implements } @NotNull - private Stack getTextStack() { - Stack stack = myTextStack; - if (stack == null) myTextStack = stack = new Stack<>(2); - return stack; - } - - @NotNull - private DoubleArrayList getFractionStack() { - DoubleArrayList stack = myFractionStack; - if (stack == null) { - stack = new DoubleArrayList(2); - myFractionStack = stack; - } - return stack; - } - - @NotNull - private Stack getText2Stack() { - Stack stack = myText2Stack; - if (stack == null) myText2Stack = stack = new Stack<>(2); + private Stack getStateStack() { + Stack stack = myStateStack; + if (stack == null) myStateStack = stack = new Stack<>(2); return stack; } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java index 326e5db6e5ee..505dfe0e7c52 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java @@ -907,6 +907,16 @@ public class ProgressIndicatorTest extends LightPlatformTestCase { indicator.popState(); // should not cause NPE } + public void testPushStateMustStoreIndeterminateFlag() { + ProgressIndicatorEx indicator = new ProgressIndicatorBase(); + indicator.setIndeterminate(true); + indicator.pushState(); + indicator.setIndeterminate(false); + assertFalse(indicator.isIndeterminate()); + indicator.popState(); + assertTrue(indicator.isIndeterminate()); + } + public void testRelayUiToDelegateIndicatorMustBeReusable() { ProgressIndicatorEx ui = new ProgressIndicatorBase(); RelayUiToDelegateIndicator relay = new RelayUiToDelegateIndicator(ui);