diff --git a/components/autofill_assistant/browser/actions/action_delegate_util_unittest.cc b/components/autofill_assistant/browser/actions/action_delegate_util_unittest.cc index 1bdda0d5f70e16..aa0ec92a36273f 100644 --- a/components/autofill_assistant/browser/actions/action_delegate_util_unittest.cc +++ b/components/autofill_assistant/browser/actions/action_delegate_util_unittest.cc @@ -216,7 +216,7 @@ TEST_F(ActionDelegateUtilTest, PerformWithPasswordManagerValue) { auto element = std::make_unique(); content::WebContentsTester::For(web_contents_.get()) ->NavigateAndCommit(GURL("https://www.example.com")); - element->container_frame_host = web_contents_->GetMainFrame(); + element->SetRenderFrameHost(web_contents_->GetMainFrame()); user_data_.selected_login_ = absl::make_optional( GURL("https://www.example.com"), "username"); @@ -239,8 +239,7 @@ TEST_F(ActionDelegateUtilTest, PerformWithPasswordManagerValue) { TEST_F(ActionDelegateUtilTest, PerformWithFailingPasswordManagerValue) { auto element = std::make_unique(); - - element->container_frame_host = web_contents_->GetMainFrame(); + element->SetRenderFrameHost(web_contents_->GetMainFrame()); user_data_.selected_login_ = absl::make_optional( GURL("https://www.example.com"), "username"); diff --git a/components/autofill_assistant/browser/actions/get_element_status_action_unittest.cc b/components/autofill_assistant/browser/actions/get_element_status_action_unittest.cc index 4a46d599b62352..0eaed9fc4b1181 100644 --- a/components/autofill_assistant/browser/actions/get_element_status_action_unittest.cc +++ b/components/autofill_assistant/browser/actions/get_element_status_action_unittest.cc @@ -702,7 +702,7 @@ TEST_F(GetElementStatusActionTest, SucceedsWithPasswordManagerValue) { .WillOnce(WithArgs<1>([this](auto&& callback) { std::unique_ptr element = std::make_unique(); - element->container_frame_host = web_contents_->GetMainFrame(); + element->SetRenderFrameHost(web_contents_->GetMainFrame()); std::move(callback).Run(OkClientStatus(), std::move(element)); })); EXPECT_CALL(mock_website_login_manager_, GetPasswordForLogin(_, _)) diff --git a/components/autofill_assistant/browser/actions/send_keystroke_events_action.cc b/components/autofill_assistant/browser/actions/send_keystroke_events_action.cc index 840ed396e393f3..113aa1ee3b559b 100644 --- a/components/autofill_assistant/browser/actions/send_keystroke_events_action.cc +++ b/components/autofill_assistant/browser/actions/send_keystroke_events_action.cc @@ -41,14 +41,14 @@ void SendKeystrokeEventsAction::InternalProcessAction( // When dealing with a password field we also want to know the last time that // a user has used it. auto selected_login_opt = delegate_->GetUserData()->selected_login_; + auto* target_render_frame_host = element_.render_frame_host(); if (proto_.send_keystroke_events().value().value_case() == TextValue::kPasswordManagerValue && - selected_login_opt) { + selected_login_opt && target_render_frame_host) { // Origin check is done in PWM based on the - // |target_element.container_frame_host->GetLastCommittedURL()| - selected_login_opt->origin = - element_.container_frame_host->GetLastCommittedURL() - .DeprecatedGetOriginAsURL(); + // |target_render_frame_host->GetLastCommittedURL()| + selected_login_opt->origin = target_render_frame_host->GetLastCommittedURL() + .DeprecatedGetOriginAsURL(); delegate_->GetWebsiteLoginManager()->GetGetLastTimePasswordUsed( *selected_login_opt, diff --git a/components/autofill_assistant/browser/actions/send_keystroke_events_action_unittest.cc b/components/autofill_assistant/browser/actions/send_keystroke_events_action_unittest.cc index 0470cd0c611375..0779f1cf49f24d 100644 --- a/components/autofill_assistant/browser/actions/send_keystroke_events_action_unittest.cc +++ b/components/autofill_assistant/browser/actions/send_keystroke_events_action_unittest.cc @@ -119,7 +119,7 @@ TEST_F(SendKeystrokeEventsActionTest, PasswordTextValueReturnLastTimeUsed) { content::WebContentsTester::For(web_contents_.get()) ->NavigateAndCommit(GURL(kUrl)); element.dom_object.object_data.object_id = "id"; - element.container_frame_host = web_contents_->GetMainFrame(); + element.SetRenderFrameHost(web_contents_->GetMainFrame()); mock_action_delegate_.GetElementStore()->AddElement("e", element.dom_object); EXPECT_CALL(mock_web_controller_, @@ -159,7 +159,7 @@ TEST_F(SendKeystrokeEventsActionTest, content::WebContentsTester::For(web_contents_.get()) ->NavigateAndCommit(GURL(kUrl)); element.dom_object.object_data.object_id = "id"; - element.container_frame_host = web_contents_->GetMainFrame(); + element.SetRenderFrameHost(web_contents_->GetMainFrame()); mock_action_delegate_.GetElementStore()->AddElement("e", element.dom_object); EXPECT_CALL(mock_web_controller_, diff --git a/components/autofill_assistant/browser/user_data_util.cc b/components/autofill_assistant/browser/user_data_util.cc index ffe230609bde46..0a175dab333cfb 100644 --- a/components/autofill_assistant/browser/user_data_util.cc +++ b/components/autofill_assistant/browser/user_data_util.cc @@ -606,7 +606,8 @@ void GetPasswordManagerValue( std::move(callback).Run(ClientStatus(PRECONDITION_FAILED), std::string()); return; } - if (!target_element.container_frame_host) { + auto* target_render_frame_host = target_element.render_frame_host(); + if (!target_render_frame_host) { std::move(callback).Run(ClientStatus(PASSWORD_ORIGIN_MISMATCH), std::string()); return; @@ -616,8 +617,8 @@ void GetPasswordManagerValue( case PasswordManagerValue::PASSWORD: { auto login = *user_data->selected_login_; // Origin check is done in PWM based on the - // |target_element.container_frame_host->GetLastCommittedURL()| - login.origin = target_element.container_frame_host->GetLastCommittedURL() + // |target_render_frame_host->GetLastCommittedURL()| + login.origin = target_render_frame_host->GetLastCommittedURL() .DeprecatedGetOriginAsURL(); website_login_manager->GetPasswordForLogin( login, base::BindOnce(&OnGetStoredPassword, std::move(callback))); diff --git a/components/autofill_assistant/browser/user_data_util_unittest.cc b/components/autofill_assistant/browser/user_data_util_unittest.cc index 7703e8cfcfd817..80bae5f441bd33 100644 --- a/components/autofill_assistant/browser/user_data_util_unittest.cc +++ b/components/autofill_assistant/browser/user_data_util_unittest.cc @@ -1261,7 +1261,7 @@ TEST_F(UserDataUtilTextValueTest, GetUsername) { GURL("https://www.example.com"), "username"); ElementFinder::Result element; - element.container_frame_host = web_contents_->GetMainFrame(); + element.SetRenderFrameHost(web_contents_->GetMainFrame()); PasswordManagerValue password_manager_value; password_manager_value.set_credential_type(PasswordManagerValue::USERNAME); @@ -1279,7 +1279,7 @@ TEST_F(UserDataUtilTextValueTest, GetStoredPassword) { GURL("https://www.example.com"), "username"); ElementFinder::Result element; - element.container_frame_host = web_contents_->GetMainFrame(); + element.SetRenderFrameHost(web_contents_->GetMainFrame()); PasswordManagerValue password_manager_value; password_manager_value.set_credential_type(PasswordManagerValue::PASSWORD); @@ -1301,7 +1301,7 @@ TEST_F(UserDataUtilTextValueTest, GetStoredPasswordFails) { ElementFinder::Result element; content::WebContentsTester::For(web_contents_.get()) ->NavigateAndCommit(GURL("https://www.example.com")); - element.container_frame_host = web_contents_->GetMainFrame(); + element.SetRenderFrameHost(web_contents_->GetMainFrame()); PasswordManagerValue password_manager_value; password_manager_value.set_credential_type(PasswordManagerValue::PASSWORD); @@ -1382,7 +1382,7 @@ TEST_F(UserDataUtilTextValueTest, TextValuePasswordManagerValue) { ElementFinder::Result element; content::WebContentsTester::For(web_contents_.get()) ->NavigateAndCommit(GURL("https://www.example.com")); - element.container_frame_host = web_contents_->GetMainFrame(); + element.SetRenderFrameHost(web_contents_->GetMainFrame()); TextValue text_value; text_value.mutable_password_manager_value()->set_credential_type( diff --git a/components/autofill_assistant/browser/web/click_or_tap_worker.cc b/components/autofill_assistant/browser/web/click_or_tap_worker.cc index f50afdae734e0b..fa5b4cff33c062 100644 --- a/components/autofill_assistant/browser/web/click_or_tap_worker.cc +++ b/components/autofill_assistant/browser/web/click_or_tap_worker.cc @@ -31,7 +31,7 @@ void ClickOrTapWorker::Start(const ElementFinder::Result& element, /* check_interval= */ base::Milliseconds(0), node_frame_id_); element_position_getter_->Start( - element.container_frame_host, element.object_id(), + element.render_frame_host(), element.object_id(), base::BindOnce(&ClickOrTapWorker::OnGetCoordinates, weak_ptr_factory_.GetWeakPtr())); } diff --git a/components/autofill_assistant/browser/web/element_finder.cc b/components/autofill_assistant/browser/web/element_finder.cc index a8bd92cecc7d4e..d9791797517478 100644 --- a/components/autofill_assistant/browser/web/element_finder.cc +++ b/components/autofill_assistant/browser/web/element_finder.cc @@ -159,10 +159,9 @@ void ElementFinder::Start(const Result& start_element, Callback callback) { return; } - if (start_element.container_frame_host == nullptr) { + current_frame_ = start_element.render_frame_host(); + if (current_frame_ == nullptr) { current_frame_ = web_contents_->GetMainFrame(); - } else { - current_frame_ = start_element.container_frame_host; } current_frame_id_ = start_element.node_frame_id(); frame_stack_ = start_element.frame_stack(); @@ -244,7 +243,7 @@ void ElementFinder::ResultFound(const std::string& object_id) { ElementFinder::Result ElementFinder::BuildResult(const std::string& object_id) { Result result; - result.container_frame_host = current_frame_; + result.SetRenderFrameHost(current_frame_); result.dom_object.object_data.object_id = object_id; result.dom_object.object_data.node_frame_id = current_frame_id_; return result; diff --git a/components/autofill_assistant/browser/web/element_finder.h b/components/autofill_assistant/browser/web/element_finder.h index c644b7992653c7..2bf03ce5c1b1d0 100644 --- a/components/autofill_assistant/browser/web/element_finder.h +++ b/components/autofill_assistant/browser/web/element_finder.h @@ -60,8 +60,9 @@ class ElementFinder : public WebControllerWorker { }; // Result is the fully resolved element that can be used without limitations. - // This means that |container_frame_host| has been found and is not nullptr. - struct Result { + // This means that |render_frame_host()| has been found and is not nullptr. + class Result { + public: Result(); ~Result(); Result(const Result&); @@ -72,8 +73,12 @@ class ElementFinder : public WebControllerWorker { DomObjectFrameStack dom_object; - // The render frame host contains the element. - raw_ptr container_frame_host = nullptr; + content::RenderFrameHost* render_frame_host() const { + if (!render_frame_id_) { + return nullptr; + } + return content::RenderFrameHost::FromID(*render_frame_id_); + } const std::string& object_id() const { return dom_object.object_data.object_id; @@ -90,6 +95,17 @@ class ElementFinder : public WebControllerWorker { bool IsEmpty() const { return object_id().empty() && node_frame_id().empty(); } + + void SetRenderFrameHost(content::RenderFrameHost* render_frame_host) { + if (!render_frame_host) { + return; + } + render_frame_id_ = render_frame_host->GetGlobalId(); + } + + private: + // The id of the render frame host that contains the element. + absl::optional render_frame_id_; }; // |web_contents|, |devtools_client| and |user_data| must be valid for the diff --git a/components/autofill_assistant/browser/web/element_store.cc b/components/autofill_assistant/browser/web/element_store.cc index 12658bf48da172..edbfebd53c471f 100644 --- a/components/autofill_assistant/browser/web/element_store.cc +++ b/components/autofill_assistant/browser/web/element_store.cc @@ -44,7 +44,7 @@ ClientStatus ElementStore::RestoreElement( VLOG(1) << __func__ << " failed to resolve frame."; return ClientStatus(CLIENT_ID_RESOLUTION_FAILED); } - out_element->container_frame_host = frame; + out_element->SetRenderFrameHost(frame); return OkClientStatus(); } diff --git a/components/autofill_assistant/browser/web/element_store_unittest.cc b/components/autofill_assistant/browser/web/element_store_unittest.cc index f638f59211d188..9888072ce5de39 100644 --- a/components/autofill_assistant/browser/web/element_store_unittest.cc +++ b/components/autofill_assistant/browser/web/element_store_unittest.cc @@ -87,7 +87,7 @@ TEST_F(ElementStoreTest, GetElementFromStoreWithNoFrameId) { ElementFinder::Result result; EXPECT_EQ(ACTION_APPLIED, element_store_->GetElement("1", &result).proto_status()); - EXPECT_EQ(web_contents_->GetMainFrame(), result.container_frame_host); + EXPECT_EQ(web_contents_->GetMainFrame(), result.render_frame_host()); } TEST_F(ElementStoreTest, AddElementToStoreOverwrites) { diff --git a/components/autofill_assistant/browser/web/fake_element_store.cc b/components/autofill_assistant/browser/web/fake_element_store.cc index 25d4c3cbcf0aec..27181715804537 100644 --- a/components/autofill_assistant/browser/web/fake_element_store.cc +++ b/components/autofill_assistant/browser/web/fake_element_store.cc @@ -25,7 +25,7 @@ ClientStatus FakeElementStore::GetElement( out_element->dom_object = it->second; if (web_contents_ != nullptr) { - out_element->container_frame_host = web_contents_->GetMainFrame(); + out_element->SetRenderFrameHost(web_contents_->GetMainFrame()); } return OkClientStatus(); } diff --git a/components/autofill_assistant/browser/web/web_controller.cc b/components/autofill_assistant/browser/web/web_controller.cc index b8a2b9a9ddbada..a2632e60251096 100644 --- a/components/autofill_assistant/browser/web/web_controller.cc +++ b/components/autofill_assistant/browser/web/web_controller.cc @@ -732,7 +732,7 @@ void WebController::WaitUntilElementIsStable( element.node_frame_id()); auto* ptr = getter.get(); pending_workers_.emplace_back(std::move(getter)); - ptr->Start(element.container_frame_host, element.object_id(), + ptr->Start(element.render_frame_host(), element.object_id(), base::BindOnce(&WebController::OnWaitUntilElementIsStable, weak_ptr_factory_.GetWeakPtr(), ptr, base::TimeTicks::Now(), std::move(callback))); @@ -1048,8 +1048,8 @@ void WebController::OnGetBackendNodeIdForFormAndFieldData( return; } - ContentAutofillDriver* driver = ContentAutofillDriver::GetForRenderFrameHost( - element.container_frame_host); + ContentAutofillDriver* driver = + ContentAutofillDriver::GetForRenderFrameHost(element.render_frame_host()); if (driver == nullptr) { DVLOG(1) << __func__ << " Failed to get the autofill driver."; std::move(callback).Run(UnexpectedErrorStatus(__FILE__, __LINE__), nullptr, diff --git a/components/autofill_assistant/browser/web/web_controller_browsertest.cc b/components/autofill_assistant/browser/web/web_controller_browsertest.cc index 5773d4cd4cb60d..88e7612a4a4161 100644 --- a/components/autofill_assistant/browser/web/web_controller_browsertest.cc +++ b/components/autofill_assistant/browser/web/web_controller_browsertest.cc @@ -602,11 +602,11 @@ class WebControllerBrowserTest : public autofill_assistant::BaseBrowserTest, bool is_main_frame) { if (is_main_frame) { EXPECT_EQ(shell()->web_contents()->GetMainFrame(), - result.container_frame_host); + result.render_frame_host()); EXPECT_EQ(result.frame_stack().size(), 0u); } else { EXPECT_NE(shell()->web_contents()->GetMainFrame(), - result.container_frame_host); + result.render_frame_host()); EXPECT_GE(result.frame_stack().size(), 1u); } EXPECT_FALSE(result.object_id().empty()); @@ -2758,7 +2758,7 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, // This makes the devtools action fail. ElementFinder::Result element; element.dom_object.object_data.node_frame_id = "doesnotexist"; - element.container_frame_host = web_contents()->GetMainFrame(); + element.SetRenderFrameHost(web_contents()->GetMainFrame()); EXPECT_EQ(ELEMENT_POSITION_NOT_FOUND, WaitUntilElementIsStable(element, 10, base::Milliseconds(100)) @@ -3210,9 +3210,9 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, RunElementFinderFromOOPIF) { // Create fake element without object id and frame information only. ElementFinder::Result fake_frame_element; - fake_frame_element.container_frame_host = frame_element.container_frame_host; + fake_frame_element.SetRenderFrameHost(frame_element.render_frame_host()); fake_frame_element.dom_object.object_data.node_frame_id = - frame_element.container_frame_host->GetDevToolsFrameToken().ToString(); + frame_element.render_frame_host()->GetDevToolsFrameToken().ToString(); ClientStatus button_status; ElementFinder::Result button_element;