[Autofill Assistant] Refactor IFrame treatment in element_finder

Refactoring, such that iframes are treated en-bloc, expecting an iframe.
This change is safe, because the main document never reaches this piece
of code in OnDescribeNode.
Adding an assertion that all IFrames have a FrameId (has shown to be
safe so far).

Removing the fallback resolution with name / url.

Bug: b/143318024
Change-Id: I515436e8012ee36727f89acae3e43ae4e95b49b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1901078
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714205}

components/autofill_assistant/browser/web/element_finder.cc

@@ -347,51 +347,9 @@ void ElementFinder::OnDescribeNode(
   auto* node = result->GetNode();
   std::vector<int> backend_ids;
 
-  if (node->HasContentDocument()) {
-    // If the frame has a ContentDocument, it's considered a local frame.
-    // We need to resolve the RenderFrameHost for autofill.
+  if (node->GetNodeName() == "IFRAME") {
+    DCHECK(node->HasFrameId());  // Ensure all frames have an id.
 
-    backend_ids.emplace_back(node->GetContentDocument()->GetBackendNodeId());
-
-    element_result_->container_frame_selector_index = index;
-
-    if (node->HasFrameId()) {
-      element_result_->container_frame_host =
-          FindCorrespondingRenderFrameHost(node->GetFrameId());
-    } else {
-      // TODO(b/143318024): Remove the fallback.
-      std::string frame_name;
-      if (node->HasAttributes()) {
-        const std::vector<std::string>* attributes = node->GetAttributes();
-        for (size_t i = 0; i < attributes->size();) {
-          if ((*attributes)[i] == "name") {
-            frame_name = (*attributes)[i + 1];
-            break;
-          }
-          // Jump two positions since attribute name and value are always
-          // paired.
-          i = i + 2;
-        }
-      }
-      element_result_->container_frame_host = FindCorrespondingRenderFrameHost(
-          frame_name, node->GetContentDocument()->GetDocumentURL());
-    }
-
-    if (!element_result_->container_frame_host) {
-      DVLOG(1) << __func__ << " Failed to find corresponding owner frame.";
-      SendResult(ClientStatus(FRAME_HOST_NOT_FOUND));
-      return;
-    }
-  } else if (node->HasFrameId()) {
-    // If the frame has no ContentDocument, it's considered an
-    // OutOfProcessIFrame.
-    // See https://www.chromium.org/developers/design-documents/oop-iframes for
-    // full documentation.
-    // We need to assign the frame id, such that devtools can resolve the
-    // session calls should be executed on. We also need to resolve the
-    // RenderFrameHost for autofill.
-
-    element_result_->node_frame_id = node->GetFrameId();
     element_result_->container_frame_selector_index = index;
     element_result_->container_frame_host =
         FindCorrespondingRenderFrameHost(node->GetFrameId());
@@ -402,12 +360,29 @@ void ElementFinder::OnDescribeNode(
       return;
     }
 
-    // Kick off another find element chain to walk down the OOP iFrame.
-    devtools_client_->GetRuntime()->Evaluate(
-        std::string(kGetDocumentElement), element_result_->node_frame_id,
-        base::BindOnce(&ElementFinder::OnGetDocumentElement,
-                       weak_ptr_factory_.GetWeakPtr(), index + 1));
-    return;
+    if (node->HasContentDocument()) {
+      // If the frame has a ContentDocument it's considered a local frame. We
+      // don't need to assign the frame id, since devtools can just send the
+      // commands to the main session.
+
+      backend_ids.emplace_back(node->GetContentDocument()->GetBackendNodeId());
+    } else {
+      // If the frame has no ContentDocument, it's considered an
+      // OutOfProcessIFrame.
+      // See https://www.chromium.org/developers/design-documents/oop-iframes
+      // for full documentation.
+      // With the iFrame running in a different process it is necessary to pass
+      // the correct session id from devtools. We need to set the frame id,
+      // such that devtools can resolve the corresponding session id.
+      element_result_->node_frame_id = node->GetFrameId();
+
+      // Kick off another find element chain to walk down the OOP iFrame.
+      devtools_client_->GetRuntime()->Evaluate(
+          std::string(kGetDocumentElement), element_result_->node_frame_id,
+          base::BindOnce(&ElementFinder::OnGetDocumentElement,
+                         weak_ptr_factory_.GetWeakPtr(), index + 1));
+      return;
+    }
   }
 
   if (node->HasShadowRoots()) {
@@ -454,19 +429,4 @@ content::RenderFrameHost* ElementFinder::FindCorrespondingRenderFrameHost(
   return nullptr;
 }
 
-content::RenderFrameHost* ElementFinder::FindCorrespondingRenderFrameHost(
-    std::string name,
-    std::string document_url) {
-  content::RenderFrameHost* ret_frame = nullptr;
-  for (auto* frame : web_contents_->GetAllFrames()) {
-    if (frame->GetFrameName() == name &&
-        frame->GetLastCommittedURL().spec() == document_url) {
-      DCHECK(!ret_frame);
-      ret_frame = frame;
-    }
-  }
-
-  return ret_frame;
-}
-
 }  // namespace autofill_assistant

components/autofill_assistant/browser/web/element_finder.h

@@ -90,9 +90,6 @@ class ElementFinder : public WebControllerWorker {
 
   content::RenderFrameHost* FindCorrespondingRenderFrameHost(
       std::string frame_id);
-  content::RenderFrameHost* FindCorrespondingRenderFrameHost(
-      std::string name,
-      std::string document_url);
 
   content::WebContents* const web_contents_;
   DevtoolsClient* const devtools_client_;

components/autofill_assistant/browser/web/web_controller_browsertest.cc

@@ -908,6 +908,15 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, FindElement) {
   FindElementAndCheck(selector, 1, false);
   selector.must_be_visible = true;
   FindElementAndCheck(selector, 1, false);
+
+  // OutOfProcessIFrame.
+  selector.selectors.clear();
+  selector.selectors.emplace_back("#iframeExternal");
+  selector.selectors.emplace_back("#button");
+  selector.must_be_visible = false;
+  FindElementAndCheck(selector, 1, false);
+  selector.must_be_visible = true;
+  FindElementAndCheck(selector, 1, false);
 }
 
 IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, FindElementNotFound) {