Fix the position of AutofillClient for PasswordAutofillManager when the focused form element is ins…

components/password_manager/content/browser/content_password_manager_driver.cc

 // Copyright 2014 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 "components/password_manager/content/browser/content_password_manager_driver.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"
 #include "components/password_manager/content/browser/bad_message.h"
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/password_manager/content/browser/visible_password_observer.h"
 #include "components/password_manager/core/browser/log_manager.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/ssl_status.h"
 #include "content/public/browser/web_contents.h"
 #include "net/cert/cert_status_flags.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 
 namespace password_manager {
 
 ContentPasswordManagerDriver::ContentPasswordManagerDriver(
     content::RenderFrameHost* render_frame_host,
@@ skipping 244 matching lines @@
       password_form, generation_field);
 }
 
 void ContentPasswordManagerDriver::ShowPasswordSuggestions(
     int key,
     base::i18n::TextDirection text_direction,
     const base::string16& typed_username,
     int options,
     const gfx::RectF& bounds) {
   password_autofill_manager_.OnShowPasswordSuggestions(
       key, text_direction, typed_username, options, bounds);

[Charlie Reis, 2017/03/16 16:51:49]

This likely has the same bug, right?

[estark, 2017/03/16 17:14:38]

+1, in my testing I saw this happen on regular autofill dropdowns too
(https://screenshot.googleplex.com/RV4eiq1nNo3).

I wonder if this is a problem for non-password dropdowns like credit cards too?

[EhsanK, 2017/03/16 20:39:00]

Implemented the same code. Would it be worthwhile to copy the same test for this
feature?

 }
 
 void ContentPasswordManagerDriver::ShowNotSecureWarning(
     base::i18n::TextDirection text_direction,
     const gfx::RectF& bounds) {
-  password_autofill_manager_.OnShowNotSecureWarning(text_direction, bounds);
+  content::RenderWidgetHostView* rwhv = render_frame_host_->GetView();

[EhsanK, 2017/03/16 07:06:58]

Not sure yet if |rwhv| can be nullptr.

[vabr (Chromium), 2017/03/16 08:25:49]

Looking at ContentAutofillDriver::TransformBoundingBoxToViewportCoordinates, the
null case should be handled.

[EhsanK, 2017/03/16 20:39:00]

Agreed. We should check. But not sure how this can happen. Maybe when we
navigate right after this is called by the renderer?

+  gfx::RectF transformed_bounds(
+      rwhv->TransformPointToRootCoordSpaceF(bounds.origin()), bounds.size());
+  password_autofill_manager_.OnShowNotSecureWarning(text_direction,

[EhsanK, 2017/03/16 06:47:33]

I looked to me better to apply the transform here as opposed to inside
AutofillClient. If I understand correctly, AutofillClient is page level so we
have to submit transformed points to it instead. WDYT?

[vabr (Chromium), 2017/03/16 08:25:49]

The browser process and the driver in particular seem like a good place to do
the transformation in, and it matches the non-password autofill
(ContentAutofillDriver::TransformBoundingBoxToViewportCoordinates).

[EhsanK, 2017/03/16 20:39:00]

Acknowledged.

+                                                    transformed_bounds);
 }
 
 void ContentPasswordManagerDriver::RecordSavePasswordProgress(
     const std::string& log) {
   client_->GetLogManager()->LogSavePasswordProgress(log);
 }
 
 bool ContentPasswordManagerDriver::CheckChildProcessSecurityPolicy(
     const GURL& url,
     BadMessageReason reason) {
@@ skipping 36 matching lines @@
 ContentPasswordManagerDriver::GetPasswordGenerationAgent() {
   if (!password_gen_agent_) {
     render_frame_host_->GetRemoteInterfaces()->GetInterface(
         mojo::MakeRequest(&password_gen_agent_));
   }
 
   return password_gen_agent_;
 }
 
 }  // namespace password_manager