Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@ColdPaleLight
Copy link
Member

@ColdPaleLight ColdPaleLight commented Jan 15, 2022

Some OnePlus phones can still update the view tree after locking the screen in some cases, which causes the surfaceCreated method of SurfaceView.Holder can be called when the screen is turned off, and surfaceCreated will not be called when the screen is turned on. And changing the visibility of FlutterSurfaceView in onStart and onStop can fix this problem.

detail
flutter/flutter#93276 (comment)

PS: Setting the visibility of FlutterView directly at onStart and onStop can also solve this problem. However based on my current investigations TextureView shouldn't have this problem (it doesn't call onSurfaceTextureDestroyed when the screen is off, so no need to call onSurfaceTextureAvailable when the screen is on), Please let me know if you guys think it makes more sense to change the visibility of FlutterView.

Fix flutter/flutter#93276, flutter/flutter#94921.

Pre-launch Checklist

  • 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.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@ColdPaleLight ColdPaleLight changed the title Remove/Add FlutterSurfaceView at onStop/onStart change visibility of FlutterSurfaceView when onStop/onStart Jan 16, 2022
@ColdPaleLight ColdPaleLight changed the title change visibility of FlutterSurfaceView when onStop/onStart Change visibility of FlutterSurfaceView when onStop/onStart Jan 16, 2022
Comment on lines 1425 to 1436
if (flutterSurfaceView != null) {
flutterSurfaceView.setVisibility(View.VISIBLE);
}
}

// Invoke this from {@code FlutterActivityAndFragmentDelegate#onStop()}
/* package */ void onStop() {
if (flutterSurfaceView != null) {
flutterSurfaceView.setVisibility(View.GONE);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've tried to resist adding API like this historically because it's likely to get missed when you don't use the FlutterActivityAndFragmentDelegate class, IIRC. @xster or @blasten wdyt?

I'm curious about where the view is getting the wrong visibility set and why this helps. I'm also curious if we're getting calls on View.invalidateDrawable at some point here that might help us - if we can take advantage of the View's own lifecycle it'd be better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively if we're getting calls to onDraw or onWindowVisibilityChanged/onAttachedToWindow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess these methods might not work. In this scenario, the onStart and onStop of the Activity are correct, but the visibility of the window/view tree is still visible after onStop.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to better understand if we can avoid the View hooking Activity/Fragment lifecycle.

Comment on lines 1425 to 1436
if (flutterSurfaceView != null) {
flutterSurfaceView.setVisibility(View.VISIBLE);
}
}

// Invoke this from {@code FlutterActivityAndFragmentDelegate#onStop()}
/* package */ void onStop() {
if (flutterSurfaceView != null) {
flutterSurfaceView.setVisibility(View.GONE);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively if we're getting calls to onDraw or onWindowVisibilityChanged/onAttachedToWindow.

@ColdPaleLight ColdPaleLight force-pushed the fix_one_plus_black_screen branch from 802ef43 to 644f990 Compare January 19, 2022 03:39
@ColdPaleLight
Copy link
Member Author

@dnfield

I changed the code to set the visibility of FlutterView in onStart and onStop of FlutterActivityAndFragmentDelegate.
Since the methods onStart and onStop of FlutterActivityAndFragmentDelegate are not public, it is only called in the onStart and onStop of FlutterActivity/FlutterFragment. So this change should be safe.

@ColdPaleLight ColdPaleLight requested a review from dnfield January 19, 2022 03:52
@ColdPaleLight ColdPaleLight changed the title Change visibility of FlutterSurfaceView when onStop/onStart Change visibility of FlutterView when onStop/onStart Jan 19, 2022
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine to at least work around the bug for now, but it would be better if we could find some way to avoid needing this.

Log.v(TAG, "onStart()");
ensureAlive();
doInitialFlutterViewRun();
flutterView.setVisibility(View.VISIBLE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please add some comment(s) here and below about why we're doing this, with a link to the bug

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@chinmaygarde chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jan 20, 2022
@fluttergithubbot fluttergithubbot merged commit ed3704c into flutter:main Jan 20, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 21, 2022
@zhangruiyu
Copy link

when will it be merged into stable

@dnfield
Copy link
Contributor

dnfield commented Jan 21, 2022

There is a work around posted on the bug that you can patch into your own app. I don't think we should go through the process of cherry picking for this.

// screen when unlocked. We can work around this by changing the visibility of FlutterView in
// onStart and onStop.
// See https://github.com/flutter/flutter/issues/93276
flutterView.setVisibility(View.VISIBLE);
Copy link

@zxjzerg zxjzerg Jun 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meet an issue because of this edit. My use case is I use FlutterFragment as a tab in Activity, when user switched to other tabs, I will hide the FlutterFragment. After this commit, every time Activity resumed, the flutterView will be foreced to visible, even the FlutterFragment is set to hide.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please file a new issue with minimum reproducible code, thank you.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please file a new issue with minimum reproducible code, thank you.

this merge doest fix the issue;

this is my code to reproduce,

@amanmalhotra-cred
Copy link

we have started facing an issue because of this change in our add to app approach when we recently migrated from flutter 2.10.3 -> 3.13.4.

we have a tab holder fragment in native where centre tab is native and adjacent two tabs are flutter.
while switching tabs we have fragmant transaction animation that happens
before this change, we were able to see the flutter view during that transition of fragment from one to another basically one goes down second comes up, post this change, when first fragment starts to go down, flutter view becomes invisible and the native fragment appears giving a flicker.

@codewithmashi
Copy link

Still Showing Black Screen on Samsung s21fe android 14. After Updating the app. Fixed the issue by clearing the data of app.

[√] Flutter (Channel stable, 3.24.4, on Microsoft Windows [Version 10.0.26100.2605], locale en-US)
[√] Windows Version (Installed version of Windows is version 10 or higher)
[√] Android toolchain - develop for Android devices (Android SDK version 35.0.0)

Got this Black Screen Error After Updating the app. Unless you do clear data.
So Anyone know how to fix it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Android] App shows black screen after being in the background.

9 participants