[FIX][Android]: The ART '<Surface>' becomes invisible in Android

[cabelitos]

Hello Everyone, this series of commits helps to fix problems related to ART on Android. The main problem in here is that the ART components would disappear if the user turns off the screen and then turn it on again. It's important to note that this behaviour only occurs after Android N (7.1 or higher).

Test Plan:

• Open the Android emulator using API level 25 or higher (Android 7.1 or higher)
• Create an react-native-app with the following code:

import React, { Component } from 'react';
import { Platform, StyleSheet, Text, View, ART } from "react-native";


const path = ART.Path();

path.moveTo(0, 0);
path.lineTo(10, 10);

type Props = {};
export default class App extends Component<Props> {
  render() {
    return (
      <View style={styles.container}>
        <ART.Surface width={300} height={300}>
          <ART.Shape d="M150 0 L75 200 L225 200 Z" fill="#ff0000"/>
        </ART.Surface>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#F5FCFF',
  },
});

• After the App is running, turn off the screen
• Wait some seconds and turn it on again
• Expected result: The triangle should be visible
• Unexpected result ( without this patch series): The triangle will not be visible

Changelog:

[Android] [BUGFIX] [ARTSurfaceViewShadowNode] - Properly release mSurface
[Android] [BUGFIX] [ARTSurfaceViewShadowNode] - Fix Surface becoming invisible in Android
[Android] [BUGFIX] [ARTSurfaceViewShadowNode] - Fix surface not being drawed on app startup

Fixes #17565

[pull-bot]

	Warnings
⚠️	📋 Changelog - This PR may have incorrectly formatted Changelog.

Generated by 🚫 dangerJS

[cabelitos]

I'm not sure why test_objc/test_detox_end_to_end are failing for two reasons:

• I did not touch on iOS code
• I did no changes on the switch.js (also detox uses iOS, which I did not touch as well)
  😔
  By the way, how can I trigger the change log bot to parse the change log again?

[Winglonelion]

@cabelitos i tried your codes but it not work.

More info: qemu (Nexus 5x android 7.0) in android studio.

[cabelitos]

Hello @Winglonelion, it's kinda difficult to know what's happening by only looking at your screenshot.
Is your app open source? can I take a look into it so I can debug it? or even better, can you isolate the problem? Did you try the test case that I provided in the PR summary?

[fnuttens]

Hello @cabelitos, I'm also trying to test your fix as I encountered the ART issue in one of my company's projects. I created a new project using your test plan, and then I directly applied your patch on React native's code (from the node_modules directory). It didn't change anything but maybe I need to build a fork of React native using your code?

[cabelitos]

@fnuttens for android there's an specific build steps. Only applaying the patch will not solve the problem. You can follow this tutorial: https://facebook.github.io/react-native/docs/building-from-source
Keep in mind that you need to apply my patch before compiling

[Winglonelion]

@cabelitos my implement steps same as @fnuttens. But i clean and re-compile project after changes codes.

I think if it is a change in native codes, it would be affected when compile ?

this is completed screen record after i affect your changes and rebuild:

https://youtu.be/QBZG6VFOi0g

[cabelitos]

Hey @Winglonelion, thanks for the video. What's the android version that you're using it? From the emulator window at least it says you're running under API 24. In my code I only enable the new ART behavior if you're at API 25 and beyond. You could try changing the code to:

    // ReactAndroid/src/main/java/com/facebook/react/views/art/ARTSurfaceViewShadowNode.java
  @Override
  public void setThemedContext(ThemedReactContext themedContext) {
    super.setThemedContext(themedContext);
     // CHANGE HERE TO '>=' JUST LIKE THE CODE BELOW
    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
      themedContext.addLifecycleEventListener(this);
    }
  }

Or you can complete remove the if and keep themedContext.addLifecycleEventListener(this);, like this:

    // ReactAndroid/src/main/java/com/facebook/react/views/art/ARTSurfaceViewShadowNode.java
  @Override
  public void setThemedContext(ThemedReactContext themedContext) {
    super.setThemedContext(themedContext);
    themedContext.addLifecycleEventListener(this);
  }

[Winglonelion]

@cabelitos Yes, as i mentioned, my simulator run as API 24.

Sorry because i didn't review your code more particullary.

What is the reason for specific behavior in 25 and larger API ?

I will test it in both API 24, 25 and reply soon

[cabelitos]

@Winglonelion, ok I'll wait

[andrewmarmion]

Any update on this?

[cabelitos]

@andrewmarmion I really wanted that the facebook folks could review this. :/

[cpojer]

I tried this out on a real Galaxy S8 on Android 9 and cannot reproduce the issue you are describing in your original PR. Is there anything specific I am missing?

[danilobuerger]

@cpojer I was able to reproduce the issue on my Pixel 3 with Android 9, RN 0.59. Don't know why it does not work for you.

[cpojer]

Could you try to repro this on master using RNTester and sharing your code example (fork of RN?) with me with instructions on how to repro? As said, it didn’t repro for me and I’m inclined to close this PR unless somebody can prove this change is necessary.

[cabelitos]

Sorry for the delay guys. Tomorrow I'll with my s8

[danilobuerger]

@cabelitos Did you get around to testing it?

[cabelitos]

Hey guys, I was not able to test on the s8, however the problem still happens on an Android Pie emulator. Here's a video: https://youtu.be/SIFRksrWX4Y

I'm using react-native latest version (0.59)

[cpojer]

Alright, let's give it a try.

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @cabelitos in 20b09e4.

^{When will my fix make it into a release? | Upcoming Releases}

[cabelitos]

thank you guys!