Suppress disconnect warning dialog until adb restart completes

Issue: #197
mlopatkin · Dec 25, 2023 · 19534a1 · 19534a1
1 parent 3d29724
commit 19534a1
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 10 deletions.
diff --git a/src/name/mlopatkin/andlogview/liblogcat/ddmlib/DeviceDisconnectedHandler.java b/src/name/mlopatkin/andlogview/liblogcat/ddmlib/DeviceDisconnectedHandler.java
@@ -20,6 +20,8 @@
 import name.mlopatkin.andlogview.ui.mainframe.ErrorDialogs;
 import name.mlopatkin.andlogview.ui.mainframe.MainFrameScoped;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 import java.util.concurrent.Executor;
 
 import javax.inject.Inject;
@@ -39,6 +41,9 @@ public interface DeviceAwaiter {
     private final AdbConfigurationPref adbConfigurationPref;
     private final Executor uiExecutor;
 
+    private boolean isSuppressed;
+    private @Nullable String suppressedError;
+
     @Inject
     DeviceDisconnectedHandler(
             DeviceAwaiter deviceAwaiter,
@@ -69,9 +74,11 @@ public void onDataSourceInvalidated(AdbDataSource.InvalidationReason reason) {
     private void onDeviceDisconnected(String message) {
         if (adbConfigurationPref.isAutoReconnectEnabled()) {
             deviceAwaiter.waitForDevice();
-        } else {
+        } else if (!isSuppressed) {
             // The dialog is modal, and must be shown in async manner. Otherwise, we'll block all other listeners.
             uiExecutor.execute(() -> showNotificationDialog(message));
+        } else {
+            suppressedError = message;
         }
     }
 
@@ -88,4 +95,25 @@ private void showNotificationDialog(String message) {
     public void startWatching(AdbDataSource dataSource) {
         dataSource.asStateObservable().addObserver(this);
     }
+
+    /**
+     * Suppresses error dialogs until {@link #resumeDialogs()} is called.
+     */
+    public void suppressDialogs() {
+        isSuppressed = true;
+    }
+
+    /**
+     * Resumes showing error dialogs. Can show the last suppressed error if any.
+     *
+     */
+    public void resumeDialogs() {
+        isSuppressed = false;
+        var suppressedError = this.suppressedError;
+        this.suppressedError = null;
+
+        if (suppressedError != null) {
+            onDeviceDisconnected(suppressedError);
+        }
+    }
 }
diff --git a/src/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenter.java b/src/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenter.java
@@ -21,6 +21,7 @@
 
 import name.mlopatkin.andlogview.AppExecutors;
 import name.mlopatkin.andlogview.device.AdbDeviceList;
+import name.mlopatkin.andlogview.liblogcat.ddmlib.DeviceDisconnectedHandler;
 import name.mlopatkin.andlogview.ui.mainframe.MainFrameScoped;
 import name.mlopatkin.andlogview.utils.Cancellable;
 import name.mlopatkin.andlogview.utils.MyFutures;
@@ -63,18 +64,23 @@ public interface View {
     private final AdbServicesBridge bridge;
     private final View view;
     private final Executor uiExecutor;
+    private final DeviceDisconnectedHandler deviceDisconnectedHandler;
     // Maintains set of the current requests to show loading progress. Useful, when new show and old hide requests
     // overlap because of the delayed execution of hide callback.
     private final Set<Object> progressTokens = new HashSet<>();
 
     private boolean hasShownErrorMessage;
 
     @Inject
-    AdbServicesInitializationPresenter(AdbServicesBridge bridge, View view,
-            @Named(AppExecutors.UI_EXECUTOR) Executor uiExecutor) {
+    AdbServicesInitializationPresenter(
+            View view,
+            AdbServicesBridge bridge,
+            @Named(AppExecutors.UI_EXECUTOR) Executor uiExecutor,
+            DeviceDisconnectedHandler deviceDisconnectedHandler) {
         this.bridge = bridge;
         this.view = view;
         this.uiExecutor = uiExecutor;
+        this.deviceDisconnectedHandler = deviceDisconnectedHandler;
     }
 
     /**
@@ -93,7 +99,7 @@ public AdbDeviceList withAdbDeviceList() {
      * Initializes the adb services and executes the action. This method should be used when the user triggers the
      * action, it takes care of indicating the pause. The actions are executed on the UI executor.
      * <p>
-     * The initialization may be cancelled by using the returned handle. When cancelled, no callbacks are invoked.
+     * The initialization may be cancelled by using the returned handle. When cancelled, the failure handler is invoked.
      *
      * @param action the action to execute
      * @param failureHandler the failure handler
@@ -111,7 +117,7 @@ public Cancellable withAdbServicesInteractive(Consumer<? super AdbServices> acti
                     uiExecutor);
         }
         future.handleAsync(
-                        consumingHandler(action, ignoreCancellations(adbErrorHandler().andThen(failureHandler))),
+                        consumingHandler(action, ignoreCancellations(adbErrorHandler()).andThen(failureHandler)),
                         uiExecutor)
                 .exceptionally(MyFutures::uncaughtException);
         // TODO(mlopatkin) Should we always show a failure message if the ADB fails/is failed for the interactive
@@ -142,8 +148,10 @@ private void hideProgressWithToken(Object token) {
     private Consumer<Throwable> adbErrorHandler() {
         return ignored -> {
             if (!hasShownErrorMessage) {
-                view.showAdbLoadingError();
                 hasShownErrorMessage = true;
+                // showAdbLoadingError blocks and opens a nested message pump. It is important to set up the flag before
+                // showing the dialog to prevent re-entrance and double dialog.
+                view.showAdbLoadingError();
             }
         };
     }
@@ -154,6 +162,9 @@ private Consumer<Throwable> adbErrorHandler() {
     public void restartAdb() {
         bridge.stopAdb();
         hasShownErrorMessage = false;
-        withAdbServicesInteractive(adb -> {}, failure -> {});
+        deviceDisconnectedHandler.suppressDialogs();
+        withAdbServicesInteractive(
+                adbServices -> deviceDisconnectedHandler.resumeDialogs(),
+                failure -> deviceDisconnectedHandler.resumeDialogs());
     }
 }
diff --git a/test/name/mlopatkin/andlogview/liblogcat/ddmlib/DeviceDisconnectedHandlerTest.java b/test/name/mlopatkin/andlogview/liblogcat/ddmlib/DeviceDisconnectedHandlerTest.java
@@ -17,6 +17,7 @@
 package name.mlopatkin.andlogview.liblogcat.ddmlib;
 
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
@@ -65,6 +66,49 @@ void waitsForDeviceWhenDeviceDisconnectsAndAutoReconnectEnabled(AdbDataSource.In
         assertWaitForDevice();
     }
 
+    @ParameterizedTest
+    @EnumSource(AdbDataSource.InvalidationReason.class)
+    void errorMessagesCanBeSuppressed(AdbDataSource.InvalidationReason reason) {
+        when(adbConfigurationPref.isAutoReconnectEnabled()).thenReturn(false);
+
+        var handler = createHandler();
+        handler.suppressDialogs();
+        handler.onDataSourceInvalidated(reason);
+
+        assertNoErrorDialogShown();
+    }
+
+    @ParameterizedTest
+    @EnumSource(AdbDataSource.InvalidationReason.class)
+    void errorMessagesAreShownAfterResuming(AdbDataSource.InvalidationReason reason) {
+        when(adbConfigurationPref.isAutoReconnectEnabled()).thenReturn(false);
+
+        var handler = createHandler();
+        handler.suppressDialogs();
+        handler.resumeDialogs();
+        handler.onDataSourceInvalidated(reason);
+
+        assertErrorDialogShown();
+    }
+
+    @ParameterizedTest
+    @EnumSource(AdbDataSource.InvalidationReason.class)
+    void suppressedErrorMessagesAreShownAfterResuming(AdbDataSource.InvalidationReason reason) {
+        when(adbConfigurationPref.isAutoReconnectEnabled()).thenReturn(false);
+
+        var handler = createHandler();
+        handler.suppressDialogs();
+        handler.onDataSourceInvalidated(reason);
+        handler.resumeDialogs();
+
+        assertErrorDialogShown();
+    }
+
+    private void assertNoErrorDialogShown() {
+        verify(errorDialogs, never()).showDeviceDisconnectedWarning(any());
+        verifyNoInteractions(deviceAwaiter);
+    }
+
     private void assertErrorDialogShown() {
         verify(errorDialogs).showDeviceDisconnectedWarning(any());
         verifyNoInteractions(deviceAwaiter);

diff --git a/test/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenterTest.java b/test/name/mlopatkin/andlogview/ui/device/AdbServicesInitializationPresenterTest.java
@@ -27,6 +27,7 @@
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 
+import name.mlopatkin.andlogview.base.MyThrowables;
 import name.mlopatkin.andlogview.base.concurrent.TestExecutor;
 import name.mlopatkin.andlogview.device.AdbException;
 import name.mlopatkin.andlogview.test.ThreadTestUtils;
@@ -42,10 +43,12 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.Answers;
+import org.mockito.ArgumentMatchers;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
 import java.util.function.Consumer;
@@ -107,7 +110,7 @@ void cancelledRequestShowsNoMessagesIfAdbFails(ServiceRequest request, AdbInitRe
 
         adbInit.tryInitAdb(this);
 
-        thenRequestNotCompleted();
+        thenRequestCancelled();
         thenNoErrorIsShown();
     }
 
@@ -313,7 +316,7 @@ private AdbServicesInitializationPresenter createPresenter(Executor uiExecutor)
         var mockBridge = mock(AdbServicesBridge.class);
         lenient().when(mockBridge.getAdbServicesAsync())
                 .thenAnswer(invocation -> servicesFuture.thenApply(Function.identity()));
-        return new AdbServicesInitializationPresenter(mockBridge, view, uiExecutor);
+        return new AdbServicesInitializationPresenter(view, mockBridge, uiExecutor, mock());
     }
 
     private void whenAdbInitFailed() {
@@ -351,6 +354,12 @@ private void thenRequestFailed() {
         verify(errorConsumer).accept(any());
     }
 
+    private void thenRequestCancelled() {
+        verify(servicesConsumer, never()).accept(any());
+        verify(errorConsumer).accept(ArgumentMatchers.argThat(
+                failure -> MyThrowables.unwrapUninteresting(failure) instanceof CancellationException));
+    }
+
     private void thenNoProgressIsShown() {
         verify(view, never()).showAdbLoadingProgress();
     }
@@ -427,7 +436,7 @@ public String toString() {
     }
 
     @FunctionalInterface
-    interface AdbInitRef  {
+    interface AdbInitRef {
         void tryInitAdb(AdbServicesInitializationPresenterTest test);
     }