Skip to content

Commit

Permalink
[Autofill Assistant] Share RadioButtonControllers with nested instances.
Browse files Browse the repository at this point in the history 
This CL introduces a distinction between root and nested UI controllers.
For now, the only difference is that root controllers own an instance of
RadioButtonController, whereas nested instance will reuse their parents'
RadioButtonController. This allows nested UIs to incrementally add to an
existing radio button group. An example use case is added as integration
test.

Bug: b/145043394
Change-Id: I663633eff7fb363f33d7e6da99fb4a6dccf98beb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2256232
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Marian Fechete <marianfe@google.com>
Reviewed-by: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#785649}
  • Loading branch information
Clemens Arbesser authored and Commit Bot committed Jul 7, 2020
1 parent 619bb97 commit 72f9d0a
Show file tree
Hide file tree
Showing 14 changed files with 516 additions and 175 deletions.
254 changes: 206 additions & 48 deletions ...ts/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantGenericUiTest.java
View file

Large diffs are not rendered by default.

68 changes: 36 additions & 32 deletions chrome/browser/BUILD.gn
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# Copyright 2014 The Chromium Authors.All rights reserved.
# Use of this source code is governed by a BSD - style license that can be
# found in the LICENSE file.

import("//base/allocator/allocator.gni")
Expand Down Expand Up @@ -63,9 +63,10 @@ if (is_win) {
"cryptui.lib",
"dwmapi.lib",
"netapi32.lib",
"ndfapi.lib", # For browser/net/net_error_diagnostics_dialog_win.h
"pdh.lib", # For browser/private_working_set_snapshot.h
"wbemuuid.lib", # For browser/metrics/antivirus_metrics_provider_win.cc
"ndfapi.lib", # For browser / net / net_error_diagnostics_dialog_win.h
"pdh.lib", # For browser / private_working_set_snapshot.h
"wbemuuid.lib", # For browser / metrics /
# antivirus_metrics_provider_win.cc
]
ldflags = [
"/DELAYLOAD:ndfapi.dll",
Expand All @@ -83,14 +84,14 @@ buildflag_header("buildflags") {
]
}

# This proto library is used for non-android NTPs below.
# This proto library is used for non - android NTPs below.
proto_library("ntp_background_proto") {
sources = [ "search/background/ntp_background.proto" ]
generate_python = false
}

