New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 866616 link

Starred by 1 user
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: Jul 31
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 866708
issue 863962



Sign in to add a comment

Autofill IPC has implicit dependency on scheduling behavior

Project Member Reported by roc...@chromium.org, Jul 23 
In attempting to address  bug 863962  by changing Mojo message scheduling behavior, it was discovered that autofill breaks due to subtle implicit dependencies on timing with respect to frame IPCs (e.g. FrameHostMsg_Detach).

The best way to address this for now is to make autofill use Channel-associated interfaces.

Comment 1 by roc...@chromium.org, Jul 23

Blocking: 866708
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 31 

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

commit 26e4e80de6ec7fa293f67a20759a8606b24919db
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Jul 31 15:03:33 2018

Move associated interface helper impls to Blink

blink::AssocaitedInterfaceRegistry and blink::AssociatedInterfaceProvider
are defined in blink/public/common but have only been implemented within
content/common. This is inconvenient if e.g. Chrome sources want to
instantiate one of those types, which is in fact a useful thing for Chrome
to be able to do.

This moves the implementation of the types into Blink so that they are
concrete types rather than virual interfaces.

The immediate motivation here is a need to convert Chrome's Autofill
implementation to use associated interfaces.

Bug:  866616 
Change-Id: I9db0906ad0b09fa917f30b838163297703e6f3cd
Reviewed-on: https://chromium-review.googlesource.com/1147923
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579409}
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/browser/find_request_manager.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/child/child_thread_impl.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/child/child_thread_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/common/BUILD.gn
[delete] https://crrev.com/af99e6efb10ece96ab258804a41fb01669ad9c66/content/common/associated_interface_provider_impl.h
[delete] https://crrev.com/af99e6efb10ece96ab258804a41fb01669ad9c66/content/common/associated_interface_registry_impl.cc
[delete] https://crrev.com/af99e6efb10ece96ab258804a41fb01669ad9c66/content/common/associated_interface_registry_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/common/associated_interfaces.mojom
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/renderer/mojo/blink_interface_registry_impl.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/renderer/mojo/blink_interface_registry_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/renderer/render_frame_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/renderer/render_thread_impl.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/content/test/test_render_frame_host.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/common/BUILD.gn
[rename] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/common/associated_interfaces/associated_interface_provider.cc
[add] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/common/associated_interfaces/associated_interface_registry.cc
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/public/common/associated_interfaces/associated_interface_provider.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/public/common/associated_interfaces/associated_interface_registry.h
[modify] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/public/mojom/BUILD.gn
[add] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/public/mojom/associated_interfaces/OWNERS
[add] https://crrev.com/26e4e80de6ec7fa293f67a20759a8606b24919db/third_party/blink/public/mojom/associated_interfaces/associated_interfaces.mojom
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 31 

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

commit 0aad7ec9d6172ecd3c7606931c07e94cd7401b8e
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Jul 31 15:58:44 2018

Use Channel-associated interfaces for autofill IPC

This converts autofill and password manager use Channel-assocaited
interfaces for the AutofillDriver and AutofillAgent mojom interfaces.
This is necessary due to subtle timing dependencies between autofill
IPC and frame IPC (e.g. FrameHostMsg_Detach) which were exposed when
attempting to make changes to mojom message dispatch scheduling.

Bug:  866616 
Change-Id: I2b2f521673715b1a6aa7bbb59dad605306b175de
Reviewed-on: https://chromium-review.googlesource.com/1148587
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579420}
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/android_webview/browser/aw_content_browser_client.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/android_webview/renderer/DEPS
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/android_webview/renderer/aw_render_frame_ext.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/android_webview/renderer/aw_render_frame_ext.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/renderer/autofill/autofill_renderer_browsertest.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/renderer/autofill/form_autocomplete_browsertest.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/renderer/chrome_render_frame_observer.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/renderer/chrome_render_frame_observer.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/test/base/chrome_render_view_test.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/chrome/test/base/chrome_render_view_test.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/browser/DEPS
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/browser/content_autofill_driver.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/browser/content_autofill_driver.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/browser/content_autofill_driver_factory.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/browser/content_autofill_driver_factory.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/browser/content_autofill_driver_unittest.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/BUILD.gn
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/password_generation_agent.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/test_password_autofill_agent.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/test_password_autofill_agent.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/test_password_generation_agent.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/autofill/content/renderer/test_password_generation_agent.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/password_manager/content/browser/DEPS
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/content/public/renderer/render_frame_observer.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/content/public/renderer/render_frame_observer.h
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/0aad7ec9d6172ecd3c7606931c07e94cd7401b8e/third_party/blink/public/common/associated_interfaces/associated_interface_provider.h

Comment 4 by roc...@chromium.org, Jul 31

Status: Fixed (was: Started)
►

Sign in to add a comment