New issue
Advanced search Search tips

Issue 862989 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
vasi...@chromium.org
fhorschig@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

browser_tests recreate agent instead of injecting them

Project Member Reported by fhorschig@chromium.org, Jul 12 
Currently, the ChromeRenderViewTest::SetUp() calls the content::RenderViewTest::SetUp() and then proceeds to create the three test agents (autofill agent, password agent and password generation agent). 

This means, any event dispatched on creation of a render frame (like FocusedNodeChanged) will be sent to instances of the agents that is replaced shortly after with test agents.

For lazy-initialized mojo bindings (e.g. PasswordManagerDriver and PasswordManagerClient), this means the binding may or may not be bound twice, depending on how the agents react to RenderFrameObserver events.

A clean solution could for example use a factory to create the agents and tests could inject the correct test instances beforehand to avoid the redundant creation.
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 16 

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

commit 9c89633176c3495e679f269fd6666ae2f61b3630
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Mon Jul 16 16:09:07 2018

Use frame-associated bindings for PasswordManagerDriver

There is an assumption that a call to PasswordManagerClient would finish
before a call to PasswordManagerDriver although both are asynchronous.
With https://crrev.com/c/1124466, more async calls happened which most
likely caused an earlier use of PasswordManagerDriver and uncovered this
issue.

This CL fixes that issue by associating PasswordManagerDriver
(like the PasswordManagerClient) to a RenderFrameHost. In order to use the
WebContentsFrameBindingSet, the PasswordManagerDriver implementation
moves into ChromePasswordManagerClient (which was called for most of these
methods anyway - one layer of indirection begone!).

In addition, it provides a workaround for a newly found bug that is
loosely related (see crbug/862989): Even though the PasswordAutofillAgent
is recreated to be accessible in tests, it is only bound once (to the
test instance).
This removes cases where FakeContentPasswordManagerDriver would be bound
twice and the binding to TestPasswordAutofillAgent was lost.

Bug: 862989

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ic5f9b1c60850b8b2b3e452784a0cbc86d4fd7bd2
Reviewed-on: https://chromium-review.googlesource.com/1127995
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575291}
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/browser/password_manager/password_manager_browsertest.cc
[delete] https://crrev.com/1d0e022ecbcd3d4fec6fae77beaaa4834d9e31fa/chrome/renderer/autofill/fake_content_password_manager_driver.cc
[add] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[rename] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/test/BUILD.gn
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/chrome/test/base/chrome_render_view_test.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/autofill/content/renderer/password_generation_agent.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/content/browser/content_password_manager_driver_factory.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/content/browser/content_password_manager_driver_factory.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/core/browser/password_manager_client.cc
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/components/password_manager/core/browser/password_manager_client.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h
[modify] https://crrev.com/9c89633176c3495e679f269fd6666ae2f61b3630/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm
►

Sign in to add a comment