Skip to content

Commit

Permalink
Revert "[One .NET] $(AndroidEnablePreloadAssemblies)=False by default" (
Browse files Browse the repository at this point in the history
#5811)

Context: #5838

This reverts commit d13d0f9.

Commit d13d0f9 inadvertently broke the
[`DebuggingTest.ClassLibraryMainLauncherRuns()` unit test][0], when
using a test app which:

 1. Uses .NET 6, which -- because of d13d0f9 -- sets
    `$(AndroidEnablePreloadAssemblies)`=False by default -- with-

 2. A "Debug" build (Fast Deployment enabled), with-

 3. `$(AndroidLinkResources)`=False (the default; see also 9e6ce03),
    and-

 4. launching the app, which

 5. Loads an `Activity` subclass located in an assembly which is
    *not* the "main" App assembly

then the app crashes during process startup:

	android.runtime.JavaProxyThrowable: System.NullReferenceException: Object reference not set to an instance of an object
	    at MyLibrary.MainActivity.OnCreate(Bundle bundle)
	    at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState)
	    at com.xamarin.classlibrarymainlauncherruns.MainActivity.n_onCreate(Native Method)
	    at com.xamarin.classlibrarymainlauncherruns.MainActivity.onCreate(MainActivity.java:29)

The cause of the crash is that, because not all assemblies are loaded
as part of process startup -- the whole *point* to d13d0f9 and
defaulting `$(AndroidEnablePreloadAssemblies)`=False -- then
[`ResourceIdManager.UpdateIdValues()`][1] isn't able to update all
fields in all `Resource` types to have the values from the app's
`Resource.designer.cs` file.  Consequently, the values *may* be
invalid, and thus the `NullReferenceException`.

As an "immediate" fix is not quickly forthcoming -- though enabling
`$(AndroidLinkResources)`=False *by default* is an "interesting"
solution, with unknown inner dev loop performance implications --
we're reverting commit d13d0f9.

Issue #5838 will track the re-enabling of
`$(AndroidEnablePreloadAssemblies)`=False by default in .NET 6 apps.

~~~

Additionally, update `DebuggingTest.ClassLibraryMainLauncherRuns()`
so that `Resources\layout\foo.xml` contains *valid* layout XML.
It was originally:

	<?xml version="1.0" encoding="utf-8" ?>
	<LinearLayout
	  xmlns:android="http://schemas.android.com/apk/res/android"
	/>

which would crash with:

	Java.Lang.RuntimeException: Unable to start activity ComponentInfo{com.xamarin.classlibrarymainlauncherruns/com.xamarin.classlibrarymainlauncherruns.MainActivity}:
	  android.view.InflateException: Binary XML file line #1 in com.xamarin.classlibrarymainlauncherruns:layout/foo: Binary XML file line #1: You must supply a layout_width attribute.
	---> Android.Views.InflateException: Binary XML file line #1 in com.xamarin.classlibrarymainlauncherruns:layout/foo: Binary XML file line #1: You must supply a layout_width attribute.
	---> Java.Lang.UnsupportedOperationException: Binary XML file line #1: You must supply a layout_width attribute.
	--- End of managed Java.Lang.UnsupportedOperationException stack trace ---
	java.lang.UnsupportedOperationException: Binary XML file line #1: You must supply a layout_width attribute.

because the layout XML was, in fact, invalid, as it wasn't providing
the required `android:layout_width` attribute.

Update `foo.xml` so that it has valid layout XML:

	<?xml version="1.0" encoding="utf-8"?>
	<LinearLayout
	  xmlns:android="http://schemas.android.com/apk/res/android"
	  android:layout_width="fill_parent"
	  android:layout_height="wrap_content"
	/>"

This way we won't be chasing invalid XML going forward.

The above `UnsupportedOperationException` *hid* the
`NullReferenceException` in d13d0f9; the `NullReferenceException`
wasn't visible until after the invalid XML was fixed.

[0]: https://github.com/xamarin/xamarin-android/blob/bf4f4f42af26cdddb46087deed59aae8424f7942/tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs#L74-L124
[1]: https://github.com/xamarin/xamarin-android/blob/bf4f4f42af26cdddb46087deed59aae8424f7942/src/Mono.Android/Android.Runtime/ResourceIdManager.cs#L9-L31
  • Loading branch information
jonathanpeppers authored Apr 14, 2021
1 parent 65bf398 commit 522d7fb
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3932,11 +3932,7 @@ public void PackageNamingPolicy ([Values ("LowercaseMD5", "LowercaseCrc64")] str
Assert.IsTrue (b.Build (proj), "build should have succeeded.");
var environment = b.Output.GetIntermediaryPath (Path.Combine ("__environment__.txt"));
FileAssert.Exists (environment);
if (Builder.UseDotNet) {
Assert.AreEqual ($"__XA_PACKAGE_NAMING_POLICY__={packageNamingPolicy}{Environment.NewLine}mono.enable_assembly_preload=0", File.ReadAllText (environment).Trim ());
} else {
Assert.AreEqual ($"__XA_PACKAGE_NAMING_POLICY__={packageNamingPolicy}", File.ReadAllText (environment).Trim ());
}
Assert.AreEqual ($"__XA_PACKAGE_NAMING_POLICY__={packageNamingPolicy}", File.ReadAllText (environment).Trim ());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<AndroidMakeBundleKeepTemporaryFiles Condition=" '$(AndroidMakeBundleKeepTemporaryFiles)' == '' ">False</AndroidMakeBundleKeepTemporaryFiles>

<!-- If true it will cause all the assemblies in the apk to be preloaded on startup time -->
<_AndroidEnablePreloadAssembliesDefault Condition=" '$(UsingAndroidNETSdk)' == 'true' ">False</_AndroidEnablePreloadAssembliesDefault>
<_AndroidEnablePreloadAssembliesDefault Condition=" '$(UsingAndroidNETSdk)' != 'true' ">True</_AndroidEnablePreloadAssembliesDefault>
<_AndroidEnablePreloadAssembliesDefault>True</_AndroidEnablePreloadAssembliesDefault>
<AndroidEnablePreloadAssemblies Condition=" '$(AndroidEnablePreloadAssemblies)' == '' ">$(_AndroidEnablePreloadAssembliesDefault)</AndroidEnablePreloadAssemblies>
<_NativeAssemblySourceDir>$(IntermediateOutputPath)android\</_NativeAssemblySourceDir>
<_AndroidUseNewTypemaps>True</_AndroidUseNewTypemaps>
Expand Down
8 changes: 7 additions & 1 deletion tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,13 @@ public void ClassLibraryMainLauncherRuns ()
// Remove the default MainActivity.cs & AndroidResources
app.AndroidResources.Clear ();
app.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\layout\\foo.xml") {
TextContent = () => "<?xml version=\"1.0\" encoding=\"utf-8\" ?><LinearLayout xmlns:android=\"http://schemas.android.com/apk/res/android\" />"
TextContent = () =>
@"<?xml version=""1.0"" encoding=""utf-8""?>
<LinearLayout
xmlns:android=""http://schemas.android.com/apk/res/android""
android:layout_width=""fill_parent""
android:layout_height=""wrap_content""
/>"
});
app.Sources.Remove (app.GetItem ("MainActivity.cs"));

Expand Down

0 comments on commit 522d7fb

Please sign in to comment.