Reland "[Android] Change suggestions based on frame of focused field"

This reverts commit 82c224b0.

Reason for revert: The linked build (7968) wasn't the first broken build. The first broken build was 7959 where https://crrev.com/c/1139057 reenabled this known-to-be-flaky test (see issue 849582). dullweber@ takes care of the new revert.

Original change's description:
> Revert "[Android] Change suggestions based on frame of focused field"
> 
> This reverts commit 4c9cbfb5.
> 
> Sorry for the revert -- this is failing on one of the ChromeOS builders:
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests/7968
> 
> You can see several failed tests in this log output:
> https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8940668796232357776/+/steps/interactive_ui_tests/0/logs/All__x2f_PasswordManagerBrowserTestWithConditionalPopupViews.AutofillLoginSignupForm__x2f_1/0
> 
> 
> Original change's description:
> > [Android] Change suggestions based on frame of focused field
> > 
> > With this CL, the renderer notifies the password accessory controller
> > when the focus moves to or away from valid input fields.
> > 
> > Bug: 854152, 854150, 854149, 853742
> > Change-Id: I63f075ce238db8b77c784e945eea8ec83d8d4344
> > Reviewed-on: https://chromium-review.googlesource.com/1124466
> > Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
> > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> > Reviewed-by: Antoine Labour <piman@chromium.org>
> > Reviewed-by: Mike West <mkwst@chromium.org>
> > Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575977}
> 
> TBR=vasilii@chromium.org,dvadym@chromium.org,piman@chromium.org,fhorschig@chromium.org,mkwst@chromium.org
> 
> Change-Id: I01cc79e8dc98e29cbdb1a61c6da5a4cf021fcbfb
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 854152, 854150, 854149, 853742
> Reviewed-on: https://chromium-review.googlesource.com/1141825
> Reviewed-by: Ian Clelland <iclelland@chromium.org>
> Commit-Queue: Ian Clelland <iclelland@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576042}

TBR=vasilii@chromium.org,dvadym@chromium.org,iclelland@chromium.org,piman@chromium.org,fhorschig@chromium.org,mkwst@chromium.org

Change-Id: Idaadebd9b00179935c3f6347cc669c0811a912e4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 854152, 854150, 854149, 853742
Reviewed-on: https://chromium-review.googlesource.com/1141885Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576066}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -561,16 +561,20 @@ void ChromePasswordManagerClient::DidFinishNavigation(
   web_contents()->GetRenderViewHost()->GetWidget()->RemoveInputEventObserver(
       this);
   web_contents()->GetRenderViewHost()->GetWidget()->AddInputEventObserver(this);
-#else
-  // Ensure that entries from old origins are properly cleaned up.
-  PasswordAccessoryController* password_accessory =
-      PasswordAccessoryController::FromWebContents(web_contents());
-  if (password_accessory) {
-    password_accessory->DidNavigateMainFrame();
-  }
 #endif
 }
 