# Use a static library here because many test binaries depend on this but don't
# require many files from it. This makes linking more efficient.
# require many files from it.This makes linking more efficient.
static_library("browser") {
sources = [
"about_flags.cc",
Expand Down Expand Up @@ -2173,12 +2174,14 @@ static_library("browser") {
"android/autofill_assistant/assistant_overlay_delegate.h",
"android/autofill_assistant/client_android.cc",
"android/autofill_assistant/client_android.h",
"android/autofill_assistant/generic_ui_controller_android.cc",
"android/autofill_assistant/generic_ui_controller_android.h",
"android/autofill_assistant/generic_ui_events_android.cc",
"android/autofill_assistant/generic_ui_events_android.h",
"android/autofill_assistant/generic_ui_interactions_android.cc",
"android/autofill_assistant/generic_ui_interactions_android.h",
"android/autofill_assistant/generic_ui_nested_controller_android.cc",
"android/autofill_assistant/generic_ui_nested_controller_android.h",
"android/autofill_assistant/generic_ui_root_controller_android.cc",
"android/autofill_assistant/generic_ui_root_controller_android.h",
"android/autofill_assistant/interaction_handler_android.cc",
"android/autofill_assistant/interaction_handler_android.h",
"android/autofill_assistant/ui_controller_android.cc",
Expand Down Expand Up @@ -2925,7 +2928,8 @@ static_library("browser") {
"//components/resources:components_resources",
"//components/security_state/content/android",
"//components/send_tab_to_self",
"//components/signin/internal/identity_manager", # cf android/signin/DEPS
"//components/signin/internal/identity_manager", # cf android / signin /
# DEPS
"//components/subresource_filter/android",
"//components/viz/common",
"//ipc:param_traits",
Expand Down Expand Up @@ -2967,7 +2971,7 @@ static_library("browser") {
]
deps += [ "//chrome/android/modules/dev_ui/provider:native" ]
}
} else { # !is_android
} else { #!is_android
sources += [
"accessibility/caption_controller.cc",
"accessibility/caption_controller.h",
Expand Down Expand Up @@ -3579,7 +3583,7 @@ static_library("browser") {
"signin/signin_ui_util.h",
"speech/extension_api/tts_extension_api_constants.cc", # Should be moved
# to extensions
# section?
# section ?
"speech/extension_api/tts_extension_api_constants.h",
"speech/speech_recognition_service.cc",
"speech/speech_recognition_service.h",
Expand Down Expand Up @@ -3963,7 +3967,7 @@ static_library("browser") {
"notifications/notification_platform_bridge_chromeos.h",
]
}
} else { # Non-ChromeOS.
} else { # Non - ChromeOS.
sources += [
"fullscreen.h",
"policy/browser_signin_policy_handler.cc",
Expand Down Expand Up @@ -4236,7 +4240,7 @@ static_library("browser") {
]
}
} else {
# Non-Windows.
# Non - Windows.
sources += [
"profile_resetter/triggered_profile_resetter_stub.cc",
"profiles/profile_shortcut_manager_stub.cc",
Expand Down Expand Up @@ -4426,7 +4430,7 @@ static_library("browser") {
}

if (is_win || is_mac) {
# Sources (generally "desktop OS importers") used only on Mac & Windows.
# Sources(generally "desktop OS importers") used only on Mac & Windows.
sources += [
"recovery/recovery_install_global_error.cc",
"recovery/recovery_install_global_error.h",
Expand Down Expand Up @@ -4574,7 +4578,7 @@ static_library("browser") {
}

if (is_posix && !is_mac) {
# TODO(crbug.com/753619): Enable crash reporting on Fuchsia.
# TODO(crbug.com / 753619): Enable crash reporting on Fuchsia.
sources += [
"//chrome/app/chrome_crash_reporter_client.cc",
"//chrome/app/chrome_crash_reporter_client.h",
Expand Down Expand Up @@ -4619,8 +4623,8 @@ static_library("browser") {
"media_galleries/fileapi/mtp_file_stream_reader.h",
]
if (is_chromeos && use_dbus) {
# TODO(donna.wu@intel.com): push this into chrome/browser/chromeos
# and chrome/browser/media_galleries/chromeos
# TODO(donna.wu@intel.com): push this into chrome / browser / chromeos
# and chrome / browser / media_galleries / chromeos
deps += [ "//services/device/public/mojom" ]
}
}
Expand All @@ -4630,7 +4634,7 @@ static_library("browser") {
}

if (use_aura) {
# Cross-platform Aura files.
# Cross - platform Aura files.
sources += [
"download/drag_download_item_aura.cc",
"lifetime/application_lifetime_aura.cc",
Expand Down Expand Up @@ -4786,7 +4790,7 @@ static_library("browser") {
public_deps += [ "//chrome/common:service_process_mojom" ]
}
} else {
# Partial-only printing support.
# Partial - only printing support.
sources += [
"printing/print_view_manager_basic.cc",
"printing/print_view_manager_basic.h",
Expand Down Expand Up @@ -5052,15 +5056,15 @@ static_library("browser") {
"//chrome/browser/apps/platform_apps",

# TODO(https://crbug.com/883570): This is unfortunate, but not easy to
# fix. Ideally, //chrome/browser:browser shouldn't depend on these APIs
# (though the APIs likely will depend on //chrome/browser), but we need
# fix.Ideally, //chrome/browser:browser shouldn't depend on these APIs
#(though the APIs likely will depend on //chrome/browser), but we need
# to pull them in here to allow registration of keyed services.
"//chrome/browser/apps/platform_apps/api",

"//chrome/browser/extensions",
"//chrome/browser/web_applications",

# TODO(loyso): Erase these. crbug.com/877898.
# TODO(loyso): Erase these.crbug.com / 877898.
"//chrome/browser/web_applications:common",
"//chrome/browser/web_applications:web_applications_on_extensions",
"//chrome/browser/web_applications/components",
Expand Down Expand Up @@ -5295,10 +5299,10 @@ static_library("browser") {
"//components/offline_pages/core/request_header:request_header",
]

# Used to build test harness locally. The harness is used manually to
# Used to build test harness locally.The harness is used manually to
# produce multiple offline pages to evaluate quality of the snapshots.
# This will only be built iff. |enable_offline_pages_harness| is set while
# |enable_offline_pages| and |is_android| are both true.
# This will only be built iff. | enable_offline_pages_harness | is set while
# | enable_offline_pages | and | is_android | are both true.
if (enable_offline_pages_harness && is_android) {
sources += [
"offline_pages/android/evaluation/evaluation_test_scheduler.cc",
Expand Down Expand Up @@ -5611,8 +5615,8 @@ static_library("browser") {
}
if (enable_supervised_users && !is_android) {
sources += [
# TODO(bauerb): The legacy code should be removed (on desktop) once child
# account support has launched (https://crbug.com/505443).
# TODO(bauerb): The legacy code should be removed(on desktop) once child
# account support has launched(https://crbug.com/505443).
"supervised_user/legacy/custodian_profile_downloader_service.cc",
"supervised_user/legacy/custodian_profile_downloader_service.h",
"supervised_user/legacy/custodian_profile_downloader_service_factory.cc",
Expand Down Expand Up @@ -5791,7 +5795,7 @@ proto_library("permissions_proto") {
grit("resources") {
source = "browser_resources.grd"

# Required due to flattenhtml="true" on a generated file.
# Required due to flattenhtml = "true" on a generated file.
enable_input_discovery_for_gn_analyze = false

use_brotli = true
Expand Down Expand Up @@ -5827,7 +5831,7 @@ grit("resources") {
]

if (enable_plugins) {
# .json is not in the default sources_assignment_filter.
#.json is not in the default sources_assignment_filter.
if (is_chromeos) {
inputs = [ "resources/plugin_metadata/plugins_chromeos.json" ]
}
Expand Down Expand Up @@ -5973,7 +5977,7 @@ source_set("unexpire_flags") {
}

# Use a static library here because many test binaries depend on this but don't
# require many files from it. This makes linking more efficient.
# require many files from it.This makes linking more efficient.
static_library("test_support") {
testonly = true

Expand Down Expand Up @@ -6254,7 +6258,7 @@ static_library("test_support") {

if (safe_browsing_mode != 0) {
# "Safe Browsing Basic" files used for safe browsing in full mode
# (safe_browsing=1) and mobile (=2)
#(safe_browsing = 1) and mobile(=2)
sources += [
"safe_browsing/certificate_reporting_service_test_utils.cc",
"safe_browsing/certificate_reporting_service_test_utils.h",
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/android/autofill_assistant/generic_ui_interactions_android.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "base/android/jni_string.h"
#include "base/optional.h"
#include "chrome/android/features/autofill_assistant/jni_headers/AssistantViewInteractions_jni.h"
#include "chrome/browser/android/autofill_assistant/generic_ui_controller_android.h"
#include "chrome/browser/android/autofill_assistant/ui_controller_android_utils.h"
#include "chrome/browser/android/autofill_assistant/view_handler_android.h"
#include "components/autofill_assistant/browser/radio_button_controller.h"
Expand Down

0 comments on commit 72f9d0a

Please sign in to comment.