Commit 8478a8f1 authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Turn IOSFormParser into a static function

IOSFormParser is a class used on iOS to turn a HTML description of
password forms into an internal PasswordForm structure. The class has
no data members and the only method, Parse(), is thus effectively
static.

It was created as a class for two reasons: (1) to allow mocking, and
(2) to enable holding some server-supplied state.

I chatted with dvadym@ and we agreed that once this actually needs
being mocked, we can assess the concrete needs, and perhaps change
some alternative (e.g., providing a fake result for testing) to mocking
which would still require adding a virtual interface and other
boilerplate. Holding the server-supplied state might also likely
end up in PasswordFormManager.

Because it is confusing to create IOSFormParser instances in the
current code and the two concerns above seem addressed, this CL
turns IOSFormParser into a single static function, ParseFormData(),
following the existing pattern found in
components/password_manager/core/browser/import/csv_reader.h.

Bug: 827945
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Idbbd5087f301bb07974364f7b8bd324ed42a0fa5
Reviewed-on: https://chromium-review.googlesource.com/990333
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548740}
parent f3236937
...@@ -307,10 +307,8 @@ void SetFields(const ParseResult& parse_result, PasswordForm* password_form) { ...@@ -307,10 +307,8 @@ void SetFields(const ParseResult& parse_result, PasswordForm* password_form) {
} // namespace } // namespace
IOSFormParser::IOSFormParser() = default; std::unique_ptr<PasswordForm> ParseFormData(const FormData& form_data,
FormParsingMode mode) {
std::unique_ptr<PasswordForm> IOSFormParser::Parse(const FormData& form_data,
FormParsingMode mode) {
FieldPointersVector fields = GetNonCreditCardFields(form_data.fields); FieldPointersVector fields = GetNonCreditCardFields(form_data.fields);
// Skip forms without password fields. // Skip forms without password fields.
......
...@@ -16,17 +16,12 @@ namespace password_manager { ...@@ -16,17 +16,12 @@ namespace password_manager {
enum class FormParsingMode { FILLING, SAVING }; enum class FormParsingMode { FILLING, SAVING };
class IOSFormParser { // Parse DOM information |form_data| into Password Manager' form representation
public: // PasswordForm.
IOSFormParser(); // Return nullptr when parsing is unsuccessful.
std::unique_ptr<autofill::PasswordForm> ParseFormData(
// Parse DOM information |form_data| to Password Manager form representation const autofill::FormData& form_data,
// PasswordForm. FormParsingMode mode);
// Return nullptr when parsing is unsuccessful.
std::unique_ptr<autofill::PasswordForm> Parse(
const autofill::FormData& form_data,
FormParsingMode mode);
};
} // namespace password_manager } // namespace password_manager
......
...@@ -63,8 +63,6 @@ class IOSFormParserTest : public testing::Test { ...@@ -63,8 +63,6 @@ class IOSFormParserTest : public testing::Test {
IOSFormParserTest() {} IOSFormParserTest() {}
protected: protected:
IOSFormParser form_parser_;
void CheckTestData(const std::vector<FormParsingTestCase>& test_cases); void CheckTestData(const std::vector<FormParsingTestCase>& test_cases);
}; };
...@@ -144,7 +142,7 @@ void IOSFormParserTest::CheckTestData( ...@@ -144,7 +142,7 @@ void IOSFormParserTest::CheckTestData(
<< test_case.description << ", parsing mode = " << test_case.description << ", parsing mode = "
<< (mode == FormParsingMode::FILLING ? "Filling" : "Saving")); << (mode == FormParsingMode::FILLING ? "Filling" : "Saving"));
std::unique_ptr<PasswordForm> parsed_form = std::unique_ptr<PasswordForm> parsed_form =
form_parser_.Parse(form_data, mode); ParseFormData(form_data, mode);
const ParseResultIndices& expected_indices = const ParseResultIndices& expected_indices =
mode == FormParsingMode::FILLING ? test_case.fill_result mode == FormParsingMode::FILLING ? test_case.fill_result
......
...@@ -272,8 +272,6 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) { ...@@ -272,8 +272,6 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
AccountSelectFillData fillData_; AccountSelectFillData fillData_;
password_manager::IOSFormParser formParser_;
// The WebState this instance is observing. Will be null after // The WebState this instance is observing. Will be null after
// -webStateDestroyed: has been called. // -webStateDestroyed: has been called.
web::WebState* webState_; web::WebState* webState_;
...@@ -545,7 +543,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) { ...@@ -545,7 +543,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
for (const auto& formData : formsData) { for (const auto& formData : formsData) {
std::unique_ptr<PasswordForm> form = std::unique_ptr<PasswordForm> form =
formParser_.Parse(formData, password_manager::FormParsingMode::FILLING); ParseFormData(formData, password_manager::FormParsingMode::FILLING);
if (form) if (form)
forms->push_back(*form); forms->push_back(*form);
} }
...@@ -582,7 +580,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) { ...@@ -582,7 +580,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
} }
std::unique_ptr<PasswordForm> form = std::unique_ptr<PasswordForm> form =
formParser_.Parse(formData, password_manager::FormParsingMode::SAVING); ParseFormData(formData, password_manager::FormParsingMode::SAVING);
if (!form) { if (!form) {
completionHandler(NO, PasswordForm()); completionHandler(NO, PasswordForm());
return; return;
...@@ -862,7 +860,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) { ...@@ -862,7 +860,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
} }
std::unique_ptr<PasswordForm> form = std::unique_ptr<PasswordForm> form =
formParser_.Parse(formData, password_manager::FormParsingMode::SAVING); ParseFormData(formData, password_manager::FormParsingMode::SAVING);
if (!form) if (!form)
return NO; return NO;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment