Don't use FormFieldData only as a key in a map

The FormFieldData was used in PasswordAutofillManagera::LoginToPasswordInfoMap merely as a map key, without needing the information in it. This CL changes the key type to int, and removes one IPC message completely in result.

Note: this CL is also relevant to http://crbug.com/377422 and http://crbug.com/118601, but does not directly contribute to their goals.

BUG=400186

Review URL: https://codereview.chromium.org/686653003

Cr-Commit-Position: refs/heads/master@{#302790}

9 files changed, 109 insertions, 98 deletions

diff --git a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
index d4362b3..d9c08c9 100644
--- a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
+++ b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
@@ -36,6 +36,8 @@ using blink::WebView;
 
 namespace {
 
+const int kPasswordFillFormDataId = 1234;
+
 // The name of the username/password element in the form.
 const char kUsernameName[] = "username";
 const char kPasswordName[] = "password";
@@ -175,7 +177,7 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
   // protected.
   void SimulateOnFillPasswordForm(
       const PasswordFormFillData& fill_data) {
-    AutofillMsg_FillPasswordForm msg(0, fill_data);
+    AutofillMsg_FillPasswordForm msg(0, kPasswordFillFormDataId, fill_data);
     static_cast<content::RenderViewObserver*>(password_autofill_agent_)
         ->OnMessageReceived(msg);
   }
@@ -394,13 +396,13 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
         render_thread_->sink().GetFirstMessageMatching(
             AutofillHostMsg_ShowPasswordSuggestions::ID);
     EXPECT_TRUE(message);
-    Tuple4<autofill::FormFieldData, base::string16, bool, gfx::RectF> args;
+    Tuple5<int, base::i18n::TextDirection, base::string16, bool, gfx::RectF>
+        args;
     AutofillHostMsg_ShowPasswordSuggestions::Read(message, &args);
     EXPECT_EQ(2u, fill_data_.basic_data.fields.size());
-    EXPECT_EQ(fill_data_.basic_data.fields[0].name, args.a.name);
-    EXPECT_EQ(ASCIIToUTF16(username), args.a.value);
-    EXPECT_EQ(ASCIIToUTF16(username), args.b);
-    EXPECT_EQ(show_all, args.c);
+    EXPECT_EQ(kPasswordFillFormDataId, args.a);
+    EXPECT_EQ(ASCIIToUTF16(username), args.c);
+    EXPECT_EQ(show_all, args.d);
 
     render_thread_->sink().ClearMessages();
   }
diff --git a/components/autofill/content/common/autofill_messages.h b/components/autofill/content/common/autofill_messages.h
index e339a64..57b6a9a 100644
--- a/components/autofill/content/common/autofill_messages.h
+++ b/components/autofill/content/common/autofill_messages.h
@@ -112,8 +112,11 @@ IPC_MESSAGE_ROUTED2(AutofillMsg_PreviewForm,
 
 // Fill a password form and prepare field autocomplete for multiple
 // matching logins. Lets the renderer know if it should disable the popup
-// because the browser process will own the popup UI.
-IPC_MESSAGE_ROUTED1(AutofillMsg_FillPasswordForm,
+// because the browser process will own the popup UI. |key| serves for
+// identifying the fill form data in subsequent
+// AutofillHostMsg_ShowPasswordSuggestions messages to the browser.
+IPC_MESSAGE_ROUTED2(AutofillMsg_FillPasswordForm,
+                    int /* key */,
                     autofill::PasswordFormFillData /* the fill form data*/)
 
 // Notification to start (|active| == true) or stop (|active| == false) logging
@@ -280,14 +283,12 @@ IPC_MESSAGE_ROUTED2(AutofillHostMsg_ShowPasswordEditingPopup,
 // Instructs the browser to hide any password generation popups.
 IPC_MESSAGE_ROUTED0(AutofillHostMsg_HidePasswordGenerationPopup)
 
-// Instruct the browser that a password mapping has been found for a field.
-IPC_MESSAGE_ROUTED2(AutofillHostMsg_AddPasswordFormMapping,
-                    autofill::FormFieldData, /* the user name field */
-                    autofill::PasswordFormFillData /* password pairings */)
-
-// Instruct the browser to show a popup with suggestions for the form field.
-IPC_MESSAGE_ROUTED4(AutofillHostMsg_ShowPasswordSuggestions,
-                    autofill::FormFieldData /* the form field */,
+// Instruct the browser to show a popup with suggestions filled from data
+// associated with |key|. The popup will use |text_direction| for displaying
+// text.
+IPC_MESSAGE_ROUTED5(AutofillHostMsg_ShowPasswordSuggestions,
+                    int /* key */,
+                    base::i18n::TextDirection /*text_direction */,
                     base::string16 /* username typed by user */,
                     bool /* show all suggestions */,
                     gfx::RectF /* input field bounds, window-relative */)
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 3c6626f..7af454a 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -1002,6 +1002,7 @@ void PasswordAutofillAgent::DidStartProvisionalLoad(
 }
 
 void PasswordAutofillAgent::OnFillPasswordForm(
+    int key,
     const PasswordFormFillData& form_data) {
   if (usernames_usage_ == NOTHING_TO_AUTOFILL) {
     if (form_data.other_possible_usernames.size())
@@ -1058,16 +1059,7 @@ void PasswordAutofillAgent::OnFillPasswordForm(
     password_info.password_field = password_element;
     login_to_password_info_[username_element] = password_info;
     password_to_username_[password_element] = username_element;
-
-    FormData form;
-    FormFieldData field;
-    if (form_contains_username_field) {
-      FindFormAndFieldForFormControlElement(
-          username_element, &form, &field, REQUIRE_NONE);
-    }
-
-    Send(new AutofillHostMsg_AddPasswordFormMapping(
-        routing_id(), field, form_data));
+    login_to_password_info_key_[username_element] = key;
   }
 }
 
@@ -1102,13 +1094,18 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
   blink::WebInputElement selected_element = user_input;
   gfx::Rect bounding_box(selected_element.boundsInViewportSpace());
 
+  LoginToPasswordInfoKeyMap::const_iterator key_it =
+      login_to_password_info_key_.find(user_input);
+  DCHECK(key_it != login_to_password_info_key_.end());
+
   float scale = web_view_->pageScaleFactor();
   gfx::RectF bounding_box_scaled(bounding_box.x() * scale,
                                  bounding_box.y() * scale,
                                  bounding_box.width() * scale,
                                  bounding_box.height() * scale);
   Send(new AutofillHostMsg_ShowPasswordSuggestions(
-      routing_id(), field, user_input.value(), show_all, bounding_box_scaled));
+      routing_id(), key_it->second, field.text_direction, user_input.value(),
+      show_all, bounding_box_scaled));
 
   bool suggestions_present = false;
   if (GetSuggestionsStats(fill_data, user_input.value(), show_all,
@@ -1160,6 +1157,7 @@ void PasswordAutofillAgent::FrameClosing(const blink::WebFrame* frame) {
     // There may not be a username field, so get the frame from the password
     // field.
     if (iter->second.password_field.document().frame() == frame) {
+      login_to_password_info_key_.erase(iter->first);
       password_to_username_.erase(iter->second.password_field);
       login_to_password_info_.erase(iter++);
     } else {
diff --git a/components/autofill/content/renderer/password_autofill_agent.h b/components/autofill/content/renderer/password_autofill_agent.h
index df43c3d..99f2a94 100644
--- a/components/autofill/content/renderer/password_autofill_agent.h
+++ b/components/autofill/content/renderer/password_autofill_agent.h
@@ -98,6 +98,7 @@ class PasswordAutofillAgent : public content::RenderViewObserver {
     PasswordInfo();
   };
   typedef std::map<blink::WebElement, PasswordInfo> LoginToPasswordInfoMap;
+  typedef std::map<blink::WebElement, int> LoginToPasswordInfoKeyMap;
   typedef std::map<blink::WebElement, blink::WebElement> PasswordToLoginMap;
   typedef std::map<blink::WebFrame*,
                    linked_ptr<PasswordForm> > FrameToPasswordFormMap;
@@ -146,7 +147,7 @@ class PasswordAutofillAgent : public content::RenderViewObserver {
                       const blink::WebFormElement& form) override;
 
   // RenderView IPC handlers:
-  void OnFillPasswordForm(const PasswordFormFillData& form_data);
+  void OnFillPasswordForm(int key, const PasswordFormFillData& form_data);
   void OnSetLoggingState(bool active);
 
   // Scans the given frame for password forms and sends them up to the browser.
@@ -191,6 +192,8 @@ class PasswordAutofillAgent : public content::RenderViewObserver {
 
   // The logins we have filled so far with their associated info.
   LoginToPasswordInfoMap login_to_password_info_;
+  // And the keys under which PasswordAutofillManager can find the same info.
+  LoginToPasswordInfoKeyMap login_to_password_info_key_;
   // A (sort-of) reverse map to |login_to_password_info_|.
   PasswordToLoginMap password_to_username_;
 
diff --git a/components/password_manager/content/browser/content_password_manager_driver.cc b/components/password_manager/content/browser/content_password_manager_driver.cc
index 0c0e348..62c20e6 100644
--- a/components/password_manager/content/browser/content_password_manager_driver.cc
+++ b/components/password_manager/content/browser/content_password_manager_driver.cc
@@ -27,7 +27,8 @@ ContentPasswordManagerDriver::ContentPasswordManagerDriver(
     : WebContentsObserver(web_contents),
       password_manager_(client),
       password_generation_manager_(client),
-      password_autofill_manager_(client, autofill_client) {
+      password_autofill_manager_(client, autofill_client),
+      next_free_key_(0) {
   DCHECK(web_contents);
 }
 
@@ -35,9 +36,11 @@ ContentPasswordManagerDriver::~ContentPasswordManagerDriver() {}
 
 void ContentPasswordManagerDriver::FillPasswordForm(
     const autofill::PasswordFormFillData& form_data) {
+  const int key = next_free_key_++;
+  password_autofill_manager_.OnAddPasswordFormMapping(key, form_data);
   DCHECK(web_contents());
   web_contents()->GetRenderViewHost()->Send(new AutofillMsg_FillPasswordForm(
-      web_contents()->GetRenderViewHost()->GetRoutingID(), form_data));
+      web_contents()->GetRenderViewHost()->GetRoutingID(), key, form_data));
 }
 
 void ContentPasswordManagerDriver::AllowPasswordGenerationForForm(
@@ -133,9 +136,6 @@ bool ContentPasswordManagerDriver::OnMessageReceived(
   IPC_MESSAGE_FORWARD(AutofillHostMsg_ShowPasswordSuggestions,
                       &password_autofill_manager_,
                       PasswordAutofillManager::OnShowPasswordSuggestions)
-  IPC_MESSAGE_FORWARD(AutofillHostMsg_AddPasswordFormMapping,
-                      &password_autofill_manager_,
-                      PasswordAutofillManager::OnAddPasswordFormMapping)
   IPC_MESSAGE_FORWARD(AutofillHostMsg_RecordSavePasswordProgress,
                       password_manager_.client(),
                       PasswordManagerClient::LogSavePasswordProgress)
diff --git a/components/password_manager/content/browser/content_password_manager_driver.h b/components/password_manager/content/browser/content_password_manager_driver.h
index c6b64f6..3d454a6 100644
--- a/components/password_manager/content/browser/content_password_manager_driver.h
+++ b/components/password_manager/content/browser/content_password_manager_driver.h
@@ -63,6 +63,12 @@ class ContentPasswordManagerDriver : public PasswordManagerDriver,
   PasswordGenerationManager password_generation_manager_;
   PasswordAutofillManager password_autofill_manager_;
 
+  // Every instance of PasswordFormFillData created by |*this| and sent to
+  // PasswordAutofillManager and PasswordAutofillAgent is given an ID, so that
+  // the latter two classes can reference to the same instance without sending
+  // it to each other over IPC. The counter below is used to generate new IDs.
+  int next_free_key_;
+
   DISALLOW_COPY_AND_ASSIGN(ContentPasswordManagerDriver);
 };
 
diff --git a/components/password_manager/core/browser/password_autofill_manager.cc b/components/password_manager/core/browser/password_autofill_manager.cc
index 2d680b6..1a45c18 100644
--- a/components/password_manager/core/browser/password_autofill_manager.cc
+++ b/components/password_manager/core/browser/password_autofill_manager.cc
@@ -69,12 +69,11 @@ PasswordAutofillManager::PasswordAutofillManager(
 PasswordAutofillManager::~PasswordAutofillManager() {
 }
 
-bool PasswordAutofillManager::FillSuggestion(
-    const autofill::FormFieldData& field,
-    const base::string16& username) {
+bool PasswordAutofillManager::FillSuggestion(int key,
+                                             const base::string16& username) {
   autofill::PasswordFormFillData fill_data;
   base::string16 password;
-  if (FindLoginInfo(field, &fill_data) &&
+  if (FindLoginInfo(key, &fill_data) &&
       GetPasswordForUsername(username, fill_data, &password)) {
     PasswordManagerDriver* driver = password_manager_client_->GetDriver();
     driver->FillSuggestion(username, password);
@@ -84,11 +83,11 @@ bool PasswordAutofillManager::FillSuggestion(
 }
 
 bool PasswordAutofillManager::PreviewSuggestion(
-    const autofill::FormFieldData& field,
+    int key,
     const base::string16& username) {
   autofill::PasswordFormFillData fill_data;
   base::string16 password;
-  if (FindLoginInfo(field, &fill_data) &&
+  if (FindLoginInfo(key, &fill_data) &&
       GetPasswordForUsername(username, fill_data, &password)) {
     PasswordManagerDriver* driver = password_manager_client_->GetDriver();
     driver->PreviewSuggestion(username, password);
@@ -98,27 +97,26 @@ bool PasswordAutofillManager::PreviewSuggestion(
 }
 
 void PasswordAutofillManager::OnAddPasswordFormMapping(
-    const autofill::FormFieldData& field,
+    int key,
     const autofill::PasswordFormFillData& fill_data) {
-  if (!autofill::IsValidFormFieldData(field) ||
-      !autofill::IsValidPasswordFormFillData(fill_data))
+  if (!autofill::IsValidPasswordFormFillData(fill_data))
     return;
 
-  login_to_password_info_[field] = fill_data;
+  login_to_password_info_[key] = fill_data;
 }
 
 void PasswordAutofillManager::OnShowPasswordSuggestions(
-    const autofill::FormFieldData& field,
+    int key,
+    base::i18n::TextDirection text_direction,
     const base::string16& typed_username,
     bool show_all,
     const gfx::RectF& bounds) {
   std::vector<base::string16> suggestions;
   std::vector<base::string16> realms;
   LoginToPasswordInfoMap::const_iterator fill_data_it =
-      login_to_password_info_.find(field);
+      login_to_password_info_.find(key);
   if (fill_data_it == login_to_password_info_.end()) {
-    // TODO(vabr): After https://codereview.chromium.org/686653003/, the only
-    // way to get here should be a compromised renderer.
+    // Probably a compromised renderer.
     NOTREACHED();
     return;
   }
@@ -126,7 +124,7 @@ void PasswordAutofillManager::OnShowPasswordSuggestions(
                  show_all);
   DCHECK_EQ(suggestions.size(), realms.size());
 
-  form_field_ = field;
+  form_data_key_ = key;
 
   if (suggestions.empty()) {
     autofill_client_->HideAutofillPopup();
@@ -137,7 +135,7 @@ void PasswordAutofillManager::OnShowPasswordSuggestions(
   std::vector<int> password_ids(suggestions.size(),
                                 autofill::POPUP_ITEM_ID_PASSWORD_ENTRY);
   autofill_client_->ShowAutofillPopup(bounds,
-                                      field.text_direction,
+                                      text_direction,
                                       suggestions,
                                       realms,
                                       empty,
@@ -150,15 +148,15 @@ void PasswordAutofillManager::Reset() {
 }
 
 bool PasswordAutofillManager::FillSuggestionForTest(
-    const autofill::FormFieldData& field,
+    int key,
     const base::string16& username) {
-  return FillSuggestion(field, username);
+  return FillSuggestion(key, username);
 }
 
 bool PasswordAutofillManager::PreviewSuggestionForTest(
-    const autofill::FormFieldData& field,
+    int key,
     const base::string16& username) {
-  return PreviewSuggestion(field, username);
+  return PreviewSuggestion(key, username);
 }
 
 void PasswordAutofillManager::OnPopupShown() {
@@ -170,13 +168,13 @@ void PasswordAutofillManager::OnPopupHidden() {
 void PasswordAutofillManager::DidSelectSuggestion(const base::string16& value,
                                                   int identifier) {
   ClearPreviewedForm();
-  bool success = PreviewSuggestion(form_field_, value);
+  bool success = PreviewSuggestion(form_data_key_, value);
   DCHECK(success);
 }
 
 void PasswordAutofillManager::DidAcceptSuggestion(const base::string16& value,
                                                   int identifier) {
-  bool success = FillSuggestion(form_field_, value);
+  bool success = FillSuggestion(form_data_key_, value);
   DCHECK(success);
   autofill_client_->HideAutofillPopup();
 }
@@ -235,9 +233,9 @@ bool PasswordAutofillManager::GetPasswordForUsername(
 }
 
 bool PasswordAutofillManager::FindLoginInfo(
-    const autofill::FormFieldData& field,
+    int key,
     autofill::PasswordFormFillData* found_password) {
-  LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(field);
+  LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(key);
   if (iter == login_to_password_info_.end())
     return false;
 
diff --git a/components/password_manager/core/browser/password_autofill_manager.h b/components/password_manager/core/browser/password_autofill_manager.h
index f3d4f5d..4ad14b9 100644
--- a/components/password_manager/core/browser/password_autofill_manager.h
+++ b/components/password_manager/core/browser/password_autofill_manager.h
@@ -8,6 +8,7 @@
 #include <map>
 
 #include "base/gtest_prod_util.h"
+#include "base/i18n/rtl.h"
 #include "components/autofill/core/browser/autofill_client.h"
 #include "components/autofill/core/browser/autofill_popup_delegate.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
@@ -39,12 +40,13 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
 
   // Invoked when a password mapping is added.
   void OnAddPasswordFormMapping(
-      const autofill::FormFieldData& field,
+      int key,
       const autofill::PasswordFormFillData& fill_data);
 
   // Handles a request from the renderer to show a popup with the given
   // |suggestions| from the password manager.
-  void OnShowPasswordSuggestions(const autofill::FormFieldData& field,
+  void OnShowPasswordSuggestions(int key,
+                                 base::i18n::TextDirection text_direction,
                                  const base::string16& typed_username,
                                  bool show_all,
                                  const gfx::RectF& bounds);
@@ -53,26 +55,21 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
   void Reset();
 
   // A public version of FillSuggestion(), only for use in tests.
-  bool FillSuggestionForTest(const autofill::FormFieldData& field,
-                             const base::string16& username);
+  bool FillSuggestionForTest(int key, const base::string16& username);
 
   // A public version of PreviewSuggestion(), only for use in tests.
-  bool PreviewSuggestionForTest(const autofill::FormFieldData& field,
-                                const base::string16& username);
+  bool PreviewSuggestionForTest(int key, const base::string16& username);
 
  private:
-  typedef std::map<autofill::FormFieldData, autofill::PasswordFormFillData>
-      LoginToPasswordInfoMap;
+  typedef std::map<int, autofill::PasswordFormFillData> LoginToPasswordInfoMap;
 
   // Attempts to fill the password associated with user name |username|, and
   // returns true if it was successful.
-  bool FillSuggestion(const autofill::FormFieldData& field,
-                      const base::string16& username);
+  bool FillSuggestion(int key, const base::string16& username);
 
   // Attempts to preview the password associated with user name |username|, and
   // returns true if it was successful.
-  bool PreviewSuggestion(const autofill::FormFieldData& field,
-                         const base::string16& username);
+  bool PreviewSuggestion(int key, const base::string16& username);
 
   // If |current_username| matches a username for one of the login mappings in
   // |fill_data|, returns true and assigns the password to |out_password|.
@@ -83,20 +80,20 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
       base::string16* out_password);
 
   // Finds login information for a |node| that was previously filled.
-  bool FindLoginInfo(const autofill::FormFieldData& field,
-                     autofill::PasswordFormFillData* found_password);
+  bool FindLoginInfo(int key, autofill::PasswordFormFillData* found_password);
 
   // The logins we have filled so far with their associated info.
   LoginToPasswordInfoMap login_to_password_info_;
 
+  // When the autofill popup should be shown, |form_data_key_| identifies the
+  // right password info in |login_to_password_info_|.
+  int form_data_key_;
+
   // Provides embedder-level operations on passwords. Must outlive |this|.
   PasswordManagerClient* const password_manager_client_;  // weak
 
   autofill::AutofillClient* const autofill_client_;  // weak
 
-  // The form field on which the autofill popup is shown.
-  autofill::FormFieldData form_field_;
-
   base::WeakPtrFactory<PasswordAutofillManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordAutofillManager);
diff --git a/components/password_manager/core/browser/password_autofill_manager_unittest.cc b/components/password_manager/core/browser/password_autofill_manager_unittest.cc
index ab09812..1f46b40 100644
--- a/components/password_manager/core/browser/password_autofill_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_autofill_manager_unittest.cc
@@ -73,13 +73,15 @@ class PasswordAutofillManagerTest : public testing::Test {
  protected:
   PasswordAutofillManagerTest()
       : test_username_(base::ASCIIToUTF16(kAliceUsername)),
-        test_password_(base::ASCIIToUTF16(kAlicePassword)) {}
+        test_password_(base::ASCIIToUTF16(kAlicePassword)),
+        fill_data_id_(0) {}
 
   void SetUp() override {
     // Add a preferred login and an additional login to the FillData.
-    username_field_.name = base::ASCIIToUTF16(kUsernameName);
-    username_field_.value = test_username_;
-    fill_data_.basic_data.fields.push_back(username_field_);
+    autofill::FormFieldData username_field;
+    username_field.name = base::ASCIIToUTF16(kUsernameName);
+    username_field.value = test_username_;
+    fill_data_.basic_data.fields.push_back(username_field);
 
     autofill::FormFieldData password_field;
     password_field.name = base::ASCIIToUTF16(kPasswordName);
@@ -92,19 +94,21 @@ class PasswordAutofillManagerTest : public testing::Test {
       autofill::AutofillClient* autofill_client) {
     password_autofill_manager_.reset(
         new PasswordAutofillManager(client, autofill_client));
-    password_autofill_manager_->OnAddPasswordFormMapping(username_field_,
+    password_autofill_manager_->OnAddPasswordFormMapping(fill_data_id_,
                                                          fill_data_);
   }
 
  protected:
+  int fill_data_id() { return fill_data_id_; }
+
   scoped_ptr<PasswordAutofillManager> password_autofill_manager_;
 
-  autofill::FormFieldData username_field_;
   base::string16 test_username_;
   base::string16 test_password_;
 
  private:
   autofill::PasswordFormFillData fill_data_;
+  const int fill_data_id_;
 
   // The TestAutofillDriver uses a SequencedWorkerPool which expects the
   // existence of a MessageLoop.
@@ -118,23 +122,22 @@ TEST_F(PasswordAutofillManagerTest, FillSuggestion) {
   EXPECT_CALL(*client->mock_driver(),
               FillSuggestion(test_username_, test_password_));
   EXPECT_TRUE(password_autofill_manager_->FillSuggestionForTest(
-      username_field_, test_username_));
+      fill_data_id(), test_username_));
   testing::Mock::VerifyAndClearExpectations(client->mock_driver());
 
   EXPECT_CALL(*client->mock_driver(),
               FillSuggestion(_, _)).Times(0);
   EXPECT_FALSE(password_autofill_manager_->FillSuggestionForTest(
-      username_field_, base::ASCIIToUTF16(kInvalidUsername)));
+      fill_data_id(), base::ASCIIToUTF16(kInvalidUsername)));
 
-  autofill::FormFieldData invalid_username_field;
-  invalid_username_field.name = base::ASCIIToUTF16(kInvalidUsername);
+  const int invalid_fill_data_id = fill_data_id() + 1;
 
   EXPECT_FALSE(password_autofill_manager_->FillSuggestionForTest(
-      invalid_username_field, test_username_));
+      invalid_fill_data_id, test_username_));
 
   password_autofill_manager_->Reset();
   EXPECT_FALSE(password_autofill_manager_->FillSuggestionForTest(
-      username_field_, test_username_));
+      fill_data_id(), test_username_));
 }
 
 TEST_F(PasswordAutofillManagerTest, PreviewSuggestion) {
@@ -144,22 +147,21 @@ TEST_F(PasswordAutofillManagerTest, PreviewSuggestion) {
   EXPECT_CALL(*client->mock_driver(),
               PreviewSuggestion(test_username_, test_password_));
   EXPECT_TRUE(password_autofill_manager_->PreviewSuggestionForTest(
-      username_field_, test_username_));
+      fill_data_id(), test_username_));
   testing::Mock::VerifyAndClearExpectations(client->mock_driver());
 
   EXPECT_CALL(*client->mock_driver(), PreviewSuggestion(_, _)).Times(0);
   EXPECT_FALSE(password_autofill_manager_->PreviewSuggestionForTest(
-      username_field_, base::ASCIIToUTF16(kInvalidUsername)));
+      fill_data_id(), base::ASCIIToUTF16(kInvalidUsername)));
 
-  autofill::FormFieldData invalid_username_field;
-  invalid_username_field.name = base::ASCIIToUTF16(kInvalidUsername);
+  const int invalid_fill_data_id = fill_data_id() + 1;
 
   EXPECT_FALSE(password_autofill_manager_->PreviewSuggestionForTest(
-      invalid_username_field, test_username_));
+      invalid_fill_data_id, test_username_));
 
   password_autofill_manager_->Reset();
   EXPECT_FALSE(password_autofill_manager_->PreviewSuggestionForTest(
-      username_field_, test_username_));
+      fill_data_id(), test_username_));
 }
 
 // Test that the popup is marked as visible after recieving password
@@ -175,7 +177,7 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) {
   data.basic_data.fields[0].value = test_username_;
   data.basic_data.fields[1].value = test_password_;
   data.preferred_realm = "http://foo.com/";
-  autofill::FormFieldData dummy_key;
+  int dummy_key = 0;
   password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data);
 
   EXPECT_CALL(*client->mock_driver(),
@@ -192,7 +194,8 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) {
                   testing::ElementsAre(autofill::POPUP_ITEM_ID_PASSWORD_ENTRY),
                   _));
   password_autofill_manager_->OnShowPasswordSuggestions(
-      dummy_key, base::string16(), false, element_bounds);
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::string16(), false,
+      element_bounds);
 
   // Accepting a suggestion should trigger a call to hide the popup.
   EXPECT_CALL(*autofill_client, HideAutofillPopup());
@@ -226,7 +229,7 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
   other_names.push_back(other_username);
   data.other_possible_usernames[usernames_key] = other_names;
 
-  autofill::FormFieldData dummy_key;
+  int dummy_key = 0;
   password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data);
 
   // First, simulate displaying suggestions matching an empty prefix.
@@ -237,7 +240,8 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
                       test_username_, additional_username, other_username),
                   _, _, _, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
-      dummy_key, base::string16(), false, element_bounds);
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::string16(), false,
+      element_bounds);
 
   // Now simulate displaying suggestions matching "John".
   EXPECT_CALL(*autofill_client,
@@ -246,7 +250,8 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
                                     additional_username, other_username),
                                 _, _, _, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
-      dummy_key, base::ASCIIToUTF16("John"), false, element_bounds);
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("John"), false,
+      element_bounds);
 
   // Finally, simulate displaying all suggestions, without any prefix matching.
   EXPECT_CALL(*autofill_client,
@@ -256,7 +261,8 @@ TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) {
                       test_username_, additional_username, other_username),
                   _, _, _, _));
   password_autofill_manager_->OnShowPasswordSuggestions(
-      dummy_key, base::ASCIIToUTF16("xyz"), true, element_bounds);
+      dummy_key, base::i18n::RIGHT_TO_LEFT, base::ASCIIToUTF16("xyz"), true,
+      element_bounds);
 }
 
 }  // namespace password_manager