+void ChromePasswordManagerClient::RenderFrameDeleted(
+    content::RenderFrameHost* render_frame_host) {
+#if defined(OS_ANDROID)
+  PasswordAccessoryController* accessory =
+      PasswordAccessoryController::FromWebContents(web_contents());
+  if (!accessory)
+    return;  // No accessory, no cleanup needed.
+  accessory->ClearSuggestions();
+#endif  // defined(OS_ANDROID)
+}
+
 #if !defined(OS_ANDROID)
 void ChromePasswordManagerClient::OnInputEvent(
     const blink::WebInputEvent& event) {
@@ -1061,6 +1065,25 @@ void ChromePasswordManagerClient::ShowPasswordGenerationPopup(
   popup_controller_->Show(PasswordGenerationPopupController::kOfferGeneration);
 }
 
+void ChromePasswordManagerClient::FocusedInputChanged(bool is_fillable,
+                                                      bool is_password_field) {
+#if defined(OS_ANDROID)
+  // Either #passwords-keyboards-accessory or #experimental-ui must be enabled.
+  if (!base::FeatureList::IsEnabled(
+          password_manager::features::kPasswordsKeyboardAccessory) &&
+      !base::FeatureList::IsEnabled(features::kExperimentalUi)) {
+    return;  // No need to even create the bridge if it's not going to be used.
+  }
+  if (is_fillable)  // Refresh but don't create a new accessory in this case.
+    PasswordAccessoryController::CreateForWebContents(web_contents());
+  PasswordAccessoryController* accessory =
+      PasswordAccessoryController::FromWebContents(web_contents());
+  if (!accessory)
+    return;  // No accessory needs change here.
+  accessory->RefreshSuggestionsForField(is_fillable, is_password_field);
+#endif  // defined(OS_ANDROID)
+}
+
 password_manager::PasswordManager*
 ChromePasswordManagerClient::GetPasswordManager() {
   return &password_manager_;

chrome/browser/password_manager/chrome_password_manager_client.h

@@ -208,12 +208,14 @@ class ChromePasswordManagerClient
       const base::string16& generation_field) override;
   void CheckSafeBrowsingReputation(const GURL& form_action,
                                    const GURL& frame_url) override;
+  void FocusedInputChanged(bool is_fillable, bool is_password_field) override;
 
   // content::WebContentsObserver overrides.
   void DidStartNavigation(
       content::NavigationHandle* navigation_handle) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
+  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
 
 // TODO(crbug.com/706392): Fix password reuse detection for Android.
 #if !defined(OS_ANDROID)

chrome/browser/password_manager/chrome_password_manager_client_unittest.cc

@@ -149,7 +149,8 @@ class FakePasswordAutofillAgent
       const autofill::PasswordFormFillData& form_data) override {}
 
   void FillIntoFocusedField(bool is_password,
-                            const base::string16& credential) override {}
+                            const base::string16& credential,
+                            FillIntoFocusedFieldCallback callback) override {}
 
   void SetLoggingState(bool active) override {
     called_set_logging_state_ = true;

chrome/browser/password_manager/password_accessory_controller.cc

@@ -57,8 +57,8 @@ PasswordAccessoryController::PasswordAccessoryController(
     : web_contents_(web_contents),
       view_(PasswordAccessoryViewInterface::Create(this)),
       create_dialog_factory_(
-          base::BindRepeating(&PasswordGenerationDialogViewInterface::Create)) {
-}
+          base::BindRepeating(&PasswordGenerationDialogViewInterface::Create)),
+      weak_factory_(this) {}
 
 // Additional creation functions in unit tests only:
 PasswordAccessoryController::PasswordAccessoryController(
@@ -67,7 +67,8 @@ PasswordAccessoryController::PasswordAccessoryController(
     CreateDialogFactory create_dialog_factory)
     : web_contents_(web_contents),
       view_(std::move(view)),
-      create_dialog_factory_(create_dialog_factory) {}
+      create_dialog_factory_(create_dialog_factory),
+      weak_factory_(this) {}
 
 PasswordAccessoryController::~PasswordAccessoryController() = default;
 
@@ -87,11 +88,12 @@ void PasswordAccessoryController::CreateForWebContentsForTesting(
 void PasswordAccessoryController::OnPasswordsAvailable(
     const std::map<base::string16, const PasswordForm*>& best_matches,
     const GURL& origin) {
-  const url::Origin& tab_origin =
-      web_contents_->GetMainFrame()->GetLastCommittedOrigin();
-  if (!tab_origin.IsSameOriginWith(url::Origin::Create(origin))) {
+  const url::Origin& frame_origin =
+      web_contents_->GetFocusedFrame()->GetLastCommittedOrigin();
+  url::Origin password_origin = url::Origin::Create(origin);
+  if (!frame_origin.IsSameOriginWith(password_origin)) {
     // TODO(fhorschig): Support iframes: https://crbug.com/854150.
-    return;
+    return;  // Don't make passwords available across origins.
   }
   DCHECK(view_);
   std::vector<Item> items;
@@ -125,13 +127,6 @@ void PasswordAccessoryController::OnPasswordsAvailable(
   view_->OnItemsAvailable(origin, items);
 }
 
-void PasswordAccessoryController::DidNavigateMainFrame() {
-  // Sending no passwords removes stale stuggestions and sends default options.
-  OnPasswordsAvailable(
-      /*best_matches=*/{},
-      web_contents_->GetMainFrame()->GetLastCommittedOrigin().GetURL());
-}
-
 void PasswordAccessoryController::OnAutomaticGenerationStatusChanged(
     bool available,
     const base::Optional<
@@ -156,7 +151,7 @@ void PasswordAccessoryController::OnAutomaticGenerationStatusChanged(
 
 void PasswordAccessoryController::OnFillingTriggered(
     bool is_password,
-    const base::string16& textToFill) const {
+    const base::string16& textToFill) {
   password_manager::ContentPasswordManagerDriverFactory* factory =
       password_manager::ContentPasswordManagerDriverFactory::FromWebContents(
           web_contents_);
@@ -167,7 +162,10 @@ void PasswordAccessoryController::OnFillingTriggered(
   if (!driver) {
     return;
   }  // |driver| can be NULL if the tab is being closed.
-  driver->FillIntoFocusedField(is_password, textToFill);
+  driver->FillIntoFocusedField(
+      is_password, textToFill,
+      base::BindOnce(&PasswordAccessoryController::OnFilledIntoFocusedField,
+                     weak_factory_.GetWeakPtr()));
 }
 
 void PasswordAccessoryController::OnOptionSelected(
@@ -216,6 +214,26 @@ void PasswordAccessoryController::OnSavedPasswordsLinkClicked() {
   chrome::android::PreferencesLauncher::ShowPasswordSettings();
 }
 
+void PasswordAccessoryController::OnFilledIntoFocusedField(
+    autofill::FillingStatus status) {
+  // TODO(crbug/853766): Record success rate.
+  // TODO(fhorschig): Update UI by hiding the sheet or communicating the error.
+}
+
+void PasswordAccessoryController::ClearSuggestions() {
+  // TODO(fhorschig): This should drop suggestion for the given frame.
+  OnPasswordsAvailable(
+      /*best_matches=*/{},
+      web_contents_->GetFocusedFrame()->GetLastCommittedOrigin().GetURL());
+}
+
+void PasswordAccessoryController::RefreshSuggestionsForField(
+    bool is_fillable,
+    bool is_password_field) {
+  // TODO(crbug/853766): Record CTR metric.
+  // TODO(fhorschig): Test and implement.
+}
+
 gfx::NativeView PasswordAccessoryController::container_view() const {
   return web_contents_->GetNativeView();
 }

chrome/browser/password_manager/password_accessory_controller.h

@@ -11,7 +11,9 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
+#include "components/autofill/core/common/filling_status.h"
 #include "components/autofill/core/common/password_generation_util.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "ui/gfx/native_widget_types.h"
@@ -55,9 +57,6 @@ class PasswordAccessoryController
           best_matches,
       const GURL& origin);
 
-  // Send empty suggestions and default options to the view.
-  void DidNavigateMainFrame();
-
   // Notifies the view that automatic password generation status changed.
   void OnAutomaticGenerationStatusChanged(
       bool available,
@@ -67,8 +66,7 @@ class PasswordAccessoryController
 
   // Called by the UI code to request that |textToFill| is to be filled into the
   // currently focused field.
-  void OnFillingTriggered(bool is_password,
-                          const base::string16& textToFill) const;
+  void OnFillingTriggered(bool is_password, const base::string16& textToFill);
 
   // Called by the UI code because a user triggered the |selectedOption|.
   void OnOptionSelected(const base::string16& selectedOption) const;
@@ -87,6 +85,17 @@ class PasswordAccessoryController
   // in the explanation text that leads to the saved passwords.
   void OnSavedPasswordsLinkClicked();
 
+  // Compeletes a filling attempt by recording metrics, giving feedback to the
+  // user and dismissing the accessory sheet.
+  void OnFilledIntoFocusedField(autofill::FillingStatus status);
+
+  // Makes sure, that all shown suggestions are appropriate for the currently
+  // focused field.
+  void RefreshSuggestionsForField(bool is_fillable, bool is_password_field);
+
+  // Remove stale suggestions by sending empty or recent suggestions to the UI.
+  void ClearSuggestions();
+
   // The web page view containing the focused field.
   gfx::NativeView container_view() const;
 
@@ -139,6 +148,8 @@ class PasswordAccessoryController
   // Creation callback for the modal dialog view meant to facilitate testing.
   CreateDialogFactory create_dialog_factory_;
 
+  base::WeakPtrFactory<PasswordAccessoryController> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(PasswordAccessoryController);
 };
 

chrome/browser/password_manager/password_accessory_controller_unittest.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/password_manager/password_accessory_controller.h"
 
 #include <memory>
+#include <string>
+#include <vector>
 
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
@@ -245,6 +247,7 @@ class PasswordAccessoryControllerTest : public ChromeRenderViewHostTestHarness {
         std::make_unique<StrictMock<MockPasswordAccessoryView>>(),
         mock_dialog_factory_.Get());
     NavigateAndCommit(GURL("https://example.com"));
+    FocusWebContentsOnMainFrame();
   }
 
   PasswordAccessoryController* controller() {
@@ -357,7 +360,7 @@ TEST_F(PasswordAccessoryControllerTest, ClearsSuggestionsOnFrameNavigation) {
           ElementsAre(MatchesLabel(passwords_empty_str(kExampleDomain)),
                       IsDivider(), MatchesOption(manage_passwords_str()))));
 
-  controller()->DidNavigateMainFrame();
+  controller()->ClearSuggestions();
 }
 
 TEST_F(PasswordAccessoryControllerTest, ProvidesEmptySuggestionsMessage) {
@@ -371,13 +374,6 @@ TEST_F(PasswordAccessoryControllerTest, ProvidesEmptySuggestionsMessage) {
   controller()->OnPasswordsAvailable({}, GURL(kExampleSite));
 }
 
-TEST_F(PasswordAccessoryControllerTest, IgnoresCrossOriginCalls) {
-  // Don't expect any call to |OnItemsAvailable|. (https://crbug.com/854150)
-  EXPECT_CALL(*view(), OnItemsAvailable(_, _)).Times(0);
-  controller()->OnPasswordsAvailable({CreateEntry("Ben", "S3cur3").first},
-                                     GURL("https://other-domain.com"));
-}
-
 TEST_F(PasswordAccessoryControllerTest, RelaysAutomaticGenerationAvailable) {
   EXPECT_CALL(*view(), OnAutomaticGenerationStatusChanged(true));
   controller()->OnAutomaticGenerationStatusChanged(

chrome/renderer/autofill/fake_mojo_password_manager_driver.cc

@@ -90,3 +90,10 @@ void FakeMojoPasswordManagerDriver::ShowManualFallbackForSaving(
 void FakeMojoPasswordManagerDriver::HideManualFallbackForSaving() {
   called_show_manual_fallback_for_saving_count_ = 0;
 }
+
+void FakeMojoPasswordManagerDriver::FocusedInputChanged(
+    bool is_fillable,
+    bool is_password_field) {
+  last_focused_element_was_fillable_ = is_fillable;
+  last_focused_input_was_password_ = is_password_field;
+}

chrome/renderer/autofill/fake_mojo_password_manager_driver.h

@@ -130,6 +130,14 @@ class FakeMojoPasswordManagerDriver
     return called_show_manual_fallback_for_saving_count_;
   }
 
+  bool last_focused_input_was_password() const {
+    return last_focused_input_was_password_;
+  }
+
+  bool last_focused_element_was_fillable() const {
+    return last_focused_element_was_fillable_;
+  }
+
  private:
   // mojom::PasswordManagerDriver:
   void PasswordFormsParsed(
@@ -165,6 +173,7 @@ class FakeMojoPasswordManagerDriver
   void ShowManualFallbackForSaving(
       const autofill::PasswordForm& password_form) override;
   void HideManualFallbackForSaving() override;
+  void FocusedInputChanged(bool is_fillable, bool is_password_field) override;
 
   // Records whether ShowPasswordSuggestions() gets called.
   bool called_show_pw_suggestions_ = false;
@@ -207,6 +216,14 @@ class FakeMojoPasswordManagerDriver
   // If it is zero, the fallback is not available.
   int called_show_manual_fallback_for_saving_count_ = 0;
 
+  // Records wether a password field was the last input that InputChanged() was
+  // called for.
+  bool last_focused_input_was_password_ = false;
+
+  // Records wether the last input that InputChanged() was called for was
+  // fillable.
+  bool last_focused_element_was_fillable_ = false;
+
   mojo::AssociatedBinding<autofill::mojom::PasswordManagerDriver> binding_;
 };
 

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

@@ -10,6 +10,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/test/mock_callback.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/renderer/autofill/fake_mojo_password_manager_driver.h"
@@ -40,6 +41,7 @@
 #include "third_party/blink/public/web/web_view.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 
+using autofill::FillingStatus;
 using autofill::FormTracker;
 using autofill::PasswordForm;
 using base::ASCIIToUTF16;
@@ -1670,8 +1672,10 @@ TEST_F(PasswordAutofillAgentTest, FillIntoFocusedReadonlyTextField) {
   // If the field is readonly, it should not be affected.
   SetElementReadOnly(username_element_, true);
   SimulateElementClick(kUsernameName);
-  password_autofill_agent_->FillIntoFocusedField(/*is_password=*/false,
-                                                 ASCIIToUTF16(kAliceUsername));
+  base::MockCallback<base::OnceCallback<void(FillingStatus)>> mock_callback;
+  EXPECT_CALL(mock_callback, Run(FillingStatus::ERROR_NO_VALID_FIELD));
+  password_autofill_agent_->FillIntoFocusedField(
+      /*is_password=*/false, ASCIIToUTF16(kAliceUsername), mock_callback.Get());
   CheckTextFieldsDOMState(std::string(), false, std::string(), false);
 }
 
@@ -1684,8 +1688,11 @@ TEST_F(PasswordAutofillAgentTest, FillIntoFocusedWritableTextField) {
   SetFocused(username_element_);
 
   SetElementReadOnly(username_element_, false);
-  password_autofill_agent_->FillIntoFocusedField(/*is_password=*/false,
-                                                 ASCIIToUTF16(kAliceUsername));
+
+  base::MockCallback<base::OnceCallback<void(FillingStatus)>> mock_callback;
+  EXPECT_CALL(mock_callback, Run(FillingStatus::SUCCESS));
+  password_autofill_agent_->FillIntoFocusedField(
+      /*is_password=*/false, ASCIIToUTF16(kAliceUsername), mock_callback.Get());
   CheckTextFieldsDOMState(kAliceUsername, true, std::string(), false);
   CheckUsernameSelection(strlen(kAliceUsername), strlen(kAliceUsername));
 }
@@ -1697,14 +1704,19 @@ TEST_F(PasswordAutofillAgentTest, FillIntoFocusedFieldOnlyIntoPasswordFields) {
 
   // Filling a password into a username field doesn't work.
   SimulateElementClick(kUsernameName);
-  password_autofill_agent_->FillIntoFocusedField(/*is_password=*/true,
-                                                 ASCIIToUTF16(kAlicePassword));
+
+  base::MockCallback<base::OnceCallback<void(FillingStatus)>> mock_callback;
+  EXPECT_CALL(mock_callback, Run(FillingStatus::ERROR_NOT_ALLOWED));
+  password_autofill_agent_->FillIntoFocusedField(
+      /*is_password=*/true, ASCIIToUTF16(kAlicePassword), mock_callback.Get());
   CheckTextFieldsDOMState(std::string(), false, std::string(), false);
 
   // When a password field is focus, the filling works.
   SimulateElementClick(kPasswordName);
-  password_autofill_agent_->FillIntoFocusedField(/*is_password=*/true,
-                                                 ASCIIToUTF16(kAlicePassword));
+
+  EXPECT_CALL(mock_callback, Run(FillingStatus::SUCCESS));
+  password_autofill_agent_->FillIntoFocusedField(
+      /*is_password=*/true, ASCIIToUTF16(kAlicePassword), mock_callback.Get());
   CheckTextFieldsDOMState(std::string(), false, kAlicePassword, true);
 }
 
@@ -1719,8 +1731,11 @@ TEST_F(PasswordAutofillAgentTest, FillIntoFocusedFieldForNonClickFocus) {
   // The completion should now affect ONLY the password field. Don't fill a
   // password so the error on failure shows where the filling happened.
   // (see FillIntoFocusedFieldOnlyIntoPasswordFields).
-  password_autofill_agent_->FillIntoFocusedField(/*is_password=*/false,
-                                                 ASCIIToUTF16("TextToFill"));
+
+  base::MockCallback<base::OnceCallback<void(FillingStatus)>> mock_callback;
+  EXPECT_CALL(mock_callback, Run(FillingStatus::SUCCESS)).Times(1);
+  password_autofill_agent_->FillIntoFocusedField(
+      /*is_password=*/false, ASCIIToUTF16("TextToFill"), mock_callback.Get());
   CheckTextFieldsDOMState(std::string(), false, "TextToFill", true);
 }
 
@@ -2887,6 +2902,28 @@ TEST_F(PasswordAutofillAgentTest,
   EXPECT_FALSE(fake_driver_.called_password_form_submitted());
 }
 
+TEST_F(PasswordAutofillAgentTest, DriverIsInformedAboutUnfillableField) {
+  EXPECT_FALSE(fake_driver_.last_focused_element_was_fillable());
+  SimulateElementClick(kPasswordName);
+  fake_driver_.Flush();
+  EXPECT_TRUE(fake_driver_.last_focused_element_was_fillable());
+
+  SetElementReadOnly(username_element_, true);
+  SetFocused(username_element_);
+  fake_driver_.Flush();
+  EXPECT_FALSE(fake_driver_.last_focused_element_was_fillable());
+}
+
+TEST_F(PasswordAutofillAgentTest, DriverIsInformedAboutPasswordFields) {
+  SimulateElementClick(kUsernameName);
+  fake_driver_.Flush();
+  EXPECT_FALSE(fake_driver_.last_focused_input_was_password());
+
+  SimulateElementClick(kPasswordName);
+  fake_driver_.Flush();
+  EXPECT_TRUE(fake_driver_.last_focused_input_was_password());
+}
+
 // Tests that credential suggestions are autofilled on a password (and change
 // password) forms having either ambiguous or empty name.
 TEST_F(PasswordAutofillAgentTest,

components/autofill/content/common/autofill_agent.mojom

@@ -85,7 +85,8 @@ interface PasswordAutofillAgent {
   FillPasswordForm(int32 key, PasswordFormFillData form_data);
 
   // Fills the given |credential| into the last focused text input.
-  FillIntoFocusedField(bool is_password, mojo_base.mojom.String16 credential);
+  FillIntoFocusedField(bool is_password, mojo_base.mojom.String16 credential)
+      => (FillingStatus status);
 
   // Notification to start (|active| == true) or stop (|active| == false)
   // logging the decisions made about saving the password.

components/autofill/content/common/autofill_driver.mojom

@@ -133,6 +133,10 @@ interface PasswordManagerDriver {
   // username/password field is on.
   CheckSafeBrowsingReputation(
       url.mojom.Url form_action, url.mojom.Url frame_url);
+
+  // The focus changed to a different input in the same frame (e.g. tabbed from
+  // email to password field).
+  FocusedInputChanged(bool is_fillable, bool is_password_field);
 };
 
 // There is one instance of this interface per web contents in the browser

components/autofill/content/common/autofill_types.mojom

@@ -100,6 +100,13 @@ enum LabelSource {
   VALUE,
 };
 
+// autofill::FillingStatus
+enum FillingStatus {
+  SUCCESS,
+  ERROR_NO_VALID_FIELD,
+  ERROR_NOT_ALLOWED,
+};
+
 // autofill::FormFieldData
 struct FormFieldData {
   mojo_base.mojom.String16 label;

components/autofill/content/common/autofill_types.typemap

@@ -4,6 +4,7 @@
 
 mojom = "//components/autofill/content/common/autofill_types.mojom"
 public_headers = [
+  "//components/autofill/core/common/filling_status.h",
   "//components/autofill/core/common/form_data.h",
   "//components/autofill/core/common/form_data_predictions.h",
   "//components/autofill/core/common/form_field_data.h",
@@ -29,6 +30,7 @@ deps = [
 
 type_mappings = [
   "autofill.mojom.CheckStatus=autofill::FormFieldData::CheckStatus",
+  "autofill.mojom.FillingStatus=autofill::FillingStatus",
   "autofill.mojom.FormData=autofill::FormData",
   "autofill.mojom.FormDataPredictions=autofill::FormDataPredictions",
   "autofill.mojom.FormFieldData=autofill::FormFieldData",
@@ -43,10 +45,10 @@ type_mappings = [
   "autofill.mojom.PasswordFormGenerationData=autofill::PasswordFormGenerationData",
   "autofill.mojom.PasswordFormLayout=autofill::PasswordForm::Layout",
   "autofill.mojom.PasswordFormScheme=autofill::PasswordForm::Scheme",
+  "autofill.mojom.PasswordFormSubmissionIndicatorEvent=autofill::PasswordForm::SubmissionIndicatorEvent",
   "autofill.mojom.PasswordFormType=autofill::PasswordForm::Type",
   "autofill.mojom.PasswordGenerationUIData=autofill::password_generation::PasswordGenerationUIData",
   "autofill.mojom.RoleAttribute=autofill::FormFieldData::RoleAttribute",
-  "autofill.mojom.ValueElementPair=autofill::ValueElementPair",
-  "autofill.mojom.PasswordFormSubmissionIndicatorEvent=autofill::PasswordForm::SubmissionIndicatorEvent",
   "autofill.mojom.SubmissionSource=autofill::SubmissionSource",
+  "autofill.mojom.ValueElementPair=autofill::ValueElementPair",
 ]

components/autofill/content/common/autofill_types_struct_traits.cc

@@ -533,6 +533,41 @@ bool EnumTraits<autofill::mojom::LabelSource,
   return false;
 }
 
+// static
+autofill::mojom::FillingStatus
+EnumTraits<autofill::mojom::FillingStatus, autofill::FillingStatus>::ToMojom(
+    autofill::FillingStatus input) {
+  switch (input) {
+    case autofill::FillingStatus::SUCCESS:
+      return autofill::mojom::FillingStatus::SUCCESS;
+    case autofill::FillingStatus::ERROR_NO_VALID_FIELD:
+      return autofill::mojom::FillingStatus::ERROR_NO_VALID_FIELD;
+    case autofill::FillingStatus::ERROR_NOT_ALLOWED:
+      return autofill::mojom::FillingStatus::ERROR_NOT_ALLOWED;
+  }
+  NOTREACHED();
+  return autofill::mojom::FillingStatus::SUCCESS;
+}
+
+// static
+bool EnumTraits<autofill::mojom::FillingStatus, autofill::FillingStatus>::
+    FromMojom(autofill::mojom::FillingStatus input,
+              autofill::FillingStatus* output) {
+  switch (input) {
+    case autofill::mojom::FillingStatus::SUCCESS:
+      *output = autofill::FillingStatus::SUCCESS;
+      return true;
+    case autofill::mojom::FillingStatus::ERROR_NO_VALID_FIELD:
+      *output = autofill::FillingStatus::ERROR_NO_VALID_FIELD;
+      return true;
+    case autofill::mojom::FillingStatus::ERROR_NOT_ALLOWED:
+      *output = autofill::FillingStatus::ERROR_NOT_ALLOWED;
+      return true;
+  }
+  NOTREACHED();
+  return false;
+}
+
 // static
 bool StructTraits<
     autofill::mojom::FormFieldDataDataView,

components/autofill/content/common/autofill_types_struct_traits.h

@@ -120,6 +120,13 @@ struct EnumTraits<autofill::mojom::LabelSource,
                         autofill::FormFieldData::LabelSource* output);
 };
 
+template <>
+struct EnumTraits<autofill::mojom::FillingStatus, autofill::FillingStatus> {
+  static autofill::mojom::FillingStatus ToMojom(autofill::FillingStatus input);
+  static bool FromMojom(autofill::mojom::FillingStatus input,
+                        autofill::FillingStatus* output);
+};
+
 template <>
 struct StructTraits<autofill::mojom::FormFieldDataDataView,
                     autofill::FormFieldData> {

components/autofill/content/renderer/autofill_agent.cc

@@ -238,7 +238,6 @@ void AutofillAgent::FocusedNodeChanged(const WebNode& node) {
   }
 
   HidePopup();
-  last_input_element_.Reset();
 
   if (node.IsNull() || !node.IsElementNode()) {
     if (!last_interacted_form_.IsNull()) {
@@ -265,7 +264,6 @@ void AutofillAgent::FocusedNodeChanged(const WebNode& node) {
     return;
 
   element_ = *element;
-  last_input_element_ = *element;
 
   FormData form;
   FormFieldData field;
@@ -840,15 +838,10 @@ void AutofillAgent::FormControlElementClicked(
   last_clicked_form_control_element_was_focused_for_testing_ = was_focused;
   was_last_action_fill_ = false;
 
-  last_input_element_.Reset();
   const WebInputElement* input_element = ToWebInputElement(&element);
   if (!input_element && !form_util::IsTextAreaElement(element))
     return;
 
-  if (input_element) {
-    last_input_element_ = *input_element;
-  }
-
   ShowSuggestionsOptions options;
   options.autofill_on_empty_values = true;
   // Show full suggestions when clicking on an already-focused form field.
@@ -1018,7 +1011,6 @@ void AutofillAgent::ResetLastInteractedElements() {
   last_clicked_form_control_element_for_testing_.Reset();
   formless_elements_user_edited_.clear();
   provisionally_saved_form_.reset();
-  last_input_element_.Reset();
 }
 
 void AutofillAgent::UpdateLastInteractedForm(blink::WebFormElement form) {

components/autofill/content/renderer/autofill_agent.h

@@ -95,11 +95,6 @@ class AutofillAgent : public content::RenderFrameObserver,
     return weak_ptr_factory_.GetWeakPtr();
   }
 
-  // Returns the input element that was last focused.
-  blink::WebInputElement GetLastFocusedInput() const {
-    return last_input_element_;
-  }
-
   // FormTracker::Observer
   void OnProvisionallySaveForm(const blink::WebFormElement& form,
                                const blink::WebFormControlElement& element,
@@ -301,9 +296,6 @@ class AutofillAgent : public content::RenderFrameObserver,
   // Last form which was interacted with by the user.
   blink::WebFormElement last_interacted_form_;
 
-  // Last input element the user interacted with.
-  blink::WebInputElement last_input_element_;
-
   // When dealing with forms that don't use a <form> tag, we keep track of the
   // elements the user has modified so we can determine when submission occurs.
   std::set<blink::WebFormControlElement> formless_elements_user_edited_;

components/autofill/content/renderer/password_autofill_agent.cc

@@ -819,22 +819,22 @@ bool PasswordAutofillAgent::FillSuggestion(
 
 void PasswordAutofillAgent::FillIntoFocusedField(
     bool is_password,
-    const base::string16& credential) {
-  if (!autofill_agent_.get()) {
-    return;
-  }
-  blink::WebInputElement input = autofill_agent_.get()->GetLastFocusedInput();
-  if (input.IsNull() || (!input.IsTextField() || !IsElementEditable(input))) {
+    const base::string16& credential,
+    FillIntoFocusedFieldCallback callback) {
+  if (focused_input_element_.IsNull()) {
+    std::move(callback).Run(autofill::FillingStatus::ERROR_NO_VALID_FIELD);
     return;
   }
   if (is_password) {
-    if (!input.IsPasswordFieldForAutofill()) {
+    if (!focused_input_element_.IsPasswordFieldForAutofill()) {
+      std::move(callback).Run(autofill::FillingStatus::ERROR_NOT_ALLOWED);
       return;
     }
-    FillPasswordFieldAndSave(&input, credential);
+    FillPasswordFieldAndSave(&focused_input_element_, credential);
   } else {
-    FillField(&input, credential);
+    FillField(&focused_input_element_, credential);
   }
+  std::move(callback).Run(autofill::FillingStatus::SUCCESS);
 }
 
 void PasswordAutofillAgent::FillField(blink::WebInputElement* input,
@@ -1342,6 +1342,32 @@ void PasswordAutofillAgent::OnWillSubmitForm(
   }
 }
 
+void PasswordAutofillAgent::FocusedNodeChanged(const blink::WebNode& node) {
+  focused_input_element_.Reset();
+
+  if (node.IsNull() ||          // |node| is null <==> focus outside of frame.
+      !node.IsElementNode()) {  // Not a valid blink::WebElement.
+    GetPasswordManagerDriver()->FocusedInputChanged(
+        /*is_fillable=*/false, /*is_password_field=*/false);
+    return;
+  }
+
+  blink::WebElement web_element = node.ToConst<blink::WebElement>();
+  const WebInputElement* input = ToWebInputElement(&web_element);
+  if (!input) {
+    GetPasswordManagerDriver()->FocusedInputChanged(
+        /*is_fillable=*/false, /*is_password_field=*/false);
+    return;  // If the node isn't an element, don't even try to convert.
+  }
+  bool is_password = false;
+  bool is_fillable = input->IsTextField() && IsElementEditable(*input);
+  if (is_fillable) {
+    focused_input_element_ = *input;
+    is_password = focused_input_element_.IsPasswordFieldForAutofill();
+  }
+  GetPasswordManagerDriver()->FocusedInputChanged(is_fillable, is_password);
+}
+
 void PasswordAutofillAgent::OnDestruct() {
   binding_.Close();
   base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);

components/autofill/content/renderer/password_autofill_agent.h

@@ -87,7 +87,8 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
   void FillPasswordForm(int key,
                         const PasswordFormFillData& form_data) override;
   void FillIntoFocusedField(bool is_password,
-                            const base::string16& credential) override;
+                            const base::string16& credential,
+                            FillIntoFocusedFieldCallback callback) override;
   void SetLoggingState(bool active) override;
   void AutofillUsernameAndPasswordDataReceived(
       const FormsPredictionsMap& predictions) override;
@@ -246,6 +247,7 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
   void WillCommitProvisionalLoad() override;
   void DidCommitProvisionalLoad(bool is_new_navigation,
                                 bool is_same_document_navigation) override;
+  void FocusedNodeChanged(const blink::WebNode& node) override;
   void OnDestruct() override;
 
   // Scans the given frame for password forms and sends them up to the browser.
@@ -390,6 +392,10 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
 
   PasswordValueGatekeeper gatekeeper_;
 
+  // The currently focused input field. Not null if its a valid input that can
+  // be filled with a suggestions.
+  blink::WebInputElement focused_input_element_;
+
   // True indicates that user debug information should be logged.
   bool logging_state_active_;
 

components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc

@@ -78,6 +78,8 @@ class FakeContentPasswordManagerDriver : public mojom::PasswordManagerDriver {
   void CheckSafeBrowsingReputation(const GURL& form_action,
                                    const GURL& frame_url) override {}
 
+  void FocusedInputChanged(bool is_fillable, bool is_password_field) override {}
+
   // Records whether RecordSavePasswordProgress() gets called.
   bool called_record_save_;
   // Records data received via RecordSavePasswordProgress() call.

components/autofill/core/common/BUILD.gn

@@ -24,6 +24,7 @@ static_library("common") {
     "autofill_switches.h",
     "autofill_util.cc",
     "autofill_util.h",
+    "filling_status.h",
     "form_data.cc",
     "form_data.h",
     "form_data_predictions.cc",

components/autofill/core/common/filling_status.h

+// Copyright 2018 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.
+
+#ifndef COMPONENTS_AUTOFILL_CORE_COMMON_FILLING_STATUS_H_
+#define COMPONENTS_AUTOFILL_CORE_COMMON_FILLING_STATUS_H_
+
+namespace autofill {
+
+// Describes how a manual filling attempt ended.
+enum class FillingStatus {
+  SUCCESS,               // The selected credential was filled into the field.
+  ERROR_NO_VALID_FIELD,  // The focused field was invalid or focus was lost.
+  ERROR_NOT_ALLOWED,  // Filling not permitted (e.g. password in email field).
+};
+
+}  // namespace autofill
+
+#endif  // COMPONENTS_AUTOFILL_CORE_COMMON_FILLING_STATUS_H_

components/password_manager/content/browser/content_password_manager_driver.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/callback.h"
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/password_form.h"
@@ -104,8 +105,10 @@ void ContentPasswordManagerDriver::FillSuggestion(
 
 void ContentPasswordManagerDriver::FillIntoFocusedField(
     bool is_password,
-    const base::string16& credential) {
-  GetPasswordAutofillAgent()->FillIntoFocusedField(is_password, credential);
+    const base::string16& credential,
+    base::OnceCallback<void(autofill::FillingStatus)> compeleted_callback) {
+  GetPasswordAutofillAgent()->FillIntoFocusedField(
+      is_password, credential, std::move(compeleted_callback));
 }
 
 void ContentPasswordManagerDriver::PreviewSuggestion(

components/password_manager/content/browser/content_password_manager_driver.h

@@ -60,7 +60,9 @@ class ContentPasswordManagerDriver : public PasswordManagerDriver {
   void FillSuggestion(const base::string16& username,
                       const base::string16& password) override;
   void FillIntoFocusedField(bool is_password,
-                            const base::string16& credential) override;
+                            const base::string16& credential,
+                            base::OnceCallback<void(autofill::FillingStatus)>
+                                compeleted_callback) override;
   void PreviewSuggestion(const base::string16& username,
                          const base::string16& password) override;
   void ShowInitialPasswordAccountSuggestions(

components/password_manager/content/browser/content_password_manager_driver_unittest.cc

@@ -80,7 +80,8 @@ class FakePasswordAutofillAgent
   // autofill::mojom::PasswordAutofillAgent:
   MOCK_METHOD2(FillPasswordForm,
                void(int, const autofill::PasswordFormFillData&));
-  MOCK_METHOD2(FillIntoFocusedField, void(bool, const base::string16&));
+  MOCK_METHOD3(FillIntoFocusedField,
+               void(bool, const base::string16&, FillIntoFocusedFieldCallback));
 
   MOCK_METHOD0(BlacklistedFormFound, void());
 

components/password_manager/core/browser/password_manager_driver.h

@@ -8,9 +8,11 @@
 #include <map>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
+#include "components/autofill/core/common/filling_status.h"
 #include "components/autofill/core/common/password_form_field_prediction_map.h"
 
 namespace autofill {
@@ -67,9 +69,11 @@ class PasswordManagerDriver
                               const base::string16& password) = 0;
 
   // Tells the renderer to fill the given credential into the focused element.
+  // Always calls |completed_callback| with a status indicating success/error.
   virtual void FillIntoFocusedField(
       bool is_password,
-      const base::string16& user_provided_credential) {}
+      const base::string16& user_provided_credential,
+      base::OnceCallback<void(autofill::FillingStatus)> compeleted_callback) {}
 
   // Tells the driver to preview filling form with the |username| and
   // |password|.

content/public/test/test_renderer_host.cc

@@ -248,6 +248,12 @@ RenderViewHostTestHarness::CreateTestWebContents() {
 
   return TestWebContents::Create(GetBrowserContext(), std::move(instance));
 }
+void RenderViewHostTestHarness::FocusWebContentsOnMainFrame() {
+  TestWebContents* contents = static_cast<TestWebContents*>(web_contents());
+  auto* root = contents->GetFrameTree()->root();
+  contents->GetFrameTree()->SetFocusedFrame(
+      root, root->current_frame_host()->GetSiteInstance());
+}
 
 void RenderViewHostTestHarness::NavigateAndCommit(const GURL& url) {
   static_cast<TestWebContents*>(web_contents())->NavigateAndCommit(url);

content/public/test/test_renderer_host.h

@@ -242,6 +242,10 @@ class RenderViewHostTestHarness : public testing::Test {
   // WebContentsTester::NavigateAndCommit for details.
   void NavigateAndCommit(const GURL& url);
 
+  // Sets the focused frame to the main frame of the WebContents for tests that
+  // rely on the focused frame not being null.
+  void FocusWebContentsOnMainFrame();
+
  protected:
   // testing::Test
   void SetUp() override;