Commit 67aef3f3 authored by arthursonzogni's avatar arthursonzogni Committed by Commit Bot

[bfcache] Make Autofill to create driver on demand.

The Autofill component is associating an Autofill driver on every
RenderFrameHost by observing:
 - WebContentsObserver::RenderFrameCreated(render_frame_host)
 - WebContentsObserver::RenderFrameDeleted(render_frame_host)

The problem is that it starts observing when the WebContent is being
added to the tab strip. It means some RenderFrameCreated events are
missing.

To resolve that, this component was simulating receiving
RenderFrameCreated for every active frame. This is not sufficient,
because it misses RenderFrameHost pending deletion and the ones in the
BackForwardCache as well.

To fix that, create the Autofill driver on demand when they are needed.

Bug: 999842
Change-Id: I64e86f9b795f3a40dc5340d49132a120912b5530
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1826885Reviewed-by: default avatarLowell Manners <lowell@chromium.org>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700228}
parent 662e0bdb
...@@ -57,17 +57,10 @@ void ContentAutofillDriverFactory::CreateForWebContentsAndDelegate( ...@@ -57,17 +57,10 @@ void ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
if (FromWebContents(contents)) if (FromWebContents(contents))
return; return;
auto new_factory = std::make_unique<ContentAutofillDriverFactory>( contents->SetUserData(
contents, client, app_locale, enable_download_manager, provider); kContentAutofillDriverFactoryWebContentsUserDataKey,
const std::vector<content::RenderFrameHost*> frames = std::make_unique<ContentAutofillDriverFactory>(
contents->GetAllFrames(); contents, client, app_locale, enable_download_manager, provider));
for (content::RenderFrameHost* frame : frames) {
if (frame->IsRenderFrameLive())
new_factory->RenderFrameCreated(frame);
}
contents->SetUserData(kContentAutofillDriverFactoryWebContentsUserDataKey,
std::move(new_factory));
} }
// static // static
...@@ -115,17 +108,20 @@ ContentAutofillDriverFactory::ContentAutofillDriverFactory( ...@@ -115,17 +108,20 @@ ContentAutofillDriverFactory::ContentAutofillDriverFactory(
ContentAutofillDriver* ContentAutofillDriverFactory::DriverForFrame( ContentAutofillDriver* ContentAutofillDriverFactory::DriverForFrame(
content::RenderFrameHost* render_frame_host) { content::RenderFrameHost* render_frame_host) {
AutofillDriver* driver = DriverForKey(render_frame_host);
// ContentAutofillDriver are created on demand here.
if (!driver) {
AddForKey(render_frame_host,
base::Bind(CreateDriver, render_frame_host, client(), app_locale_,
enable_download_manager_, provider_));
driver = DriverForKey(render_frame_host);
}
// This cast is safe because AutofillDriverFactory::AddForKey is protected // This cast is safe because AutofillDriverFactory::AddForKey is protected
// and always called with ContentAutofillDriver instances within // and always called with ContentAutofillDriver instances within
// ContentAutofillDriverFactory. // ContentAutofillDriverFactory.
return static_cast<ContentAutofillDriver*>(DriverForKey(render_frame_host)); return static_cast<ContentAutofillDriver*>(driver);
}
void ContentAutofillDriverFactory::RenderFrameCreated(
content::RenderFrameHost* render_frame_host) {
AddForKey(render_frame_host,
base::Bind(CreateDriver, render_frame_host, client(), app_locale_,
enable_download_manager_, provider_));
} }
void ContentAutofillDriverFactory::RenderFrameDeleted( void ContentAutofillDriverFactory::RenderFrameDeleted(
......
...@@ -63,7 +63,6 @@ class ContentAutofillDriverFactory : public AutofillDriverFactory, ...@@ -63,7 +63,6 @@ class ContentAutofillDriverFactory : public AutofillDriverFactory,
content::RenderFrameHost* render_frame_host); content::RenderFrameHost* render_frame_host);
// content::WebContentsObserver: // content::WebContentsObserver:
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override; void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
void DidFinishNavigation( void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override; content::NavigationHandle* navigation_handle) override;
......
# These tests currently fail when run with --enable-features=BackForwardCache # These tests currently fail when run with --enable-features=BackForwardCache
# Autofill component issue. The AutofillDriver is associated with the
# RenderFrameHost, but is is used before it has been associated.
#
# https://crbug.com/999842
-TabsApiUnitTest.TabsGoForwardAndBack
-TabsApiUnitTest.TabsGoForwardAndBackWithoutTabId
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment