diff --git a/components/autofill/content/renderer/autofill_agent.cc b/components/autofill/content/renderer/autofill_agent.cc index a663c48414a08d2d548f797b7404adb471fd5cda..62bc49cb26bf4497e7a0c284026fdb46303a9a5b 100644 --- a/components/autofill/content/renderer/autofill_agent.cc +++ b/components/autofill/content/renderer/autofill_agent.cc @@ -406,8 +406,16 @@ void AutofillAgent::FocusedElementChanged(const WebElement& element) { if ((IsKeyboardAccessoryEnabled() || !focus_requires_scroll_) && !element.IsNull() && element.GetDocument().GetFrame()->HasTransientUserActivation()) { - focused_node_was_last_clicked_ = true; - HandleFocusChangeComplete(); + // If the focus change was caused by a user gesture, + // DidReceiveLeftMouseDownOrGestureTapInNode() will show the autofill + // suggestions. See crbug.com/730764 for why showing autofill suggestions as + // a result of JavaScript changing focus is enabled on WebView. + bool focused_node_was_last_clicked = + !base::FeatureList::IsEnabled( + features::kAutofillAndroidDisableSuggestionsOnJSFocus) || + !focus_requires_scroll_; + HandleFocusChangeComplete( + /*focused_node_was_last_clicked=*/focused_node_was_last_clicked); } if (focus_moved_to_new_form) @@ -1079,7 +1087,10 @@ void AutofillAgent::DidCompleteFocusChangeInFrame() { } if (!IsKeyboardAccessoryEnabled() && focus_requires_scroll_) - HandleFocusChangeComplete(); + HandleFocusChangeComplete( + /*focused_node_was_last_clicked=*/ + last_left_mouse_down_or_gesture_tap_in_node_caused_focus_); + last_left_mouse_down_or_gesture_tap_in_node_caused_focus_ = false; SendPotentiallySubmittedFormToBrowser(); } @@ -1087,14 +1098,10 @@ void AutofillAgent::DidCompleteFocusChangeInFrame() { void AutofillAgent::DidReceiveLeftMouseDownOrGestureTapInNode( const WebNode& node) { DCHECK(!node.IsNull()); - focused_node_was_last_clicked_ = node.Focused(); - #if defined(ANDROID) - HandleFocusChangeComplete(); + HandleFocusChangeComplete(/*focused_node_was_last_clicked=*/node.Focused()); #else - if (!focus_requires_scroll_) { - HandleFocusChangeComplete(); - } + last_left_mouse_down_or_gesture_tap_in_node_caused_focus_ = node.Focused(); #endif } @@ -1194,7 +1201,8 @@ void AutofillAgent::FormControlElementClicked( SendPotentiallySubmittedFormToBrowser(); } -void AutofillAgent::HandleFocusChangeComplete() { +void AutofillAgent::HandleFocusChangeComplete( + bool focused_node_was_last_clicked) { WebElement focused_element = render_frame()->GetWebFrame()->GetDocument().FocusedElement(); // When using Talkback on Android, and possibly others, traversing to and @@ -1204,7 +1212,7 @@ void AutofillAgent::HandleFocusChangeComplete() { // When the focus is on a non-input field on Android, keyboard accessory may // be shown if autofill data is available. Make sure to hide the accessory if // focus changes to another element. - if ((focused_node_was_last_clicked_ || is_screen_reader_enabled_) && + if ((focused_node_was_last_clicked || is_screen_reader_enabled_) && !focused_element.IsNull() && focused_element.IsFormControlElement()) { WebFormControlElement focused_form_control_element = focused_element.To(); @@ -1217,8 +1225,6 @@ void AutofillAgent::HandleFocusChangeComplete() { GetAutofillDriver().HidePopup(); } - focused_node_was_last_clicked_ = false; - SendPotentiallySubmittedFormToBrowser(); } diff --git a/components/autofill/content/renderer/autofill_agent.h b/components/autofill/content/renderer/autofill_agent.h index 450911ef92cd6c5f3ebc0d8e7dcfd29c8b518867..36705263d6877317069539afce94abebd800b4d1 100644 --- a/components/autofill/content/renderer/autofill_agent.h +++ b/components/autofill/content/renderer/autofill_agent.h @@ -260,7 +260,7 @@ class AutofillAgent : public content::RenderFrameObserver, void FormElementReset(const blink::WebFormElement& form) override; void PasswordFieldReset(const blink::WebInputElement& element) override; - void HandleFocusChangeComplete(); + void HandleFocusChangeComplete(bool focused_node_was_last_clicked); void SendFocusedInputChangedNotificationToBrowser( const blink::WebElement& node); @@ -396,7 +396,7 @@ class AutofillAgent : public content::RenderFrameObserver, // doesn't use PasswordAutofillAgent to handle password form. bool query_password_suggestion_ = false; - bool focused_node_was_last_clicked_ = false; + bool last_left_mouse_down_or_gesture_tap_in_node_caused_focus_ = false; FieldRendererId last_clicked_form_control_element_for_testing_; FormTracker form_tracker_; diff --git a/components/autofill/core/common/autofill_features.cc b/components/autofill/core/common/autofill_features.cc index 2eebd38e9f948300803b29d01909de4214ad24e3..349cc1e610fe037100bffa6151469e1595c50db2 100644 --- a/components/autofill/core/common/autofill_features.cc +++ b/components/autofill/core/common/autofill_features.cc @@ -569,6 +569,18 @@ const base::FeatureParam kAutofillAblationStudyEnabledForPaymentsParam{ const base::FeatureParam kAutofillAblationStudyAblationWeightPerMilleParam{ &kAutofillEnableAblationStudy, "ablation_weight_per_mille", 10}; +// Controls whether user tap on an element is needed to show autofill +// suggestions. If enabled, this flag would disable android autofill suggestions +// if the focus on an element is Javascript-originated. +// DidReceiveLeftMouseDownOrGestureTapInNode() will show suggestions if the +// focus change occurred as a result of a gesture. See crbug.com/730764 for why +// showing autofill suggestions as a result of JavaScript changing focus is +// enabled on WebView. +// TODO(crbug.com/1496382) Clean up autofill feature flag +// `kAutofillAndroidDisableSuggestionsOnJSFocus` +extern const base::Feature kAutofillAndroidDisableSuggestionsOnJSFocus{ + "AutofillAndroidDisableSuggestionsOnJSFocus", base::FEATURE_DISABLED_BY_DEFAULT}; + // If enabled, crowdsourcing considers not just the value V but also the human // readable text HRT of an for voting. // TODO(crbug.com/1395740). This is a kill switch, remove once the feature has diff --git a/components/autofill/core/common/autofill_features.h b/components/autofill/core/common/autofill_features.h index d9e734e08db51668d72c6a6cea7122437718ef3f..96b845ece7d11c0be816f49b3a59e9b7f7ef744e 100644 --- a/components/autofill/core/common/autofill_features.h +++ b/components/autofill/core/common/autofill_features.h @@ -210,6 +210,9 @@ BASE_DECLARE_FEATURE(kAutofillVoteForSelectOptionValues); COMPONENT_EXPORT(AUTOFILL) extern const base::FeatureParam kAutofillMoreProminentPopupMaxOffsetToCenterParam; +COMPONENT_EXPORT(AUTOFILL) +extern const base::Feature + kAutofillAndroidDisableSuggestionsOnJSFocus; COMPONENT_EXPORT(AUTOFILL) BASE_DECLARE_FEATURE(kAutofillMoreProminentPopup); COMPONENT_EXPORT(AUTOFILL) BASE_DECLARE_FEATURE(kAutofillLogUKMEventsWithSampleRate); diff --git a/content/browser/devtools/protocol/network_handler.cc b/content/browser/devtools/protocol/network_handler.cc index a46d5171b9491c6dd90bc01ab2cba84156d87e3b..de941a55cc986d9d5b4966fd3b75c0b4c36e4763 100644 --- a/content/browser/devtools/protocol/network_handler.cc +++ b/content/browser/devtools/protocol/network_handler.cc @@ -109,7 +109,8 @@ using DeleteCookiesCallback = Network::Backend::DeleteCookiesCallback; using ClearBrowserCookiesCallback = Network::Backend::ClearBrowserCookiesCallback; -const char kInvalidCookieFields[] = "Invalid cookie fields"; +static constexpr char kInvalidCookieFields[] = "Invalid cookie fields"; +static constexpr char kNotAllowedError[] = "Not allowed"; Network::CertificateTransparencyCompliance SerializeCTPolicyCompliance( net::ct::CTPolicyCompliance ct_compliance) { @@ -1027,11 +1028,14 @@ NetworkHandler::NetworkHandler( const base::UnguessableToken& devtools_token, DevToolsIOContext* io_context, base::RepeatingClosure update_loader_factories_callback, - bool allow_file_access) + bool allow_file_access, + bool client_is_trusted) : DevToolsDomainHandler(Network::Metainfo::domainName), host_id_(host_id), devtools_token_(devtools_token), io_context_(io_context), + allow_file_access_(allow_file_access), + client_is_trusted_(client_is_trusted), browser_context_(nullptr), storage_partition_(nullptr), host_(nullptr), @@ -1042,8 +1046,7 @@ NetworkHandler::NetworkHandler( bypass_service_worker_(false), cache_disabled_(false), update_loader_factories_callback_( - std::move(update_loader_factories_callback)), - allow_file_access_(allow_file_access) { + std::move(update_loader_factories_callback)) { DCHECK(io_context_); static bool have_configured_service_worker_context = false; if (have_configured_service_worker_context) @@ -1505,6 +1508,9 @@ void NetworkHandler::GetCookies(Maybe> protocol_urls, void NetworkHandler::GetAllCookies( std::unique_ptr callback) { + if (!client_is_trusted_) { + callback->sendFailure(Response::ServerError(kNotAllowedError)); + } if (!storage_partition_) { callback->sendFailure(Response::InternalError()); return; diff --git a/content/browser/devtools/protocol/network_handler.h b/content/browser/devtools/protocol/network_handler.h index f966a2de04ce71d507412c45bfcba14e052d3687..50f0fb418959ffeb3016dc287b63440338a998a7 100644 --- a/content/browser/devtools/protocol/network_handler.h +++ b/content/browser/devtools/protocol/network_handler.h @@ -72,7 +72,8 @@ class NetworkHandler : public DevToolsDomainHandler, const base::UnguessableToken& devtools_token, DevToolsIOContext* io_context, base::RepeatingClosure update_loader_factories_callback, - bool allow_file_access); + bool allow_file_access, + bool client_is_trusted); NetworkHandler(const NetworkHandler&) = delete; NetworkHandler& operator=(const NetworkHandler&) = delete; @@ -337,6 +338,8 @@ class NetworkHandler : public DevToolsDomainHandler, const base::UnguessableToken devtools_token_; DevToolsIOContext* const io_context_; + const bool allow_file_access_; + const bool client_is_trusted_; std::unique_ptr frontend_; BrowserContext* browser_context_; @@ -358,7 +361,6 @@ class NetworkHandler : public DevToolsDomainHandler, loaders_; absl::optional> accepted_stream_types_; - const bool allow_file_access_; std::unordered_map> received_body_data_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/content/browser/devtools/render_frame_devtools_agent_host.cc b/content/browser/devtools/render_frame_devtools_agent_host.cc index cc4ab0d6998385f17fa21474de0e79046c714dca..ec3c2320d8688a5d6da2b3b62b1730d6968ae11a 100644 --- a/content/browser/devtools/render_frame_devtools_agent_host.cc +++ b/content/browser/devtools/render_frame_devtools_agent_host.cc @@ -339,7 +339,8 @@ bool RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session, base::BindRepeating( &RenderFrameDevToolsAgentHost::UpdateResourceLoaderFactories, base::Unretained(this)), - session->GetClient()->MayReadLocalFiles()); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); session->CreateAndAddHandler( GetIOContext(), base::BindRepeating( [](RenderFrameDevToolsAgentHost* self, diff --git a/content/browser/devtools/service_worker_devtools_agent_host.cc b/content/browser/devtools/service_worker_devtools_agent_host.cc index d2b307373ea1fdf62d5fcd956f5c7366690f6650..7278a116ec78c6751c995b05e7700c4b8dc6a83d 100644 --- a/content/browser/devtools/service_worker_devtools_agent_host.cc +++ b/content/browser/devtools/service_worker_devtools_agent_host.cc @@ -230,7 +230,8 @@ bool ServiceWorkerDevToolsAgentHost::AttachSession(DevToolsSession* session, session->CreateAndAddHandler(); session->CreateAndAddHandler( GetId(), devtools_worker_token_, GetIOContext(), base::DoNothing(), - session->GetClient()->MayReadLocalFiles()); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); session->CreateAndAddHandler( GetIOContext(), diff --git a/content/browser/devtools/shared_worker_devtools_agent_host.cc b/content/browser/devtools/shared_worker_devtools_agent_host.cc index 6cfb49a9cb6343505452b6a38bf63ec7204d2bb2..da9c8a3d18a4038a87decb2661d64707d3214215 100644 --- a/content/browser/devtools/shared_worker_devtools_agent_host.cc +++ b/content/browser/devtools/shared_worker_devtools_agent_host.cc @@ -91,7 +91,8 @@ bool SharedWorkerDevToolsAgentHost::AttachSession(DevToolsSession* session, session->CreateAndAddHandler(); session->CreateAndAddHandler( GetId(), devtools_worker_token_, GetIOContext(), - base::BindRepeating([] {}), session->GetClient()->MayReadLocalFiles()); + base::BindRepeating([] {}), session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); // TODO(crbug.com/1143100): support pushing updated loader factories down to // renderer. session->CreateAndAddHandler( diff --git a/content/browser/devtools/worker_devtools_agent_host.cc b/content/browser/devtools/worker_devtools_agent_host.cc index 5bca24a4bb1656532e4edf47e852eb9c3d84086b..dbce6e066adbea06f44a5b2f4c921bc21b3c06f7 100644 --- a/content/browser/devtools/worker_devtools_agent_host.cc +++ b/content/browser/devtools/worker_devtools_agent_host.cc @@ -137,7 +137,8 @@ bool WorkerDevToolsAgentHost::AttachSession(DevToolsSession* session, auto_attacher_.get(), session); session->CreateAndAddHandler( GetId(), devtools_worker_token_, GetIOContext(), base::DoNothing(), - session->GetClient()->MayReadLocalFiles()); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); return true; }