[Mfill Android] Ensure generation accepted message is sent to the same frame

A pointer to the PasswordManagerDriver is sent to the modal dialog, which
passes it back to the generation controller when the user accepts the
generated password. The driver is frame scoped and is used to communicate
back to the renderer that the password was accepted.

Bug:835234, 954196

Change-Id: If9ddf66f5466f5f22a90bbb5ff7897f28ce236df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1571664
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652186}

chrome/browser/autofill/manual_filling_controller_impl_unittest.cc

@@ -53,7 +53,9 @@ class MockPasswordGenerationController : public PasswordGenerationController {
            const base::WeakPtr<password_manager::PasswordManagerDriver>&));
   MOCK_METHOD0(OnGenerationElementLostFocus, void());
   MOCK_METHOD0(OnGenerationRequested, void());
-  MOCK_METHOD1(GeneratedPasswordAccepted, void(const base::string16&));
+  MOCK_METHOD2(GeneratedPasswordAccepted,
+               void(const base::string16&,
+                    base::WeakPtr<password_manager::PasswordManagerDriver>));
   MOCK_METHOD0(GeneratedPasswordRejected, void());
   MOCK_CONST_METHOD0(top_level_native_window, gfx::NativeWindow());
 };

chrome/browser/password_manager/password_generation_controller.h

@@ -69,7 +69,10 @@ class PasswordGenerationController {
   virtual void OnGenerationRequested() = 0;
 
   // Called from the modal dialog if the user accepted the generated password.
-  virtual void GeneratedPasswordAccepted(const base::string16& password) = 0;
+  // |driver| is used to communicate the message back to the renderer.
+  virtual void GeneratedPasswordAccepted(
+      const base::string16& password,
+      base::WeakPtr<password_manager::PasswordManagerDriver> driver) = 0;
 
   // Called from the modal dialog if the user rejected the generated password.
   virtual void GeneratedPasswordRejected() = 0;

chrome/browser/password_manager/password_generation_controller_impl.cc

@@ -97,6 +97,7 @@ void PasswordGenerationControllerImpl::OnGenerationElementLostFocus() {
   if (manual_filling_controller_ && generation_element_data_) {
     manual_filling_controller_->OnAutomaticGenerationStatusChanged(false);
   }
+  target_frame_driver_ = nullptr;
   generation_element_data_.reset();
 }
 
@@ -116,15 +117,17 @@ void PasswordGenerationControllerImpl::OnGenerationRequested() {
         ->ReportSpecPriorityForGeneratedPassword(generation_element_data_->form,
                                                  spec_priority);
   }
-  dialog_view_->Show(password);
+  dialog_view_->Show(password, std::move(target_frame_driver_));
+  target_frame_driver_ = nullptr;
 }
 
 void PasswordGenerationControllerImpl::GeneratedPasswordAccepted(
-    const base::string16& password) {
-  if (!target_frame_driver_)
+    const base::string16& password,
+    base::WeakPtr<password_manager::PasswordManagerDriver> driver) {
+  if (!driver)
     return;
   RecordGenerationDialogDismissal(true);
-  target_frame_driver_->GeneratedPasswordAccepted(password);
+  driver->GeneratedPasswordAccepted(password);
   dialog_view_.reset();
 }
 

chrome/browser/password_manager/password_generation_controller_impl.h

@@ -37,7 +37,9 @@ class PasswordGenerationControllerImpl
       override;
   void OnGenerationElementLostFocus() override;
   void OnGenerationRequested() override;
-  void GeneratedPasswordAccepted(const base::string16& password) override;
+  void GeneratedPasswordAccepted(
+      const base::string16& password,
+      base::WeakPtr<password_manager::PasswordManagerDriver> driver) override;
   void GeneratedPasswordRejected() override;
   gfx::NativeWindow top_level_native_window() const override;
 

chrome/browser/password_manager/password_generation_controller_impl_unittest.cc

@@ -39,6 +39,7 @@ class MockPasswordManagerDriver
 
   MOCK_METHOD0(GetPasswordGenerationHelper,
                password_manager::PasswordGenerationFrameHelper*());
+  MOCK_METHOD1(GeneratedPasswordAccepted, void(const base::string16&));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockPasswordManagerDriver);
@@ -68,7 +69,9 @@ class MockPasswordGenerationDialogView
  public:
   MockPasswordGenerationDialogView() = default;
 
-  MOCK_METHOD1(Show, void(base::string16&));
+  MOCK_METHOD2(Show,
+               void(base::string16&,
+                    base::WeakPtr<password_manager::PasswordManagerDriver>));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockPasswordGenerationDialogView);
@@ -102,6 +105,10 @@ PasswordGenerationUIData GetTestGenerationUIData2() {
   return data;
 }
 
+MATCHER_P(PointsToSameAddress, expected, "") {
+  return arg.get() == expected;
+}
+
 }  // namespace
 
 class PasswordGenerationControllerTest
@@ -236,7 +243,9 @@ TEST_F(PasswordGenerationControllerTest,
               GeneratePassword(_, form_signature, field_signature,
                                uint32_t(new_ui_data.max_length), _))
       .WillOnce(Return(generated_password));
