Commit 83c3f50b authored by Hajime Hoshi's avatar Hajime Hoshi Committed by Commit Bot

Re^2-land: Fix a leak of a document that is held by AutofillAgent

This is a retry of https://chromium-review.googlesource.com/c/chromium/src/+/908371,
with fixing the DCHECK (NOTREACHED) error. We were not sure when the
NOTREACHED is reached before the fix, but now this case becomes
deterministic (see the discussion at crbug.com/816533)

Note that this might still cause Telemetry regressions reported at
crbug.com/753071 before, but this is expected.

A document is leaked because AutofillAgent hold a lastly used element
even after navigation happens, and the document of the element remains
until a new (input) element is focused. This CL fixes this issue by
resetting the element in AutofillAgent when navigation happens.

Bug: 734427, 755489, 816533
Change-Id: I7eb2f9fb879d826297247c4c66e366ec0b73067c
Reviewed-on: https://chromium-review.googlesource.com/939224Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543015}
parent 6ba514d3
......@@ -193,14 +193,7 @@ void AutofillAgent::DidCommitProvisionalLoad(bool is_new_navigation,
// Navigation to a new page or a page refresh.
// Do Finch testing to see how much regressions are caused by this leak fix
// (crbug/753071).
std::string group_name =
base::FieldTrialList::FindFullName("FixDocumentLeakInAutofillAgent");
if (base::StartsWith(group_name, "enabled",
base::CompareCase::INSENSITIVE_ASCII)) {
element_.Reset();
}
element_.Reset();
form_cache_.Reset();
ResetLastInteractedElements();
......@@ -460,20 +453,17 @@ void AutofillAgent::ClearForm() {
}
void AutofillAgent::ClearPreviewedForm() {
if (!element_.IsNull()) {
if (password_autofill_agent_->DidClearAutofillSelection(element_))
return;
// TODO(crbug.com/816533): It is very rare, but it looks like the |element_|
// can be null if a provisional load was committed immediately prior to
// clearing the previewed form.
if (element_.IsNull())
return;
form_util::ClearPreviewedFormWithElement(element_,
was_query_node_autofilled_);
} else {
// TODO(isherman): There seem to be rare cases where this code *is*
// reachable: see [ http://crbug.com/96321#c6 ]. Ideally we would
// understand those cases and fix the code to avoid them. However, so far I
// have been unable to reproduce such a case locally. If you hit this
// NOTREACHED(), please file a bug against me.
NOTREACHED();
}
if (password_autofill_agent_->DidClearAutofillSelection(element_))
return;
form_util::ClearPreviewedFormWithElement(element_,
was_query_node_autofilled_);
}
void AutofillAgent::FillFieldWithValue(const base::string16& value) {
......
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