Remove static leaks #8825
Remove static leaks #8825
Conversation
|
This is ready for review. Once this one is good, we should be able to close flutter/flutter#28848 |
|
|
||
| /// Returns true if successfully unpacked APK resources, | ||
| /// otherwise deletes all resources and returns false. | ||
| private boolean extractAPK(File dataDir) { |
There was a problem hiding this comment.
Is this a long-running method? If so, should it be executed on the @UiThread or @WorkerThread?
There was a problem hiding this comment.
@sbaranov would know. I'm going to imagine it's long running.
There was a problem hiding this comment.
Actually, this is only called from doInBackground, which I believe should mean it's fine to annotate this with @WorkerThread
There was a problem hiding this comment.
Yeah that sounds good.
matthew-carroll
left a comment
matthew-carroll
left a comment
There was a problem hiding this comment.
LGTM pending a couple updates that would be nice to make while you're here.
|
|
||
| /// Returns true if successfully unpacked APK resources, | ||
| /// otherwise deletes all resources and returns false. | ||
| private boolean extractAPK(@NonNull File dataDir) { |
There was a problem hiding this comment.
Can this just be made static and kept in the outer class, instead of moving it here?
There was a problem hiding this comment.
It could, you just then have to pass more references around to the method here (mResources, mAssetManager, mDataDirPath)
There was a problem hiding this comment.
Nit - It might be slightly better, to keep it symmetrical with deleteFiles and checkTimestamp.
flutter/engine@33cf682...88e82d3 git log 33cf682..88e82d3 --no-merges --oneline 88e82d3 Remove static leaks (flutter/engine#8825) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (amirha@google.com), and stop the roller if necessary.
5 participants
Fixes flutter/flutter#28848
Depends on #8821
Should not be landed until after #8824
This PR gets rid of any lint warnings about static leaks. It involves reworking a bit of ResourceExtractor.java to avoid passing the
Contextobject into a static field.