0

[MPArch] Remove call to Frame::Top in PasswordManager in the renderer

PasswordAutofillAgent tries to figure out if the main frame (in
particular the primary main frame) has finished loading in the renderer.
It does that by accessing render_frame()->GetWebFrame()->Top() and
requesting if the top frame is still loading.

With MPArch the renderer doesn’t know the whole ancestor tree,
so a Fenced Frame cannot access the primary main frame by using
render_frame()->GetWebFrame()->Top().

Looking at the code, |did_stop_loading| is not used by |PasswordManager::OnPasswordFormsRendered| meaningfully. Remove |did_stop_loading| completely.

Bug: 1294378
Change-Id: I9d1ee743d5391aa797c8bbe377609f7ca52209f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708134
Commit-Queue: Liviu Tinta <liviutinta@chromium.org>
Reviewed-by: Christoph Schwering <schwering@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Norge Vizcay <vizcay@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032709}
This commit is contained in:
Liviu Tinta
2022-08-08 20:38:53 +00:00
committed by Chromium LUCI CQ
parent 844597821d
commit 126915223c
17 changed files with 179 additions and 219 deletions

@ -4192,8 +4192,7 @@ class MockPrerenderPasswordManagerDriver
(override));
MOCK_METHOD(void,
PasswordFormsRendered,
(const std::vector<autofill::FormData>& visible_form_data,
bool load_completed),
(const std::vector<autofill::FormData>& visible_form_data),
(override));
MOCK_METHOD(void,
PasswordFormSubmitted,
@ -4259,9 +4258,8 @@ class MockPrerenderPasswordManagerDriver
});
ON_CALL(*this, PasswordFormsRendered)
.WillByDefault(
[this](const std::vector<autofill::FormData>& visible_form_data,
bool load_completed) {
impl_->PasswordFormsRendered(visible_form_data, load_completed);
[this](const std::vector<autofill::FormData>& visible_form_data) {
impl_->PasswordFormsRendered(visible_form_data);
RemoveWaitType(WAIT_FOR_PASSWORD_FORMS::WAIT_FOR_RENDERED);
});
ON_CALL(*this, PasswordFormSubmitted)

@ -30,8 +30,7 @@ void FakeMojoPasswordManagerDriver::PasswordFormsParsed(
}
void FakeMojoPasswordManagerDriver::PasswordFormsRendered(
const std::vector<autofill::FormData>& visible_forms_data,
bool did_stop_loading) {
const std::vector<autofill::FormData>& visible_forms_data) {
called_password_forms_rendered_ = true;
form_data_rendered_ = visible_forms_data;
}

@ -142,8 +142,7 @@ class FakeMojoPasswordManagerDriver
const std::vector<autofill::FormData>& forms_data) override;
void PasswordFormsRendered(
const std::vector<autofill::FormData>& visible_forms_data,
bool did_stop_loading) override;
const std::vector<autofill::FormData>& visible_forms_data) override;
void PasswordFormSubmitted(const autofill::FormData& form_data) override;

