[Autofill Assistant] Resolve RFH via frame id instead of name

Previously, the frame's RenderFrameHost (RFH) was selected
based on its name / url. This only works if the site actively
sets the name attribute on the iframe it uses, which was not
done everywhere. Resolving with id seems more stable.
Fallback to name / url is kept in place for the time being.

This fixes the resolution (and thus the autofill) for both local
iframes and OOPIF.

Note: This is a copy of crrev/c/1893876
Refer to that for discussions. The original change could not be
submitted because an unintended somehow irreversible checkin blocked
the submit.

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

components/autofill_assistant/browser/client_status.cc

@@ -113,6 +113,9 @@ std::ostream& operator<<(std::ostream& out,
     case ProcessedActionStatusProto::AUTOFILL_INFO_NOT_AVAILABLE:
       out << "AUTOFILL_INFO_NOT_AVAILABLE";
       break;
+    case ProcessedActionStatusProto::FRAME_HOST_NOT_FOUND:
+      out << "FRAME_HOST_NOT_FOUND";
+      break;
 
       // Intentionally no default case to make compilation fail if a new value
       // was added to the enum but not to this list.

components/autofill_assistant/browser/service.proto

@@ -725,6 +725,9 @@ enum ProcessedActionStatusProto {
   // The requested autofill info (e.g., Chrome password manager login) was not
   // available. It might have been recently deleted.
   AUTOFILL_INFO_NOT_AVAILABLE = 21;
+
+  // An unexpected error occurred during element resolution.
+  FRAME_HOST_NOT_FOUND = 22;
 }
 
 // The pseudo type values come from

components/autofill_assistant/browser/web/element_finder.cc

@@ -166,11 +166,11 @@ void ElementFinder::OnGetDocumentElement(
     return;
   }
 
-  // The frame gets set again when we encounter an iFrame, even an OOPIF.
-  // Setting the correct host frame is handled by
-  // FindCorrespondingRenderFrameHost further down.
-  element_result_->container_frame_host = web_contents_->GetMainFrame();
   element_result_->container_frame_selector_index = index;
+  if (element_result_->container_frame_host == nullptr) {
+    // Don't overwrite results from previous OOPIF passes.
+    element_result_->container_frame_host = web_contents_->GetMainFrame();
+  }
   element_result_->object_id = std::string();
   RecursiveFindElement(object_id, index);
 }
@@ -346,15 +346,20 @@ 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.
+
     backend_ids.emplace_back(node->GetContentDocument()->GetBackendNodeId());
 
     element_result_->container_frame_selector_index = index;
 
-    // Find out the corresponding render frame host through document url and
-    // name.
-    // TODO(crbug.com/806868): Use more attributes to find out the render frame
-    // host if name and document url are not enough to uniquely identify it.
+    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();
@@ -363,21 +368,39 @@ void ElementFinder::OnDescribeNode(
             frame_name = (*attributes)[i + 1];
             break;
           }
-        // Jump two positions since attribute name and value are always paired.
+          // 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(
-          UnexpectedDevtoolsErrorStatus(reply_status, __FILE__, __LINE__));
+      SendResult(ClientStatus(FRAME_HOST_NOT_FOUND));
       return;
     }
   } else if (node->HasFrameId()) {
-    element_result_->container_frame_selector_index = index;
+    // 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());
+
+    if (!element_result_->container_frame_host) {
+      DVLOG(1) << __func__ << " Failed to find corresponding owner frame.";
+      SendResult(ClientStatus(FRAME_HOST_NOT_FOUND));
+      return;
+    }
 
     // Kick off another find element chain to walk down the OOP iFrame.
     devtools_client_->GetRuntime()->Evaluate(
@@ -420,6 +443,17 @@ void ElementFinder::OnResolveNode(
   RecursiveFindElement(result->GetObject()->GetObjectId(), ++index);
 }
 
+content::RenderFrameHost* ElementFinder::FindCorrespondingRenderFrameHost(
+    std::string frame_id) {
+  for (auto* frame : web_contents_->GetAllFrames()) {
+    if (frame->GetDevToolsFrameToken().ToString() == frame_id) {
+      return frame;
+    }
+  }
+
+  return nullptr;
+}
+
 content::RenderFrameHost* ElementFinder::FindCorrespondingRenderFrameHost(
     std::string name,
     std::string document_url) {

components/autofill_assistant/browser/web/element_finder.h

@@ -88,6 +88,8 @@ class ElementFinder : public WebControllerWorker {
                      const DevtoolsClient::ReplyStatus& reply_status,
                      std::unique_ptr<dom::ResolveNodeResult> result);
 
+  content::RenderFrameHost* FindCorrespondingRenderFrameHost(
+      std::string frame_id);
   content::RenderFrameHost* FindCorrespondingRenderFrameHost(
       std::string name,
       std::string document_url);