Merge "Revert "Revert "Split Java libraries per apex"""
diff --git a/README.md b/README.md
index 531ef4c..60d7d5a 100644
--- a/README.md
+++ b/README.md
@@ -236,6 +236,11 @@
 If no `default_visibility` property can be found then the module uses the
 global default of `//visibility:legacy_public`.
 
+The `visibility` property has no effect on a defaults module although it does
+apply to any non-defaults module that uses it. To set the visibility of a
+defaults module, use the `defaults_visibility` property on the defaults module;
+not to be confused with the `default_visibility` property on the package module.
+
 Once the build has been completely switched over to soong it is possible that a
 global refactoring will be done to change this to `//visibility:private` at
 which point all packages that do not currently specify a `default_visibility`
diff --git a/android/defaults.go b/android/defaults.go
index ae2c820..f489c02 100644
--- a/android/defaults.go
+++ b/android/defaults.go
@@ -42,9 +42,16 @@
 	d.defaultableProperties = props
 }
 
+// Interface that must be supported by any module to which defaults can be applied.
 type Defaultable interface {
+	// Get a pointer to the struct containing the Defaults property.
 	defaults() *defaultsProperties
+
+	// Set the property structures into which defaults will be added.
 	setProperties([]interface{})
+
+	// Apply defaults from the supplied Defaults to the property structures supplied to
+	// setProperties(...).
 	applyDefaults(TopDownMutatorContext, []Defaults)
 }
 
@@ -56,13 +63,25 @@
 var _ Defaultable = (*DefaultableModuleBase)(nil)
 
 func InitDefaultableModule(module DefaultableModule) {
-	module.(Defaultable).setProperties(module.(Module).GetProperties())
+	module.setProperties(module.(Module).GetProperties())
 
 	module.AddProperties(module.defaults())
 }
 
+// The Defaults_visibility property.
+type DefaultsVisibilityProperties struct {
+
+	// Controls the visibility of the defaults module itself.
+	Defaults_visibility []string
+}
+
 type DefaultsModuleBase struct {
 	DefaultableModuleBase
+
+	// Container for defaults of the common properties
+	commonProperties commonProperties
+
+	defaultsVisibilityProperties DefaultsVisibilityProperties
 }
 
 // The common pattern for defaults modules is to register separate instances of
@@ -87,33 +106,75 @@
 // rather than disabling the defaults module itself.
 type Defaults interface {
 	Defaultable
+
+	// Although this function is unused it is actually needed to ensure that only modules that embed
+	// DefaultsModuleBase will type-assert to the Defaults interface.
 	isDefaults() bool
+
+	// Get the structures containing the properties for which defaults can be provided.
 	properties() []interface{}
+
+	// Return the defaults common properties.
+	common() *commonProperties
+
+	// Return the defaults visibility properties.
+	defaultsVisibility() *DefaultsVisibilityProperties
 }
 
 func (d *DefaultsModuleBase) isDefaults() bool {
 	return true
 }
 
+type DefaultsModule interface {
+	Module
+	Defaults
+}
+
 func (d *DefaultsModuleBase) properties() []interface{} {
 	return d.defaultableProperties
 }
 
+func (d *DefaultsModuleBase) common() *commonProperties {
+	return &d.commonProperties
+}
+
+func (d *DefaultsModuleBase) defaultsVisibility() *DefaultsVisibilityProperties {
+	return &d.defaultsVisibilityProperties
+}
+
 func (d *DefaultsModuleBase) GenerateAndroidBuildActions(ctx ModuleContext) {
 }
 
