Fix FluentWait so it completes in more cases.

FluentWait could potentially never complete when the condition
passed to it made no progress (either because it fall into an
infinite loop or waiting for a resource or response over
network).

Fixes #7494

Signed-off-by: Alexei Barantsev <[email protected]>

java/client/src/org/openqa/selenium/support/ui/FluentWait.java

@@ -28,10 +28,11 @@
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
-import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -203,44 +204,70 @@ public FluentWait<T> ignoring(Class<? extends Throwable> firstType,
    */
   @Override
   public <V> V until(Function<? super T, V> isTrue) {
-    Instant end = clock.instant().plus(timeout);
-
-    Throwable lastException;
-    while (true) {
-      try {
-        V value = isTrue.apply(input);
-        if (value != null && (Boolean.class != value.getClass() || Boolean.TRUE.equals(value))) {
-          return value;
+    try {
+      return CompletableFuture.supplyAsync(checkConditionInLoop(isTrue))
+          .get(deriveSafeTimeout(), TimeUnit.MILLISECONDS);
+    } catch (ExecutionException cause) {
+      if (cause.getCause() instanceof RuntimeException) {
+        throw (RuntimeException) cause.getCause();
+      } else if (cause.getCause() instanceof Error) {
+        throw (Error) cause.getCause();
+      }
+
+      throw new RuntimeException(cause);
+    } catch (InterruptedException cause) {
+      throw new RuntimeException(cause);
+    } catch (java.util.concurrent.TimeoutException cause) {
+      throw new TimeoutException("Supplied function might have stalled", cause);
+    }
+  }
+
+  private <V> Supplier<V> checkConditionInLoop(Function<? super T, V> isTrue) {
+    return () -> {
+      Instant end = clock.instant().plus(timeout);
+
+      Throwable lastException;
+      while (true) {
+        //noinspection ProhibitedExceptionCaught
+        try {
+          V value = isTrue.apply(input);
+          if (value != null && (Boolean.class != value.getClass() || Boolean.TRUE.equals(value))) {
+            return value;
+          }
+
+          // Clear the last exception; if another retry or timeout exception would
+          // be caused by a false or null value, the last exception is not the
+          // cause of the timeout.
+          lastException = null;
+        } catch (Throwable e) {
+          lastException = propagateIfNotIgnored(e);
         }
 
-        // Clear the last exception; if another retry or timeout exception would
-        // be caused by a false or null value, the last exception is not the
-        // cause of the timeout.
-        lastException = null;
-      } catch (Throwable e) {
-        lastException = propagateIfNotIgnored(e);
-      }
+        // Check the timeout after evaluating the function to ensure conditions
+        // with a zero timeout can succeed.
+        if (end.isBefore(clock.instant())) {
+          String message = messageSupplier != null ? messageSupplier.get() : null;
 
-      // Check the timeout after evaluating the function to ensure conditions
-      // with a zero timeout can succeed.
-      if (end.isBefore(clock.instant())) {
-        String message = messageSupplier != null ?
-                         messageSupplier.get() : null;
-
-        String timeoutMessage = String.format(
-            "Expected condition failed: %s (tried for %d second(s) with %d milliseconds interval)",
-            message == null ? "waiting for " + isTrue : message,
-            timeout.getSeconds(), interval.toMillis());
-        throw timeoutException(timeoutMessage, lastException);
-      }
+          String timeoutMessage = String.format(
+              "Expected condition failed: %s (tried for %d second(s) with %d milliseconds interval)",
+              message == null ? "waiting for " + isTrue : message,
+              timeout.getSeconds(), interval.toMillis());
+          throw timeoutException(timeoutMessage, lastException);
+        }
 
-      try {
-        sleeper.sleep(interval);
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-        throw new WebDriverException(e);
+        try {
+          sleeper.sleep(interval);
+        } catch (InterruptedException e) {
+          Thread.currentThread().interrupt();
+          throw new WebDriverException(e);
+        }
       }
-    }
+    };
+  }
+
+  /** This timeout is somewhat arbitrary.  */
+  private long deriveSafeTimeout() {
+    return this.timeout.toMillis() + this.interval.toMillis();
   }
 
   private Throwable propagateIfNotIgnored(Throwable e) {
@@ -265,33 +292,4 @@ private Throwable propagateIfNotIgnored(Throwable e) {
   protected RuntimeException timeoutException(String message, Throwable lastException) {
     throw new TimeoutException(message, lastException);
   }
-
-  /**
-   * Converts the {@code TimeUnit} to the equivalent {@code ChronoUnit}.
-   *
-   * This is a backport from Java 9, see https://bugs.openjdk.java.net/browse/JDK-8141452.
-   *
-   * @param timeUnit the TimeUnit to convert
-   * @return the converted equivalent ChronoUnit
-   */
-  private ChronoUnit toChronoUnit(TimeUnit timeUnit) {
-    switch (timeUnit) {
-      case NANOSECONDS:
-        return ChronoUnit.NANOS;
-      case MICROSECONDS:
-        return ChronoUnit.MICROS;
-      case MILLISECONDS:
-        return ChronoUnit.MILLIS;
-      case SECONDS:
-        return ChronoUnit.SECONDS;
-      case MINUTES:
-        return ChronoUnit.MINUTES;
-      case HOURS:
-        return ChronoUnit.HOURS;
-      case DAYS:
-        return ChronoUnit.DAYS;
-      default:
-        throw new IllegalArgumentException("No ChronoUnit equivalent for " + timeUnit);
-    }
-  }
 }

java/client/test/org/openqa/selenium/support/ui/FluentWaitTest.java

@@ -256,6 +256,25 @@ protected RuntimeException timeoutException(String message, Throwable lastExcept
         .satisfies(expected -> assertThat(sentinelException).isSameAs(expected));
   }
 
+  @Test
+  public void timeoutWhenConditionMakesNoProgress() {
+
+    when(mockClock.instant()).thenReturn(EPOCH, EPOCH.plusMillis(2500));
+    when(mockCondition.apply(mockDriver)).then(invocation -> {
+      while (true) {
+        // it gets into an endless loop and makes no progress.
+      }
+    });
+
+    FluentWait<WebDriver> wait = new FluentWait<>(mockDriver, mockClock, mockSleeper)
+        .withTimeout(Duration.ofSeconds(0))
+        .pollingEvery(Duration.ofSeconds(2));
+
+    assertThatExceptionOfType(org.openqa.selenium.TimeoutException.class)
+        .isThrownBy(() -> wait.until(mockCondition))
+        .satisfies(actual -> assertThat(actual.getMessage()).startsWith("Supplied function might have stalled"));
+  }
+
   private static class TestException extends RuntimeException {
 
   }