Skip to content

Commit

Permalink
Move feature AutofillDetectRemovedFormControls from blink to autofill
Browse files Browse the repository at this point in the history
This feature can now be checked within autofill code. This CL therefore
moves it to the autofill codebase where it's relevant.

Bug: 1007974
Change-Id: I04cb06e9327e84cd2de87a3259c7295461a5d0a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5002542
Reviewed-by: Christoph Schwering <schwering@google.com>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Jihad Hanna <jihadghanna@google.com>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1224687}
  • Loading branch information
jihadghanna authored and Chromium LUCI CQ committed Nov 15, 2023
1 parent 4f20353 commit d47bcca
Show file tree
Hide file tree
Showing 11 changed files with 22 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ private ProductionSupportedFlagList() {}
AutofillFeatures.AUTOFILL_ENABLE_SUPPORT_FOR_PHONE_NUMBER_TRUNK_TYPES,
"Rationalizes city-and-number and city-code fields to the "
+ "correct trunk-prefix types."),
Flag.baseFeature(
AutofillFeatures.AUTOFILL_DETECT_REMOVED_FORM_CONTROLS,
"Enables Autofill to detect if form controls are removed from the DOM"),
Flag.baseFeature(
AutofillFeatures.AUTOFILL_DONT_PRESERVE_AUTOFILL_STATE,
"Retrieves is_autofilled state from blink instead of the cache"),
Expand Down Expand Up @@ -612,9 +615,6 @@ private ProductionSupportedFlagList() {}
Flag.baseFeature(
BlinkFeatures.AUTOFILL_USE_DOM_NODE_ID_FOR_RENDERER_ID,
"Enables Autofill to detect use DOM Node IDs for renderer IDs"),
Flag.baseFeature(
BlinkFeatures.AUTOFILL_DETECT_REMOVED_FORM_CONTROLS,
"Enables Autofill to detect if form controls are removed from the DOM"),
Flag.baseFeature(
NetFeatures.PARTITIONED_COOKIES, "Enables the Partitioned cookie attribute"),
Flag.baseFeature(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ class AutofillAcrossIframesTest_FullIframes
public:
AutofillAcrossIframesTest_FullIframes() {
feature_list_.InitAndEnableFeature(
blink::features::kAutofillDetectRemovedFormControls);
features::kAutofillDetectRemovedFormControls);
}

[[nodiscard]] const FormStructure* LoadForm() {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/autofill/autofill_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class AutofillTest : public InProcessBrowserTest {

AutofillTest() {
feature_list_.InitAndEnableFeature(
blink::features::kAutofillDetectRemovedFormControls);
features::kAutofillDetectRemovedFormControls);
}

void SetUpOnMainThread() override {
Expand Down
5 changes: 5 additions & 0 deletions components/autofill/content/renderer/autofill_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,11 @@ void AutofillAgent::HidePopup() {
void AutofillAgent::DidChangeFormRelatedElementDynamically(
const WebElement& element,
blink::WebFormRelatedChangeType form_related_change) {
if (form_related_change == blink::WebFormRelatedChangeType::kRemove &&
!base::FeatureList::IsEnabled(
features::kAutofillDetectRemovedFormControls)) {
return;
}
// If the control flow is here than the document was at least loaded. The
// whole page doesn't have to be loaded.
ExtractForms(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class AutofillAgentTestWithFeatures : public AutofillAgentTest {
scoped_features_.InitWithFeatures(
/*enabled_features=*/
{blink::features::kAutofillUseDomNodeIdForRendererId,
blink::features::kAutofillDetectRemovedFormControls,
features::kAutofillDetectRemovedFormControls,
features::kAutofillContentEditables},
/*disabled_features=*/{});
}
Expand Down
7 changes: 7 additions & 0 deletions components/autofill/core/common/autofill_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,13 @@ BASE_FEATURE(kAutofillExtractAllDatalists,
"AutofillExtractAllDatalists",
base::FEATURE_DISABLED_BY_DEFAULT);

// If enabled, whenever form controls are removed from the DOM, the ChromeClient
// is informed about this. This enables Autofill to trigger a reparsing of
// forms.
BASE_FEATURE(kAutofillDetectRemovedFormControls,
"AutofillDetectRemovedFormControls",
base::FEATURE_DISABLED_BY_DEFAULT);

// Replaces cached web elements in AutofillAgent and FormTracker by their
// renderer ids.
// DONOTSUMBIT: Disable.
Expand Down
2 changes: 2 additions & 0 deletions components/autofill/core/common/autofill_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ BASE_DECLARE_FEATURE(kAutofillExtractAllDatalists);
COMPONENT_EXPORT(AUTOFILL)
BASE_DECLARE_FEATURE(kAutofillEnableSupportForPhoneNumberTrunkTypes);
COMPONENT_EXPORT(AUTOFILL)
BASE_DECLARE_FEATURE(kAutofillDetectRemovedFormControls);
COMPONENT_EXPORT(AUTOFILL)
BASE_DECLARE_FEATURE(kAutofillReplaceCachedWebElementsByRendererIds);
COMPONENT_EXPORT(AUTOFILL)
BASE_DECLARE_FEATURE(kAutofillUseAddressRewriterInProfileSubsetComparison);
Expand Down
7 changes: 0 additions & 7 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,6 @@ BASE_FEATURE(kAudioWorkletThreadRealtimePriority,
"AudioWorkletThreadRealtimePriority",
base::FEATURE_ENABLED_BY_DEFAULT);

// If enabled, whenever form controls are removed from the DOM, the ChromeClient
// is informed about this. This enables Autofill to trigger a reparsing of
// forms.
BASE_FEATURE(kAutofillDetectRemovedFormControls,
"AutofillDetectRemovedFormControls",
base::FEATURE_DISABLED_BY_DEFAULT);

// If disabled (default for many years), autofilling triggers KeyDown and
// KeyUp events that do not send any key codes. If enabled, these events
// contain the "Unidentified" key.
Expand Down
3 changes: 0 additions & 3 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kAndroidExtendedKeyboardShortcuts);

BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kAudioWorkletThreadRealtimePriority);

BLINK_COMMON_EXPORT
BASE_DECLARE_FEATURE(kAutofillDetectRemovedFormControls);

BLINK_COMMON_EXPORT
BASE_DECLARE_FEATURE(kAutofillSendUnidentifiedKeyAfterFill);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,7 @@ void HTMLFormElement::RemovedFrom(ContainerNode& insertion_point) {
GetDocument().GetFormController().WillDeleteForm(this);
HTMLElement::RemovedFrom(insertion_point);

if (base::FeatureList::IsEnabled(
blink::features::kAutofillDetectRemovedFormControls) &&
insertion_point.isConnected()) {
if (insertion_point.isConnected()) {
GetDocument().DidChangeFormRelatedElementDynamically(
this, WebFormRelatedChangeType::kRemove);
}
Expand Down
4 changes: 1 addition & 3 deletions third_party/blink/renderer/core/html/forms/listed_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,7 @@ void ListedElement::RemovedFrom(ContainerNode& insertion_point) {

InvalidateShadowIncludingAncestorForms(insertion_point);

if (base::FeatureList::IsEnabled(
blink::features::kAutofillDetectRemovedFormControls) &&
insertion_point.isConnected()) {
if (insertion_point.isConnected()) {
// We don't insist on form_ being non-null as the form does not take care of
// reporting the removal.
element.GetDocument().DidChangeFormRelatedElementDynamically(
Expand Down

0 comments on commit d47bcca

Please sign in to comment.