@ -106,10 +106,8 @@ interface PasswordManagerDriver {
PasswordFormsParsed(array<FormData> forms_data);
// Notification that initial layout has occurred and the following password
// forms are visible on the page (e.g. not set to display:none.), and whether
// all frames in the page have been rendered.
PasswordFormsRendered(array<FormData> visible_forms_data,
bool did_stop_loading);
// forms are visible on the page (e.g. not set to display:none.).
PasswordFormsRendered(array<FormData> visible_forms_data);
// Notification that this password form was submitted by the user.
PasswordFormSubmitted(FormData form_data);

@ -544,10 +544,10 @@ class PasswordAutofillAgent::DeferringPasswordManagerDriver
void PasswordFormsParsed(const std::vector<FormData>& forms_data) override {
DeferMsg(&mojom::PasswordManagerDriver::PasswordFormsParsed, forms_data);
}
void PasswordFormsRendered(const std::vector<FormData>& visible_forms_data,
bool did_stop_loading) override {
void PasswordFormsRendered(
const std::vector<FormData>& visible_forms_data) override {
DeferMsg(&mojom::PasswordManagerDriver::PasswordFormsRendered,
visible_forms_data, did_stop_loading);
visible_forms_data);
}
void PasswordFormSubmitted(const FormData& form_data) override {
DeferMsg(&mojom::PasswordManagerDriver::PasswordFormSubmitted, form_data);
@ -1329,10 +1329,7 @@ void PasswordAutofillAgent::SendPasswordForms(bool only_visible) {
// Send the PasswordFormsRendered message regardless of whether
// |password_forms_data| is empty. The empty |password_forms_data| are a
// possible signal to the browser that a pending login attempt succeeded.
WebFrame* main_frame = render_frame()->GetWebFrame()->Top();
bool did_stop_loading = !main_frame || !main_frame->IsLoading();
GetPasswordManagerDriver().PasswordFormsRendered(password_forms_data,
did_stop_loading);
GetPasswordManagerDriver().PasswordFormsRendered(password_forms_data);
} else {
// If there is a password field, but the list of password forms is empty for
// some reason, add a dummy form to the list. It will cause a request to the

@ -45,8 +45,7 @@ class FakeContentPasswordManagerDriver : public mojom::PasswordManagerDriver {
const std::vector<autofill::FormData>& form_data) override {}
void PasswordFormsRendered(
const std::vector<autofill::FormData>& visible_forms_data,
bool did_stop_loading) override {}
const std::vector<autofill::FormData>& visible_forms_data) override {}
void PasswordFormSubmitted(const autofill::FormData& form_data) override {}

@ -406,7 +406,7 @@ TEST_F(WebsiteLoginManagerImplTest, SaveSubmittedPasswordNewLogin) {
// The user submits the an entirely new credential.
password_manager_->OnPasswordFormsParsed(&driver_, {form.form_data});
password_manager_->OnPasswordFormsRendered(&driver_, {form.form_data}, true);
password_manager_->OnPasswordFormsRendered(&driver_, {form.form_data});
password_manager_->OnPasswordFormSubmitted(&driver_, form.form_data);
EXPECT_TRUE(password_manager_->GetSubmittedManagerForTest());
EXPECT_TRUE(manager_->ReadyToSaveSubmittedPassword());

@ -265,8 +265,7 @@ void ContentPasswordManagerDriver::PasswordFormsParsed(
}
void ContentPasswordManagerDriver::PasswordFormsRendered(
const std::vector<autofill::FormData>& raw_forms,
bool did_stop_loading) {
const std::vector<autofill::FormData>& raw_forms) {
if (!password_manager::bad_message::CheckFrameNotPrerendering(
render_frame_host_))
return;
@ -280,7 +279,7 @@ void ContentPasswordManagerDriver::PasswordFormsRendered(
for (auto& form : forms)
SetFrameAndFormMetaData(render_frame_host_, form);
GetPasswordManager()->OnPasswordFormsRendered(this, forms, did_stop_loading);
GetPasswordManager()->OnPasswordFormsRendered(this, forms);
}
void ContentPasswordManagerDriver::PasswordFormSubmitted(

@ -124,8 +124,7 @@ class ContentPasswordManagerDriver
void PasswordFormsParsed(
const std::vector<autofill::FormData>& forms_data) override;
void PasswordFormsRendered(
const std::vector<autofill::FormData>& visible_forms_data,
bool did_stop_loading) override;
const std::vector<autofill::FormData>& visible_forms_data) override;
void PasswordFormSubmitted(const autofill::FormData& form_data) override;
void InformAboutUserInput(const autofill::FormData& form_data) override;
void DynamicFormSubmission(autofill::mojom::SubmissionIndicatorEvent

@ -132,8 +132,7 @@ class MockPasswordManager : public PasswordManager {
MOCK_METHOD(void,
OnPasswordFormsRendered,
(PasswordManagerDriver * driver,
const std::vector<autofill::FormData>&,
bool),
const std::vector<autofill::FormData>&),
(override));
MOCK_METHOD(void,
OnPasswordFormSubmitted,
@ -354,9 +353,9 @@ TEST_F(ContentPasswordManagerDriverURLTest, PasswordFormsRendered) {
EXPECT_CALL(password_manager_,
OnPasswordFormsRendered(
_, ElementsAre(FormDataEqualTo(ExpectedFormData())), _));
_, ElementsAre(FormDataEqualTo(ExpectedFormData()))));
driver()->PasswordFormsRendered({form}, false);
driver()->PasswordFormsRendered({form});
}
TEST_F(ContentPasswordManagerDriverURLTest, PasswordFormSubmitted) {

@ -902,8 +902,7 @@ bool PasswordManager::ShouldBlockPasswordForSameOriginButDifferentScheme(
void PasswordManager::OnPasswordFormsRendered(
password_manager::PasswordManagerDriver* driver,
const std::vector<FormData>& visible_forms_data,
bool did_stop_loading) {
const std::vector<FormData>& visible_forms_data) {
CreatePendingLoginManagers(driver, visible_forms_data);
std::unique_ptr<BrowserSavePasswordProgressLogger> logger;
if (password_manager_util::IsLoggingActive(client_)) {
@ -940,16 +939,6 @@ void PasswordManager::OnPasswordFormsRendered(
visible_forms_data_.insert(visible_forms_data_.end(),
visible_forms_data.begin(),
visible_forms_data.end());
if (!did_stop_loading &&
!submitted_manager->GetSubmittedForm()
->form_data.is_gaia_with_skip_save_password_form) {
// |form_data.is_gaia_with_skip_save_password_form| = true means that this
// is a Chrome sign-in page. Chrome sign-in pages are redirected to an empty
// pages, and for some reasons |did_stop_loading| might be false. So
// |did_stop_loading| is ignored for them.
return;
}
if (!driver->IsInPrimaryMainFrame() &&
submitted_manager->driver_id() != driver->GetId()) {
// Frames different from the main frame and the frame of the submitted form

@ -77,8 +77,7 @@ class PasswordManager : public PasswordManagerInterface {
const std::vector<autofill::FormData>& forms_data) override;
void OnPasswordFormsRendered(
PasswordManagerDriver* driver,
const std::vector<autofill::FormData>& visible_forms_data,
bool did_stop_loading) override;
const std::vector<autofill::FormData>& visible_forms_data) override;
void OnPasswordFormSubmitted(PasswordManagerDriver* driver,
const autofill::FormData& form_data) override;
void OnPasswordFormCleared(PasswordManagerDriver* driver,

@ -36,8 +36,7 @@ class PasswordManagerInterface : public FormSubmissionObserver {
// Handles password forms being rendered.
virtual void OnPasswordFormsRendered(
PasswordManagerDriver* driver,
const std::vector<autofill::FormData>& visible_forms_data,
bool did_stop_loading) = 0;
const std::vector<autofill::FormData>& visible_forms_data) = 0;
// Handles a password form being submitted.
virtual void OnPasswordFormSubmitted(PasswordManagerDriver* driver,

File diff suppressed because it is too large Load Diff

@ -577,7 +577,7 @@ NSString* const kSuggestionSuffix = @" ••••••••";
// added elements to the form.
if (!triggeredByFormChange) {
_passwordManager->OnPasswordFormsRendered(_delegate.passwordManagerDriver,
forms, true);
forms);
}
}

@ -54,9 +54,7 @@ class MockPasswordManager : public PasswordManagerInterface {
(override));
MOCK_METHOD(void,
OnPasswordFormsRendered,
(PasswordManagerDriver*,
const std::vector<autofill::FormData>&,
bool),
(PasswordManagerDriver*, const std::vector<autofill::FormData>&),
(override));
MOCK_METHOD(void,
OnPasswordFormSubmitted,

@ -37,8 +37,7 @@ class PasswordManagerDriverFactory::PasswordManagerDriver
void PasswordFormsParsed(
const std::vector<autofill::FormData>& raw_forms_data) override {}
void PasswordFormsRendered(
const std::vector<autofill::FormData>& raw_visible_forms_data,
bool did_stop_loading) override {}
const std::vector<autofill::FormData>& raw_visible_forms_data) override {}
void PasswordFormSubmitted(const autofill::FormData& raw_form_data) override {
}
void InformAboutUserInput(const autofill::FormData& raw_form_data) override {