-func InitDefaultsModule(module DefaultableModule) {
+func InitDefaultsModule(module DefaultsModule) {
+	commonProperties := module.common()
+
 	module.AddProperties(
 		&hostAndDeviceProperties{},
-		&commonProperties{},
+		commonProperties,
 		&variableProperties{})
 
 	InitArchModule(module)
 	InitDefaultableModule(module)
 
-	module.AddProperties(&module.base().nameProperties)
+	// Add properties that will not have defaults applied to them.
+	base := module.base()
+	defaultsVisibility := module.defaultsVisibility()
+	module.AddProperties(&base.nameProperties, defaultsVisibility)
 
-	module.base().module = module
+	// The defaults_visibility property controls the visibility of a defaults module.
+	base.primaryVisibilityProperty =
+		newVisibilityProperty("defaults_visibility", &defaultsVisibility.Defaults_visibility)
+
+	// Unlike non-defaults modules the visibility property is not stored in m.base().commonProperties.
+	// Instead it is stored in a separate instance of commonProperties created above so use that.
+	// The visibility property needs to be checked (but not parsed) by the visibility module during
+	// its checking phase and parsing phase.
+	base.visibilityPropertyInfo = []visibilityProperty{
+		base.primaryVisibilityProperty,
+		newVisibilityProperty("visibility", &commonProperties.Visibility),
+	}
+
+	base.module = module
 }
 
 var _ Defaults = (*DefaultsModuleBase)(nil)
diff --git a/android/module.go b/android/module.go
index adb9454..138b9cd 100644
--- a/android/module.go
+++ b/android/module.go
@@ -211,6 +211,9 @@
 
 	// Get information about the properties that can contain visibility rules.
 	visibilityProperties() []visibilityProperty
+
+	// Get the visibility rules that control the visibility of this module.
+	visibility() []string
 }
 
 // Qualified id for a module
@@ -302,6 +305,11 @@
 	// If no `default_visibility` property can be found then the module uses the
 	// global default of `//visibility:legacy_public`.
 	//
+	// The `visibility` property has no effect on a defaults module although it does
+	// apply to any non-defaults module that uses it. To set the visibility of a
+	// defaults module, use the `defaults_visibility` property on the defaults module;
+	// not to be confused with the `default_visibility` property on the package module.
+	//
 	// See https://android.googlesource.com/platform/build/soong/+/master/README.md#visibility for
 	// more details.
 	Visibility []string
@@ -503,6 +511,12 @@
 		&base.variableProperties)
 	base.generalProperties = m.GetProperties()
 	base.customizableProperties = m.GetProperties()
+
+	// The default_visibility property needs to be checked and parsed by the visibility module during
+	// its checking and parsing phases.
+	base.primaryVisibilityProperty =
+		newVisibilityProperty("visibility", &base.commonProperties.Visibility)
+	base.visibilityPropertyInfo = []visibilityProperty{base.primaryVisibilityProperty}
 }
 
 func InitAndroidArchModule(m Module, hod HostOrDeviceSupported, defaultMultilib Multilib) {
@@ -582,6 +596,13 @@
 	archProperties          [][]interface{}
 	customizableProperties  []interface{}
 
+	// Information about all the properties on the module that contains visibility rules that need
+	// checking.
+	visibilityPropertyInfo []visibilityProperty
+
+	// The primary visibility property, may be nil, that controls access to the module.
+	primaryVisibilityProperty visibilityProperty
+
 	noAddressSanitizer bool
 	installFiles       Paths
 	checkbuildFiles    Paths
@@ -668,10 +689,15 @@
 }
 
 func (m *ModuleBase) visibilityProperties() []visibilityProperty {
-	return []visibilityProperty{
-		newVisibilityProperty("visibility", func() []string {
-			return m.base().commonProperties.Visibility
-		}),
+	return m.visibilityPropertyInfo
+}
+
+func (m *ModuleBase) visibility() []string {
+	// The soong_namespace module does not initialize the primaryVisibilityProperty.
+	if m.primaryVisibilityProperty != nil {
+		return m.primaryVisibilityProperty.getStrings()
+	} else {
+		return nil
 	}
 }
 
diff --git a/android/package.go b/android/package.go
index 03f6a1e..880d6a9 100644
--- a/android/package.go
+++ b/android/package.go
@@ -64,16 +64,6 @@
 	return newPackageId(ctx.ModuleDir())
 }
 
-// Override to ensure that the default_visibility rules are checked by the visibility module during
-// its checking phase.
-func (p *packageModule) visibilityProperties() []visibilityProperty {
-	return []visibilityProperty{
-		newVisibilityProperty("default_visibility", func() []string {
-			return p.properties.Default_visibility
-		}),
-	}
-}
-
 func (p *packageModule) Name() string {
 	return p.properties.Name
 }
@@ -97,6 +87,13 @@
 	module.properties.Name = name
 
 	module.AddProperties(&module.properties)
+
+	// The default_visibility property needs to be checked and parsed by the visibility module during
+	// its checking and parsing phases.
+	module.primaryVisibilityProperty =
+		newVisibilityProperty("default_visibility", &module.properties.Default_visibility)
+	module.visibilityPropertyInfo = []visibilityProperty{module.primaryVisibilityProperty}
+
 	return module
 }
 
