Revert "Fix FluentWait so it completes in more cases."

This reverts commit 51de536.

We decided to revert back to legacy (blocking) implementation because the new one causes issues with clients' code that uses ThreadLocal variables in conditions.

Author: barancev

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

@@ -27,11 +27,10 @@
 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,71 +202,44 @@ public FluentWait<T> ignoring(Class<? extends Throwable> firstType,
    */
   @Override
   public <V> V until(Function<? super T, V> isTrue) {
-    try {
-      return CompletableFuture.supplyAsync(checkConditionInLoop(isTrue))
-          .get(deriveSafeTimeout(), TimeUnit.MILLISECONDS);
-    } catch (ExecutionException cause) {
-      if (cause.getCause() instanceof RuntimeException) {
-        throw (RuntimeException) cause.getCause().fillInStackTrace();
-      } else if (cause.getCause() instanceof Error) {
-        throw (Error) cause.getCause();
-      }
-
-      throw new RuntimeException(cause);
-    } catch (InterruptedException cause) {
-      Thread.currentThread().interrupt();
-      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;
-          }
+    Instant end = clock.instant().plus(timeout);
 
-          // 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);
+    Throwable lastException;
+    while (true) {
+      try {
+        V value = isTrue.apply(input);
+        if (value != null && (Boolean.class != value.getClass() || Boolean.TRUE.equals(value))) {
+          return value;
         }
 
-        // 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;
+        // 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);
+      }
 
-          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);
-        }
+      // 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;
 
-        try {
-          sleeper.sleep(interval);
-        } catch (InterruptedException e) {
-          Thread.currentThread().interrupt();
-          throw new WebDriverException(e);
-        }
+        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);
       }
-    };
-  }
 
-  /** This timeout is somewhat arbitrary.  */
-  private long deriveSafeTimeout() {
-    return this.timeout.toMillis() + this.interval.toMillis();
+      try {
+        sleeper.sleep(interval);
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        throw new WebDriverException(e);
+      }
+    }
   }
 
   private Throwable propagateIfNotIgnored(Throwable e) {

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

@@ -259,25 +259,6 @@ 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 {
 
   }