Introduce flutterfragmentactivity #12305
Conversation
matthew-carroll
force-pushed
the
introduce_flutterfragmentactivity
branch
from
130c2bf
to
ff89bbb
Compare
| * platform channels to instruct Android to do so at the appropriate time. This will avoid any | ||
| * jarring visual changes during app startup. | ||
| */ | ||
| private void switchLaunchThemeForNormalTheme() { |
There was a problem hiding this comment.
Are some of these below copy pasted from the FlutterActivity file? Could some of it be extracted into a shared class?
| */ | ||
| private boolean isDebuggable() { | ||
| return (getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0; | ||
| } |
There was a problem hiding this comment.
It seems like most of the code is duplicated. Could we extract somehow?
There was a problem hiding this comment.
It is possible to extract, but I'm not sure we should. As with the delegate change that we made, I'm hesitant to tear apart Activity implementations into other objects that do all the work on behalf of the Activity. When that happens, reading the Activity becomes an exercise in constantly jumping to other files and back. Do you think it's a meaningful duplication risk to leave this as-is?
There was a problem hiding this comment.
Discussed offline. You decide. I don't feel strongly, as long as we leave hints for other maintainers.
There was a problem hiding this comment.
We spoke offline. I'm suggesting that we leave duplicated for now for readability. If we find that this creates maintenance problems in the future then we can re-evaluate. I'll post a PR immediately after this one that adds javadoc comments mentioning that these files need to change together.
|
You described running into friction when writing tests. What was the specific blocker that prevented progress? LGTM otherwise. |
@xster we were seeing an unclear compile time error that implied a version mismatch between the support library versions the engine uses (28) and the Android SDK that we compile it with (29). Speculatively I think we may need to fix the mismatch in the engine and migrate our Android support libraries to 29 to match our SDK version to resolve the error, but there is no SDK 29 |
|
@mklim understood. I assume the root cause is because of flutter/buildroot#291? If the 29 'usage' is actually real and justifiable, then this should push our timetables for flutter/flutter#39283 forward. |
git@github.com:flutter/engine.git/compare/d1692d4cc703...6a96417 git log d1692d4..6a96417 --no-merges --oneline 2019-09-17 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from RRgw-... to F-g18... (flutter/engine#12326) 2019-09-17 chinmaygarde@google.com Account for root surface transformation on the surfaces managed by the external view embedder. (flutter/engine#11384) 2019-09-17 matthew-carroll@users.noreply.github.com Introduce FlutterFragmentActivity (flutter/engine#12305) 2019-09-17 chinmaygarde@google.com Shuffle test order and repeat test runs once. (flutter/engine#12275) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
git@github.com:flutter/engine.git/compare/d1692d4cc703...6a96417 git log d1692d4..6a96417 --no-merges --oneline 2019-09-17 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from RRgw-... to F-g18... (flutter/engine#12326) 2019-09-17 chinmaygarde@google.com Account for root surface transformation on the surfaces managed by the external view embedder. (flutter/engine#11384) 2019-09-17 matthew-carroll@users.noreply.github.com Introduce FlutterFragmentActivity (flutter/engine#12305) 2019-09-17 chinmaygarde@google.com Shuffle test order and repeat test runs once. (flutter/engine#12275) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
This PR introduces a
FlutterFragmentActivity, which is likeFlutterActivityexcept that it extendsFragmentActivity. This is required to satisfy at least one plugin that requires aFragmentActivity.We tried to test this class. There were compilation errors in the test infrastructure that @mklim and I could not effectively debug. I filed an issue here to write tests when infrastructure allows for it: flutter/flutter#40629