diff --git a/android/prebuilt_etc.go b/android/prebuilt_etc.go
index 069e1f5..9722a25 100644
--- a/android/prebuilt_etc.go
+++ b/android/prebuilt_etc.go
@@ -91,6 +91,10 @@
 	return PathForModuleSrc(ctx, String(p.properties.Src))
 }
 
+func (p *PrebuiltEtc) InstallDirPath() OutputPath {
+	return p.installDirPath
+}
+
 // This allows other derivative modules (e.g. prebuilt_etc_xml) to perform
 // additional steps (like validating the src) before the file is installed.
 func (p *PrebuiltEtc) SetAdditionalDependencies(paths Paths) {
@@ -165,15 +169,16 @@
 	}
 }
 
-func InitPrebuiltEtcModule(p *PrebuiltEtc) {
+func InitPrebuiltEtcModule(p *PrebuiltEtc, dirBase string) {
+	p.installDirBase = dirBase
 	p.AddProperties(&p.properties)
 }
 
 // prebuilt_etc is for a prebuilt artifact that is installed in
 // <partition>/etc/<sub_dir> directory.
 func PrebuiltEtcFactory() Module {
-	module := &PrebuiltEtc{installDirBase: "etc"}
-	InitPrebuiltEtcModule(module)
+	module := &PrebuiltEtc{}
+	InitPrebuiltEtcModule(module, "etc")
 	// This module is device-only
 	InitAndroidArchModule(module, DeviceSupported, MultilibFirst)
 	return module
@@ -182,8 +187,8 @@
 // prebuilt_etc_host is for a host prebuilt artifact that is installed in
 // $(HOST_OUT)/etc/<sub_dir> directory.
 func PrebuiltEtcHostFactory() Module {
-	module := &PrebuiltEtc{installDirBase: "etc"}
-	InitPrebuiltEtcModule(module)
+	module := &PrebuiltEtc{}
+	InitPrebuiltEtcModule(module, "etc")
 	// This module is host-only
 	InitAndroidArchModule(module, HostSupported, MultilibCommon)
 	return module
@@ -192,8 +197,8 @@
 // prebuilt_usr_share is for a prebuilt artifact that is installed in
 // <partition>/usr/share/<sub_dir> directory.
 func PrebuiltUserShareFactory() Module {
-	module := &PrebuiltEtc{installDirBase: "usr/share"}
-	InitPrebuiltEtcModule(module)
+	module := &PrebuiltEtc{}
+	InitPrebuiltEtcModule(module, "usr/share")
 	// This module is device-only
 	InitAndroidArchModule(module, DeviceSupported, MultilibFirst)
 	return module
@@ -202,8 +207,8 @@
 // prebuild_usr_share_host is for a host prebuilt artifact that is installed in
 // $(HOST_OUT)/usr/share/<sub_dir> directory.
 func PrebuiltUserShareHostFactory() Module {
-	module := &PrebuiltEtc{installDirBase: "usr/share"}
-	InitPrebuiltEtcModule(module)
+	module := &PrebuiltEtc{}
+	InitPrebuiltEtcModule(module, "usr/share")
 	// This module is host-only
 	InitAndroidArchModule(module, HostSupported, MultilibCommon)
 	return module
@@ -254,8 +259,8 @@
 
 // prebuilt_font installs a font in <partition>/fonts directory.
 func PrebuiltFontFactory() Module {
-	module := &PrebuiltEtc{installDirBase: "fonts"}
-	InitPrebuiltEtcModule(module)
+	module := &PrebuiltEtc{}
+	InitPrebuiltEtcModule(module, "fonts")
 	// This module is device-only
 	InitAndroidArchModule(module, DeviceSupported, MultilibFirst)
 	return module
@@ -265,8 +270,9 @@
 // If soc_specific property is set to true, the firmware file is installed to the vendor <partition>/firmware
 // directory for vendor image.
 func PrebuiltFirmwareFactory() Module {
-	module := &PrebuiltEtc{installDirBase: "etc/firmware", socInstallDirBase: "firmware"}
-	InitPrebuiltEtcModule(module)
+	module := &PrebuiltEtc{}
+	module.socInstallDirBase = "firmware"
+	InitPrebuiltEtcModule(module, "etc/firmware")
 	// This module is device-only
 	InitAndroidArchModule(module, DeviceSupported, MultilibFirst)
 	return module
diff --git a/android/visibility.go b/android/visibility.go
index 94af343..a7e718b 100644
--- a/android/visibility.go
+++ b/android/visibility.go
@@ -67,8 +67,8 @@
 
 // Describes the properties provided by a module that contain visibility rules.
 type visibilityPropertyImpl struct {
-	name          string
-	stringsGetter func() []string
+	name            string
+	stringsProperty *[]string
 }
 
 type visibilityProperty interface {
@@ -76,10 +76,10 @@
 	getStrings() []string
 }
 
-func newVisibilityProperty(name string, stringsGetter func() []string) visibilityProperty {
+func newVisibilityProperty(name string, stringsProperty *[]string) visibilityProperty {
 	return visibilityPropertyImpl{
-		name:          name,
-		stringsGetter: stringsGetter,
+		name:            name,
+		stringsProperty: stringsProperty,
 	}
 }
 
@@ -88,7 +88,7 @@
 }
 
 func (p visibilityPropertyImpl) getStrings() []string {
-	return p.stringsGetter()
+	return *p.stringsProperty
 }
 
 // A compositeRule is a visibility rule composed from a list of atomic visibility rules.
@@ -211,16 +211,7 @@
 // Checks the per-module visibility rule lists before defaults expansion.
 func visibilityRuleChecker(ctx BottomUpMutatorContext) {
 	qualified := createQualifiedModuleName(ctx)
-	if d, ok := ctx.Module().(Defaults); ok {
-		// Defaults modules don't store the payload properties in m.base().
-		for _, props := range d.properties() {
-			if cp, ok := props.(*commonProperties); ok {
-				if visibility := cp.Visibility; visibility != nil {
-					checkRules(ctx, qualified.pkg, "visibility", visibility)
-				}
-			}
-		}
-	} else if m, ok := ctx.Module().(Module); ok {
+	if m, ok := ctx.Module().(Module); ok {
 		visibilityProperties := m.visibilityProperties()
 		for _, p := range visibilityProperties {
 			if visibility := p.getStrings(); visibility != nil {
@@ -294,14 +285,12 @@
 	qualifiedModuleId := m.qualifiedModuleId(ctx)
 	currentPkg := qualifiedModuleId.pkg
 
-	// Parse all the properties into rules and store them.
-	visibilityProperties := m.visibilityProperties()
-	for _, p := range visibilityProperties {
-		if visibility := p.getStrings(); visibility != nil {
-			rule := parseRules(ctx, currentPkg, visibility)
-			if rule != nil {
-				moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule)
-			}
+	// Parse the visibility rules that control access to the module and store them by id
+	// for use when enforcing the rules.
+	if visibility := m.visibility(); visibility != nil {
+		rule := parseRules(ctx, currentPkg, m.visibility())
+		if rule != nil {
+			moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule)
 		}
 	}
 }
diff --git a/android/visibility_test.go b/android/visibility_test.go
index af6acf4..c44dc9e 100644
--- a/android/visibility_test.go
+++ b/android/visibility_test.go
@@ -658,6 +658,40 @@
 				` visible to this module`,
 		},
 	},
+
+	// Defaults module's defaults_visibility tests
+	{
+		name: "defaults_visibility invalid",
+		fs: map[string][]byte{
+			"top/Blueprints": []byte(`
+				mock_defaults {
+					name: "top_defaults",
+					defaults_visibility: ["//visibility:invalid"],
+				}`),
+		},
+		expectedErrors: []string{
+			`defaults_visibility: unrecognized visibility rule "//visibility:invalid"`,
+		},
+	},
+	{
+		name: "defaults_visibility overrides package default",
+		fs: map[string][]byte{
+			"top/Blueprints": []byte(`
+				package {
+					default_visibility: ["//visibility:private"],
+				}
+				mock_defaults {
+					name: "top_defaults",
+					defaults_visibility: ["//visibility:public"],
+				}`),
+			"outsider/Blueprints": []byte(`
+				mock_library {
+					name: "liboutsider",
+					defaults: ["top_defaults"],
+				}`),
+		},
+	},
+
 	// Package default_visibility tests
 	{
 		name: "package default_visibility property is checked",
diff --git a/cc/library.go b/cc/library.go
index 24985df..7cbfdb5 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -33,7 +33,7 @@
 
 type StaticSharedLibraryProperties struct {
 	Srcs   []string `android:"path,arch_variant"`
-	Cflags []string `android:"path,arch_variant"`
+	Cflags []string `android:"arch_variant"`
 
 	Enabled            *bool    `android:"arch_variant"`
 	Whole_static_libs  []string `android:"arch_variant"`
diff --git a/xml/xml.go b/xml/xml.go
index 7c670fb..3a680ec 100644
--- a/xml/xml.go
+++ b/xml/xml.go
@@ -71,10 +71,6 @@
 	return android.PathForModuleOut(ctx, p.PrebuiltEtc.SourceFilePath(ctx).Base()+"-timestamp")
 }
 
-func (p *prebuiltEtcXml) DepsMutator(ctx android.BottomUpMutatorContext) {
-	p.PrebuiltEtc.DepsMutator(ctx)
-}
-
 func (p *prebuiltEtcXml) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	p.PrebuiltEtc.GenerateAndroidBuildActions(ctx)
 
@@ -125,9 +121,8 @@
 func PrebuiltEtcXmlFactory() android.Module {
 	module := &prebuiltEtcXml{}
 	module.AddProperties(&module.properties)
-
-	android.InitPrebuiltEtcModule(&module.PrebuiltEtc)
+	android.InitPrebuiltEtcModule(&module.PrebuiltEtc, "etc")
 	// This module is device-only
-	android.InitAndroidArchModule(module, android.DeviceSupported, android.MultilibCommon)
+	android.InitAndroidArchModule(module, android.DeviceSupported, android.MultilibFirst)
 	return module
 }
diff --git a/xml/xml_test.go b/xml/xml_test.go
index e8fa49c..ae3e9fe 100644
--- a/xml/xml_test.go
+++ b/xml/xml_test.go
@@ -34,6 +34,7 @@
 		"foo.dtd":    nil,
 		"bar.xml":    nil,
 		"bar.xsd":    nil,
+		"baz.xml":    nil,
 	}
 	ctx.MockFileSystem(mockFiles)
 	_, errs := ctx.ParseFileList(".", []string{"Android.bp"})
@@ -59,6 +60,13 @@
 	os.RemoveAll(buildDir)
 }
 
+func assertEqual(t *testing.T, name, expected, actual string) {
+	t.Helper()
+	if expected != actual {
+		t.Errorf(name+" expected %q != got %q", expected, actual)
+	}
+}
+
 // Minimal test
 func TestPrebuiltEtcXml(t *testing.T) {
 	ctx := testXml(t, `
@@ -72,15 +80,28 @@
 			src: "bar.xml",
 			schema: "bar.xsd",
 		}
+		prebuilt_etc_xml {
+			name: "baz.xml",
+			src: "baz.xml",
+		}
 	`)
 
-	xmllint := ctx.ModuleForTests("foo.xml", "android_common").Rule("xmllint")
-	input := xmllint.Input.String()
-	if input != "foo.xml" {
-		t.Errorf("input expected %q != got %q", "foo.xml", input)
+	for _, tc := range []struct {
+		rule, input, schemaType, schema string
+	}{
+		{rule: "xmllint-dtd", input: "foo.xml", schemaType: "dtd", schema: "foo.dtd"},
+		{rule: "xmllint-xsd", input: "bar.xml", schemaType: "xsd", schema: "bar.xsd"},
+		{rule: "xmllint-minimal", input: "baz.xml"},
+	} {
+		t.Run(tc.schemaType, func(t *testing.T) {
+			rule := ctx.ModuleForTests(tc.input, "android_arm64_armv8-a").Rule(tc.rule)
+			assertEqual(t, "input", tc.input, rule.Input.String())
+			if tc.schemaType != "" {
+				assertEqual(t, "schema", tc.schema, rule.Args[tc.schemaType])
+			}
+		})
 	}
-	schema := xmllint.Args["dtd"]
-	if schema != "foo.dtd" {
-		t.Errorf("dtd expected %q != got %q", "foo.dtdl", schema)
-	}
+
+	m := ctx.ModuleForTests("foo.xml", "android_arm64_armv8-a").Module().(*prebuiltEtcXml)
+	assertEqual(t, "installDir", "target/product/test_device/system/etc", m.InstallDirPath().RelPathString())
 }