-  EXPECT_CALL(*raw_dialog_view, Show(generated_password));
+  EXPECT_CALL(*raw_dialog_view,
+              Show(generated_password,
+                   PointsToSameAddress(mock_password_manager_driver_.get())));
   controller()->OnGenerationRequested();
 }
 
@@ -262,8 +271,30 @@ TEST_F(PasswordGenerationControllerTest, RecordsGeneratedPasswordAccepted) {
   base::HistogramTester histogram_tester;
 
   controller()->OnGenerationRequested();
-  controller()->GeneratedPasswordAccepted(test_password);
+  controller()->GeneratedPasswordAccepted(
+      test_password, mock_password_manager_driver_->AsWeakPtr());
 
   histogram_tester.ExpectUniqueSample(
       "KeyboardAccessory.GeneratedPasswordDialog", true, 1);
 }
+
+TEST_F(PasswordGenerationControllerTest,
+       RelaysGenerationAcceptedToCorrectDriver) {
+  base::string16 test_password = ASCIIToUTF16("t3stp@ssw0rd");
+
+  InitializeGeneration(test_password);
+
+  controller()->OnGenerationRequested();
+
+  MockPasswordManagerDriver wrong_driver;
+  EXPECT_CALL(mock_manual_filling_controller_,
+              OnAutomaticGenerationStatusChanged(true));
+  controller()->OnAutomaticGenerationAvailable(GetTestGenerationUIData2(),
+                                               wrong_driver.AsWeakPtr());
+
+  EXPECT_CALL(*mock_password_manager_driver_,
+              GeneratedPasswordAccepted(test_password));
+  EXPECT_CALL(wrong_driver, GeneratedPasswordAccepted(_)).Times(0);
+  controller()->GeneratedPasswordAccepted(
+      test_password, mock_password_manager_driver_->AsWeakPtr());
+}

chrome/browser/password_manager/password_generation_dialog_view_interface.h

@@ -5,16 +5,22 @@
 #ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_GENERATION_DIALOG_VIEW_INTERFACE_H_
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_GENERATION_DIALOG_VIEW_INTERFACE_H_
 
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
-
 class PasswordGenerationController;
 
+namespace password_manager {
+class PasswordManagerDriver;
+}  // namespace password_manager
+
 class PasswordGenerationDialogViewInterface {
  public:
   virtual ~PasswordGenerationDialogViewInterface() = default;
 
   // Called to show the dialog. |password| is the generated password.
-  virtual void Show(base::string16& password) = 0;
+  virtual void Show(base::string16& password,
+                    base::WeakPtr<password_manager::PasswordManagerDriver>
+                        target_frame_driver) = 0;
 
  private:
   friend class PasswordGenerationControllerImpl;

chrome/browser/ui/android/passwords/password_generation_dialog_view_android.cc

@@ -6,10 +6,12 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/password_manager/password_generation_controller_impl.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/strings/grit/components_strings.h"
 #include "jni/PasswordGenerationDialogBridge_jni.h"
 #include "ui/android/window_android.h"
@@ -32,7 +34,11 @@ PasswordGenerationDialogViewAndroid::~PasswordGenerationDialogViewAndroid() {
       base::android::AttachCurrentThread(), java_object_);
 }
 
-void PasswordGenerationDialogViewAndroid::Show(base::string16& password) {
+void PasswordGenerationDialogViewAndroid::Show(
+    base::string16& password,
+    base::WeakPtr<password_manager::PasswordManagerDriver>
+        target_frame_driver) {
+  target_frame_driver_ = std::move(target_frame_driver);
   JNIEnv* env = base::android::AttachCurrentThread();
 
   base::string16 explanation_text =
@@ -48,7 +54,8 @@ void PasswordGenerationDialogViewAndroid::PasswordAccepted(
     const base::android::JavaParamRef<jobject>& obj,
     const base::android::JavaParamRef<jstring>& password) {
   controller_->GeneratedPasswordAccepted(
-      base::android::ConvertJavaStringToUTF16(env, password));
+      base::android::ConvertJavaStringToUTF16(env, password),
+      std::move(target_frame_driver_));
 }
 
 void PasswordGenerationDialogViewAndroid::PasswordRejected(

chrome/browser/ui/android/passwords/password_generation_dialog_view_android.h

@@ -26,7 +26,9 @@ class PasswordGenerationDialogViewAndroid
   ~PasswordGenerationDialogViewAndroid() override;
 
   // Called to show the dialog. |password| is the generated password.
-  void Show(base::string16& password) override;
+  void Show(base::string16& password,
+            base::WeakPtr<password_manager::PasswordManagerDriver>
+                target_frame_driver) override;
 
   // Called from Java via JNI.
   void PasswordAccepted(JNIEnv* env,
@@ -44,6 +46,10 @@ class PasswordGenerationDialogViewAndroid
   // The corresponding java object.
   base::android::ScopedJavaGlobalRef<jobject> java_object_;
 
+  // The driver corresponding to the frame for which the generation request was
+  // made. Used to ensure that the accepted password message is sent back to the
+  // same frame.
+  base::WeakPtr<password_manager::PasswordManagerDriver> target_frame_driver_;
   DISALLOW_COPY_AND_ASSIGN(PasswordGenerationDialogViewAndroid);
 };