[CEX] Use dummy WebContents to execute JS flow

With the previous implementation the flow would stop/crash when a navigation happened. Using a dummy WebContents makes sure that the flow is isolated from events like this on the web page.

(cherry picked from commit 248461b)

Bug: 1329059
Change-Id: If03ad0dd45340ae2548f9712ed0287bd772e6e6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3663243
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Commit-Queue: Florian Gauger <fga@google.com>
Auto-Submit: Florian Gauger <fga@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1007340}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3679544
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#419}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}

components/autofill_assistant/browser/actions/js_flow_action.cc

@@ -26,9 +26,9 @@ const char kJsFlowActionEnabledGroup[] = "Enabled";
 
 JsFlowAction::JsFlowAction(ActionDelegate* delegate, const ActionProto& proto)
     : Action(delegate, proto),
-      js_flow_executor_(
-          std::make_unique<JsFlowExecutorImpl>(delegate->GetWebContents(),
-                                               this)) {
+      js_flow_executor_(std::make_unique<JsFlowExecutorImpl>(
+          delegate->GetWebContents()->GetBrowserContext(),
+          this)) {
   DCHECK(proto_.has_js_flow());
 }
 

components/autofill_assistant/browser/js_flow_executor_impl.cc

@@ -14,6 +14,9 @@
 #include "components/autofill_assistant/browser/js_flow_util.h"
 #include "components/autofill_assistant/browser/parse_jspb.h"
 #include "components/autofill_assistant/browser/web/web_controller_util.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/web_contents.h"
 
 namespace autofill_assistant {
 namespace {
@@ -115,14 +118,21 @@ absl::optional<std::string> ConvertActionToBytes(const base::Value* action,
 
 }  // namespace
 
-JsFlowExecutorImpl::JsFlowExecutorImpl(content::WebContents* web_contents,
+JsFlowExecutorImpl::JsFlowExecutorImpl(content::BrowserContext* browser_context,
                                        Delegate* delegate)
     : delegate_(delegate),
+      // To execute the JS flow we create a dummy WebContents.
+      dummy_web_contents_(content::WebContents::Create(
+          content::WebContents::CreateParams(browser_context))),
       devtools_client_(std::make_unique<DevtoolsClient>(
-          content::DevToolsAgentHost::GetOrCreateFor(web_contents),
+          content::DevToolsAgentHost::GetOrCreateFor(dummy_web_contents_.get()),
           base::FeatureList::IsEnabled(
               autofill_assistant::features::
-                  kAutofillAssistantFullJsFlowStackTraces))) {}
+                  kAutofillAssistantFullJsFlowStackTraces))) {
+  // Navigate to a blank page to connect to a frame tree.
+  dummy_web_contents_->GetController().LoadURLWithParams(
+      content::NavigationController::LoadURLParams(GURL("about:blank")));
+}
 
 JsFlowExecutorImpl::~JsFlowExecutorImpl() = default;
 
@@ -387,6 +397,8 @@ void JsFlowExecutorImpl::RunCallback(
   if (!status.ok() && result_value) {
     VLOG(1) << "Flow failed with " << status
             << " and result: " << *result_value;
+  } else if (!status.ok()) {
+    VLOG(1) << "Flow failed with " << status;
   }
 
   std::move(callback_).Run(status, std::move(result_value));

components/autofill_assistant/browser/js_flow_executor_impl.h

@@ -21,7 +21,8 @@ namespace autofill_assistant {
 class JsFlowExecutorImpl : public JsFlowExecutor {
  public:
   // |delegate| must outlive the JsFlowExecutorImpl.
-  JsFlowExecutorImpl(content::WebContents* web_contents, Delegate* delegate);
+  JsFlowExecutorImpl(content::BrowserContext* browser_context,
+                     Delegate* delegate);
   ~JsFlowExecutorImpl() override;
   JsFlowExecutorImpl(const JsFlowExecutorImpl&) = delete;
   JsFlowExecutorImpl& operator=(const JsFlowExecutorImpl&) = delete;
@@ -119,6 +120,7 @@ class JsFlowExecutorImpl : public JsFlowExecutor {
   }
 
   Delegate* const delegate_;
+  std::unique_ptr<content::WebContents> dummy_web_contents_;
   std::unique_ptr<DevtoolsClient> devtools_client_;
   int isolated_world_context_id_ = -1;
 

components/autofill_assistant/browser/js_flow_executor_impl_browsertest.cc

@@ -34,6 +34,7 @@
 #include "components/autofill_assistant/browser/trigger_context.h"
 #include "components/autofill_assistant/browser/web/mock_web_controller.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/test_browser_context.h"
 #include "content/shell/browser/shell.h"
 #include "net/http/http_status_code.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -75,12 +76,7 @@ class MockJsFlowExecutorImplDelegate : public JsFlowExecutorImpl::Delegate {
 
 class JsFlowExecutorImplTest : public BaseBrowserTest {
  public:
-  void SetUpOnMainThread() override {
-    BaseBrowserTest::SetUpOnMainThread();
-
-    flow_executor_ = std::make_unique<JsFlowExecutorImpl>(
-        shell()->web_contents(), &mock_delegate_);
-  }
+  void SetUpOnMainThread() override { BaseBrowserTest::SetUpOnMainThread(); }
 
   // Overload, ignore result value, just return the client status.
   ClientStatus RunTest(const std::string& js_flow) {
@@ -92,7 +88,13 @@ class JsFlowExecutorImplTest : public BaseBrowserTest {
                        std::unique_ptr<base::Value>& result_value) {
     ClientStatus status;
     base::RunLoop run_loop;
-    flow_executor_->Start(
+
+    // Needs to be created inside the RunLoop since we are creating a dummy
+    // WebContents and navigating to a new url.
+    JsFlowExecutorImpl flow_executor = JsFlowExecutorImpl(
+        shell()->web_contents()->GetBrowserContext(), &mock_delegate_);
+
+    flow_executor.Start(
         js_flow, base::BindOnce(&JsFlowExecutorImplTest::OnFlowFinished,
                                 base::Unretained(this), run_loop.QuitClosure(),
                                 &status, std::ref(result_value)));
@@ -112,7 +114,6 @@ class JsFlowExecutorImplTest : public BaseBrowserTest {
 
  protected:
   NiceMock<MockJsFlowExecutorImplDelegate> mock_delegate_;
-  std::unique_ptr<JsFlowExecutorImpl> flow_executor_;
 };
 
 IN_PROC_BROWSER_TEST_F(JsFlowExecutorImplTest, SmokeTest) {
@@ -387,37 +388,6 @@ IN_PROC_BROWSER_TEST_F(JsFlowExecutorImplTest,
   EXPECT_EQ(status.proto_status(), INVALID_ACTION);
 }
 
-IN_PROC_BROWSER_TEST_F(JsFlowExecutorImplTest, StartWhileAlreadyRunningFails) {
-  EXPECT_CALL(mock_delegate_, RunNativeAction)
-      .WillOnce(WithArg<2>([&](auto callback) {
-        // Starting a second flow while the first one is running should fail.
-        EXPECT_EQ(RunTest(std::string()).proto_status(), INVALID_ACTION);
-
-        // The first flow should be able to finish successfully.
-        std::move(callback).Run(ClientStatus(ACTION_APPLIED), nullptr);
-      }));
-
-  std::unique_ptr<base::Value> result;
-  ClientStatus status = RunTest(
-      R"(
-      let [status, result] = await runNativeAction(1, "dGVzdA==" /*test*/);
-      return status;
-      )",
-      result);
-  EXPECT_EQ(status.proto_status(), ACTION_APPLIED);
-  EXPECT_EQ(*result, base::Value(2));
-}
-
-IN_PROC_BROWSER_TEST_F(JsFlowExecutorImplTest,
-                       EnvironmentIsPreservedBetweenRuns) {
-  EXPECT_EQ(RunTest("globalFlowState.i = 5;").proto_status(), ACTION_APPLIED);
-
-  std::unique_ptr<base::Value> result;
-  EXPECT_EQ(RunTest("return globalFlowState.i;", result).proto_status(),
-            ACTION_APPLIED);
-  EXPECT_EQ(*result, base::Value(5));
-}
-
 class JsFlowExecutorImplScriptExecutorTest : public BaseBrowserTest {
  public:
   void SetUpOnMainThread() override {