[Test Automation] Improvements to WaitForStateChange()

Addressing concerns in the attached bug. There are some tests for which navigation can happen at odd times or as the result of asynchronous processes. If only the end state is being tested, allowing a StateChange to not care if the page navigates (or becomes momentarily unavailable) increases the number of tests that can be made reliable and non-flaky.

This CL adds a new field to StateChange, continue_across_navigation. When set, WaitForStateChange() will not immediately fail if the page navigates or becomes momentarily unavailable.

The CL also addresses an issue where many StateChange structs that were declared had parameters that did not match the default StateChange::Type, but worked accidentally due to implementation details of the system.

The type of the change now defaults to a new `kAuto` value which causes auto-inference of the type based on the parameters, since in the vast majority of cases it's obvious from the parameters that are set. This puts less onus on the person writing the test and doesn't force them to set a redundant parameter. It also reflects the old default behavior - in other words, it makes explicit what people were already doing (previously incorrectly, now correct).

Bug: b/303402087
Change-Id: Ife3fcd81b6e3dd637a868e33083fa5037983e735
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4924271
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Tomek Jurkiewicz <tju@google.com>
Cr-Commit-Position: refs/heads/main@{#1209065}

chrome/browser/ui/views/performance_controls/high_efficiency_interactive_ui_test.cc

@@ -228,7 +228,6 @@ IN_PROC_BROWSER_TEST_F(HighEfficiencyDiscardPolicyInteractiveTest,
   StateChange video_is_playing;
   video_is_playing.event = kVideoIsPlaying;
   video_is_playing.where = video;
-  video_is_playing.type = StateChange::Type::kConditionTrue;
   video_is_playing.test_function = kMediaIsPlaying;
 
   RunTestSequence(

chrome/test/interaction/interactive_browser_test.cc

@@ -45,29 +45,6 @@ namespace {
 constexpr ui::InteractionSequence::ContextMode kDefaultWebContentsContextMode =
     ui::InteractionSequence::ContextMode::kAny;
 
-std::string DescribeStateChange(
-    const WebContentsInteractionTestUtil::StateChange& state_change) {
-  std::ostringstream oss;
-  oss << "StateChange{ ";
-  switch (state_change.type) {
-    case WebContentsInteractionTestUtil::StateChange::Type::kDoesNotExist:
-      oss << "does not exist: " << state_change.where << " }";
-      break;
-    case WebContentsInteractionTestUtil::StateChange::Type::kExists:
-      oss << "exists: " << state_change.where << " }";
-      break;
-    case WebContentsInteractionTestUtil::StateChange::Type::
-        kExistsAndConditionTrue:
-      oss << "exists: " << state_change.where << " and condition true:\n"
-          << state_change.test_function << "\n}";
-      break;
-    case WebContentsInteractionTestUtil::StateChange::Type::kConditionTrue:
-      oss << "condition true:\n" << state_change.test_function << "\n}";
-      break;
-  }
-  return oss.str();
-}
-
 // Matcher that determines whether a particular value is truthy.
 class IsTruthyMatcher : public testing::MatcherInterface<const base::Value&> {
  public:
@@ -377,28 +354,32 @@ InteractiveBrowserTestApi::WaitForStateChange(
   ui::CustomElementEventType event_type =
       expect_timeout ? state_change.timeout_event : state_change.event;
   CHECK(event_type);
-  const auto desc = base::StringPrintf(
-      "WaitForStateChange( %s, %s )", DescribeStateChange(state_change).c_str(),
-      expect_timeout ? "true" : "false");
+  std::ostringstream desc;
+  desc << "WaitForStateChange( " << state_change << ", "
+       << (expect_timeout ? "true" : "false") << " )";
+  const bool fail_on_close = !state_change.continue_across_navigation;
   return Steps(
       std::move(StepBuilder()
-                    .SetDescription(base::StrCat({desc, ": Queue Event"}))
+                    .SetDescription(base::StrCat({desc.str(), ": Queue Event"}))
                     .SetElementID(webcontents_id)
                     .SetContext(kDefaultWebContentsContextMode)
+                    .SetMustRemainVisible(fail_on_close)
                     .SetStartCallback(base::BindOnce(
                         [](StateChange state_change, ui::TrackedElement* el) {
                           el->AsA<TrackedElementWebContents>()
                               ->owner()
                               ->SendEventOnStateChange(state_change);
                         },
                         state_change))),
-      std::move(StepBuilder()
-                    .SetDescription(base::StrCat({desc, ": Wait For Event"}))
-                    .SetElementID(webcontents_id)
-                    .SetContext(
-                        ui::InteractionSequence::ContextMode::kFromPreviousStep)
-                    .SetType(ui::InteractionSequence::StepType::kCustomEvent,
-                             event_type)));
+      std::move(
+          StepBuilder()
+              .SetDescription(base::StrCat({desc.str(), ": Wait For Event"}))
+              .SetElementID(webcontents_id)
+              .SetContext(
+                  ui::InteractionSequence::ContextMode::kFromPreviousStep)
+              .SetType(ui::InteractionSequence::StepType::kCustomEvent,
+                       event_type)
+              .SetMustBeVisibleAtStart(fail_on_close)));
 }
 
 // static

chrome/test/interaction/interactive_browser_test_browsertest.cc

@@ -13,6 +13,7 @@
 #include "chrome/test/base/test_switches.h"
 #include "content/public/test/browser_test.h"
 #include "ui/base/interaction/element_identifier.h"
+#include "ui/base/interaction/element_tracker.h"
 #include "ui/base/interaction/expect_call_in_scope.h"
 #include "ui/base/interaction/interaction_sequence.h"
 #include "url/gurl.h"
@@ -511,6 +512,53 @@ IN_PROC_BROWSER_TEST_F(InteractiveBrowserTestBrowsertest, ScrollIntoView) {
                   CheckJsResultAt(kTabId, kText, kElementIsInViewport, true));
 }
 
+IN_PROC_BROWSER_TEST_F(InteractiveBrowserTestBrowsertest,
+                       WaitForStateChangeAcrossNavigation) {
+  DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kTabId);
+  DEFINE_LOCAL_CUSTOM_ELEMENT_EVENT_TYPE(kFoundElementEvent);
+  const GURL url1 = embedded_test_server()->GetURL(kDocumentWithLinks);
+  const GURL url2 = embedded_test_server()->GetURL(kDocumentWithNamedElement);
+
+  StateChange state_change;
+  state_change.type = StateChange::Type::kExists;
+  state_change.where = {"#select"};
+  state_change.continue_across_navigation = true;
+  state_change.event = kFoundElementEvent;
+
+  RunTestSequence(
+      InstrumentTab(kTabId),
+      // This is needed to prevent subsequent navigation from causing the
+      // previous step to fail due to the element immediately losing visibility.
+      FlushEvents(),
+      InParallel(Steps(NavigateWebContents(kTabId, url1),
+                       NavigateWebContents(kTabId, url2)),
+                 WaitForStateChange(kTabId, state_change)));
+}
+
+IN_PROC_BROWSER_TEST_F(InteractiveBrowserTestBrowsertest,
+                       WaitForStateChangeWithConditionAcrossNavigation) {
+  DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kTabId);
+  DEFINE_LOCAL_CUSTOM_ELEMENT_EVENT_TYPE(kFoundElementEvent);
+  const GURL url1 = embedded_test_server()->GetURL(kDocumentWithLinks);
+  const GURL url2 = embedded_test_server()->GetURL(kDocumentWithNamedElement);
+
+  StateChange state_change;
+  state_change.type = StateChange::Type::kExistsAndConditionTrue;
+  state_change.where = {"#select option[selected]"};
+  state_change.test_function = "(el) => (el.innerText === 'Apple')";
+  state_change.continue_across_navigation = true;
+  state_change.event = kFoundElementEvent;
+
+  RunTestSequence(
+      InstrumentTab(kTabId),
+      // This is needed to prevent subsequent navigation from causing the
+      // previous step to fail due to the element immediately losing visibility.
+      FlushEvents(),
+      InParallel(Steps(NavigateWebContents(kTabId, url1),
+                       NavigateWebContents(kTabId, url2)),
+                 WaitForStateChange(kTabId, state_change)));
+}
+
 // Parameter for WebUI coverage tests.
 struct CoverageConfig {
   // Whether to set the --devtools-code-coverage flag. If it's not set, nothing