Fix splash screen with theme references crash on Android API 21

[itsarjunsinh]

Fixes flutter/flutter#73118 by providing theme reference on API 21.

What's changed?

• API 21: Splash screen drawables can access app theme references.
• New tests: Loads splash screen drawables; Loads themed (splash screen) drawables; No splash drawable resource key defined from don't crash if a splash screen resource drawable isn't added #17894.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.
• The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

[itsarjunsinh]

Friendly ping @jason-simmons, you suggested against catching the exception in engine#17894. Your thoughts about this?

[jason-simmons]

I was concerned that catching this exception could hide real errors in applications using FlutterActivity. But if some Lollipop devices are unable to resolve the resources in the splash screen, then I guess it's preferable to catch it here.

[itsarjunsinh]

But if some Lollipop devices are unable to resolve the resources in the splash screen, then I guess it's preferable to catch it here.

To clarify, NotFoundException won't occur on API 21 anymore. The reason to catch it is to prevent such crashes on pre API 21 devices where theme references in xml are just not supported.

[jason-simmons]

In the default Flutter app template the android:colorBackground reference is used in the drawable-v21 version of launch_background.xml. Pre API 21 devices should use android:color/white.

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/templates/app/android.tmpl/app/src/main/res/drawable-v21/launch_background.xml
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/templates/app/android.tmpl/app/src/main/res/drawable/launch_background.xml

Is the colorBackground resource somehow being loaded on pre API 21 devices? Or are these apps using a different version of launch_background?

[itsarjunsinh]

The main motive is to protect from avoidable crashes. Splash screens are often modified. Using unsupported theme references on older API levels is something that can occur (more so in Flutter projects). And since the crashes don't occur in newer Android versions, this can easily be missed. It happened with flutter/flutter#65182. Then you fixed crashes on API<21 devices with flutter/flutter#69255.

This can also happen in other apps. Native android projects are protected from this by a lint warning but not Flutter projects. It is worthwhile to prevent crashes at launch in this case. While testing on older API levels, seeing a blank splash screen and the detailed log warning is enough to alert the dev.

[itsarjunsinh]

@jonahwilliams, this is ready.
Edit: Formatting issues fixed. CI happy (a flaky? timeout aside)

[behzodfaiziev]

@jason-simmons I've tried to change ?android:colorBackground to @android:color/white and this has resolved an issue
I'm using Samsung Galaxy S4 on API 21

[itsarjunsinh]

@behzodfaiziev ?android:colorBackground is particularly useful on newer API levels for making the splash background dark during night mode. The Flutter embedding package wasn't utilizing theme references on API 21, hence the crash. This PR is so that no changes will be necessary in the XML of all affected projects.

As a temporary fix, you can also use a color element to still support dark backgrounds.

[dnfield]

/cc @blasten

[blasten]

First, thanks for the contribution.

As the linter is inactive in Flutter and the crashes only occur on older APIs, the bug is less likely to be detected while testing. This bug even got merged into Flutter's default template.

@itsarjunsinh This sounds like the real bug. The Android API surface is much larger than the property you are looking at, so we can't possibly catch all the exceptions to address similar concerns.

Would you be interested in exploring calling the linter for release builds?
It seem relatively trivial to call from the Flutter tool https://developer.android.com/studio/write/lint

[chinmaygarde]

@dnfield is of the opinion that this must not land but is willing to defer to @blasten on the final call. We should definitely add additional linting for such issues but I am not sure if that is work is tracked or progress is being made.

[itsarjunsinh]

@chinmaygarde I don't think adding the linter is being tracked. In my experiment, the linter makes the build times notably slower. So in the spirit of Flutter's fast iteration times it might be wise to just run it on release builds. Secondly, I had trouble running the linter on newer JDK. It will likely require flutter/flutter#62924.

That said, To address crashes on API 21 a fix is still required.

return splashScreenId != null
  // Previously > Build.VERSION_CODES.LOLLIPOP
  ? Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP
    ? getResources().getDrawable(splashScreenId, getTheme())
    : getResources().getDrawable(splashScreenId)
  : null;

We could reduce a couple lines and rely on ResourcesCompat (it's already bundled). Under the hood it makes the same call but the AndroidX team have many tests setup (including themed drawables).

return splashScreenId != null
  ? ResourcesCompat.getDrawable(getResources(), splashScreenId, getTheme())
  : null;

The test coverage for when no splash screen is defined in manifest is straightforward. However, I'm not sure the current test setup allows for loading drawable files. The AndroidX Core ResourcesCompat test uses an actual drawable file. Coincidentally, even the linter recommends calls through ResourcesCompat instead of accessing it directly. Please give me a go ahead to use it, it'll make the code more robust and streamlined.

[blasten]

Hi @itsarjunsinh. Please apply the changes, and the formatter ( ./ci/format.sh | patch -p0). LGTM otherwise

[itsarjunsinh]

@blasten, I've incorporated @dnfield and your suggestions. Can you take a look again?

• New tests will ensure that splash screen drawables aren't loaded without theme reference on API 21+.
• Whenever the linter lands, unsupported use of themed drawables in API levels < 21 will be caught at build time. Until then an error message is logged with the Resources.NotFoundException.

[chinmaygarde]

LGTM per @blasten s last comment which seems to have been addressed.

[dnfield]

LGTM