New issue
Advanced search Search tips

Issue 854149 link

Starred by 1 user
Status: Fixed
Owner:
fhorschig@chromium.org
Closed: Jul 18
Cc:
vasi...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 854150



Sign in to add a comment

Bind password sheets to input fields, not frames

Project Member Reported by fhorschig@chromium.org, Jun 19 2018 
Currently, a password sheet (contents) exists once per site (== bound to WebContens).

Ideally, a password sheet exists once per field.

Reasons:
 - <iframe>s would not need a special treatment as the information bound to their frame is scoped to their frame
 - Passwords would get immediate notice that they cannot be filled into non-password fields (instead of silently failing)

Comment 1 by fhorschig@chromium.org, Jun 19 2018

Blocking: 854150

Comment 2 by fhorschig@chromium.org, Jul 5

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6

commit 4c9cbfb597cd0014ad5200b0a94cf0f13c466af6
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Wed Jul 18 08:13:03 2018

[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}
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_agent.mojom
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/core/common/BUILD.gn
[add] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/core/common/filling_status.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/core/browser/password_manager_driver.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/content/public/test/test_renderer_host.h

Comment 4 by fhorschig@chromium.org, Jul 18

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82c224b0aa0eed806ded736f5ad3fed09257926e

commit 82c224b0aa0eed806ded736f5ad3fed09257926e
Author: Ian Clelland <iclelland@chromium.org>
Date: Wed Jul 18 14:25:19 2018

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

This reverts commit 4c9cbfb597cd0014ad5200b0a94cf0f13c466af6.

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}
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_agent.mojom
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/core/common/BUILD.gn
[delete] https://crrev.com/b644b96d5341d71f216f1ae6e551589903106aec/components/autofill/core/common/filling_status.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/core/browser/password_manager_driver.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/content/public/test/test_renderer_host.h
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6cab120126f8a1f7ddcaafb41d59e041a1c9e095

commit 6cab120126f8a1f7ddcaafb41d59e041a1c9e095
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Wed Jul 18 15:23:45 2018

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

This reverts commit 82c224b0aa0eed806ded736f5ad3fed09257926e.

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 4c9cbfb597cd0014ad5200b0a94cf0f13c466af6.
> 
> 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/1141885
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576066}
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_agent.mojom
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/core/common/BUILD.gn
[add] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/core/common/filling_status.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/core/browser/password_manager_driver.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/content/public/test/test_renderer_host.h
►

Sign in to add a comment