Add Password Autofill Manager to New Autofill

chrome/browser/autofill/password_autofill_manager.cc

Index: chrome/browser/autofill/password_autofill_manager.cc
diff --git a/chrome/browser/autofill/password_autofill_manager.cc b/chrome/browser/autofill/password_autofill_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..883b2390d8fede99c1a641774115923956cad5f9
--- /dev/null
+++ b/chrome/browser/autofill/password_autofill_manager.cc
@@ -0,0 +1,139 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/autofill/password_autofill_manager.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/common/autofill_messages.h"
+#include "content/browser/renderer_host/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/renderer/render_view.h"
+#include "ui/base/keycodes/keyboard_codes.h"
+
+////////////////////////////////////////////////////////////////////////////////
+// PasswordAutofillManager, public:
+
+PasswordAutofillManager::PasswordAutofillManager(
+    TabContentsWrapper* tab_contents_wrapper)
+    : tab_contents_wrapper_(tab_contents_wrapper) {
+}
+
+PasswordAutofillManager::~PasswordAutofillManager() {
+}
+
+bool PasswordAutofillManager::TextFieldHandlingKeyDown(
+    const webkit::forms::FormField& field,
+    int key_code) {
+  LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(field);
+  if (iter == login_to_password_info_.end())
+    return false;
+
+  if (tab_contents_wrapper_ && tab_contents_wrapper_->web_contents() &&
+      tab_contents_wrapper_->web_contents()->GetRenderViewHost()) {

[Ilya Sherman, 2012/03/06 20:32:44]

Hmm, can any of these actually ever be NULL?

[csharp, 2012/03/07 15:49:16]

I don't think so, but I since I only use this to send IPC messages I pass in a
NULL pointer in the unit tests.

Is there a better way to do this? Should I just take in a pointer to
RenderViewHost instead of TabContentsWrapper in the constructor and then just
make sure it isn't NULL here?

[Ilya Sherman, 2012/03/07 22:33:18]

If the RenderViewHost remains constant for a TabContentsWrapper, it would
definitely be better to take a pointer to that instead, in part to more narrowly
define the class dependency.

[csharp, 2012/03/08 16:48:37]

Done.

+    RenderViewHost* host =
+        tab_contents_wrapper_->web_contents()->GetRenderViewHost();
+    host->Send(new AutofillMsg_PasswordHandleKeyDown(host->GetRoutingID(),
+                                                     key_code));
+  }
+
+  iter->second.backspace_pressed_last =
+      (key_code == ui::VKEY_BACK || key_code == ui::VKEY_DELETE);

[Ilya Sherman, 2012/03/06 20:32:44]

It looks like you're also doing this work on the renderer side.  Why do we need
to do it in both places?

[csharp, 2012/03/07 15:49:16]

I had thought I still had code here that looked at the backspace_pressed_last
value but that seems to have been moved out. Removing the handling from here.

+
+  return true;
+}
+
+bool PasswordAutofillManager::DidAcceptAutofillSuggestion(
+    const webkit::forms::FormField& field,
+    const string16& value) {
+  webkit::forms::FormField input;
+  PasswordInfo password;
+  if (!FindLoginInfo(field, &input, &password))
+    return false;
+
+  if (WillFillUserNameAndPassword(input, password.fill_data)) {
+    if (tab_contents_wrapper_ && tab_contents_wrapper_->web_contents() &&
+        tab_contents_wrapper_->web_contents()->GetRenderViewHost()) {
+      RenderViewHost* host =
+          tab_contents_wrapper_->web_contents()->GetRenderViewHost();
+      host->Send(new AutofillMsg_PasswordAcceptAutofillSuggestion(
+          host->GetRoutingID(),
+          value));

[Ilya Sherman, 2012/03/06 20:32:44]

Why not send the full fill data here, rather than having the renderer side redo
the work?

[csharp, 2012/03/07 15:49:16]

We kinda get the full fill data for free in the renderer, since the the
PasswordInfo class here is fairly different from the one in the renderer, it
lacks a few variables uses webkit::forms:FormField instead of
WebKit::WebInputElement.

This means that we need to call FindLoginInfo to get those missing variables
anyhow, and the fill data is stored with them so we still get it.

[Ilya Sherman, 2012/03/07 22:33:18]

Hmm, it seems a little wrong to have both the browser and the renderer tracking
this info, though.  What are the missing bits from the browser side that the
renderer has access to?

[csharp, 2012/03/08 16:48:37]

Ok, so I think we can removed the duplicated data. Basically we keep the
PasswordInfo class in the renderer, but remove the fill_data from it and just
store that information on the  browser side. It should be fairly easy to just
pass the required fill data when we need it.

I'm not sure that at this point it makes sense to do this though. Since the old
autofill UI still needs the fill_data on the render we can't remove it from the
password_info class. I think the best approach here is probably to file a bug on
myself to remove this duplicated data and once we have switched over to the new
UI for all platform, fix the code. 

[Ilya Sherman, 2012/03/08 23:22:14]

Ok, I think I better understand now why it's hard to separate the data.  The
renderer side wants to know all of the login info immediately, in case there's
no need to show UI.  Both sides want to know all the data (really just the
usernames, not the passwords), so that they know when to show the UI.  I don't
see a clean way to support those needs without duplicating some of the data, so
I guess I'm ok with having the duplicate tracking for now.

+    }
+    return true;
+  }
+
+  return false;
+}
+
+bool PasswordAutofillManager::DidSelectAutofillSuggestion(
+    const webkit::forms::FormField& field) {
+  webkit::forms::FormField input;
+  PasswordInfo password;
+  return FindLoginInfo(field, &input, &password);
+}
+
+bool PasswordAutofillManager::DidClearAutofillSelection(
+    const webkit::forms::FormField& field) {
+  webkit::forms::FormField input;
+  PasswordInfo password;
+  return FindLoginInfo(field, &input, &password);
+}
+
+void PasswordAutofillManager::FillPasswordForm(
+    const webkit::forms::FormField& username_element,
+    const webkit::forms::PasswordFormFillData& fill_data,
+    int frame_id) {
+  PasswordInfo password_info;
+  password_info.frame_id = frame_id;
+  password_info.fill_data = fill_data;
+
+  login_to_password_info_[username_element] = password_info;
+}
+
+void PasswordAutofillManager::FrameClosing(long long frame_id) {
+  for (LoginToPasswordInfoMap::iterator iter = login_to_password_info_.begin();
+       iter != login_to_password_info_.end();) {
+    if (iter->second.frame_id == frame_id)
+      login_to_password_info_.erase(iter++);
+    else
+      ++iter;
+  }

[Ilya Sherman, 2012/03/06 20:32:44]

Does anything go wrong if we just empty the map whenever the frame containing
the to-be-autofilled element closes?

[csharp, 2012/03/07 15:49:16]

From my understanding of this code, I don't think so.

But this is how it is done in the renderer side, so I assume there is a good
reason for doing it this way (maybe when there are multiple frames on the same
page?).

+}
+
+////////////////////////////////////////////////////////////////////////////////
+// PasswordAutofillManager, private:
+
+bool PasswordAutofillManager::WillFillUserNameAndPassword(
+    const webkit::forms::FormField& username_element,
+    const webkit::forms::PasswordFormFillData& fill_data) {
+  string16 current_username = username_element.value;
+
+  // Look for any suitable matches to current field text.
+  if (fill_data.basic_data.fields[0].value == current_username) {
+    return true;
+  } else {
+    // Scan additional logins for a match.
+    webkit::forms::PasswordFormFillData::LoginCollection::const_iterator iter;
+    for (iter = fill_data.additional_logins.begin();
+         iter != fill_data.additional_logins.end(); ++iter) {
+      if (iter->first == current_username) {
+        return true;
+      }

[Ilya Sherman, 2012/03/06 20:32:44]

nit: The if-stmt body is 1 line, so no need for curlys.

[csharp, 2012/03/07 15:49:16]

Done.

+    }
+  }
+
+  return false;
+}
+
+bool PasswordAutofillManager::FindLoginInfo(
+    const webkit::forms::FormField& field,
+    webkit::forms::FormField* found_input,
+    PasswordInfo* found_password) {
+  LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(field);
+  if (iter == login_to_password_info_.end())
+    return false;
+
+  *found_input = field;
+  *found_password = iter->second;
+  return true;
+}