Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAndroid life cycle improvements and Gradle integration #15773
Conversation
highfive
commented
Feb 28, 2017
|
Heads up! This PR modifies the following files:
|
afdf316
to
e635469
|
Awesome work, @MortimerGoro! @fabricedesre Would you mind doing the initial review of this? :-) |
e635469
to
528c553
|
@larsbergstrom sure, will do. |
f42ddd4
to
b0e0497
|
Thanks for doing that, this is super nice! |
|
|
||
| defaultConfig { | ||
| applicationId "com.mozilla.servo" | ||
| minSdkVersion 18 |
This comment has been minimized.
This comment has been minimized.
fabricedesre
Mar 2, 2017
Contributor
There is little value supporting such an old version (This is Jelly Bean!). Let's move to 21 instead (Lollipop).
This comment has been minimized.
This comment has been minimized.
MortimerGoro
Mar 2, 2017
Author
Contributor
I agree that setting target 21 would simplify parts of the code (is the minimum api level for native arm64 builds too). But Jelly bean + KitKat still has around 35-40% market share (https://developer.android.com/about/dashboards/index.html). What do you think about this @larsbergstrom ?
This comment has been minimized.
This comment has been minimized.
mmatyas
Mar 9, 2017
Contributor
Older devices are still widely used (think developing countries and regional differences), and several of our test devices also run Android 4.4. I'd like API 18 support too.
| // For each dependency call the proper compile command and set the correct dependency path | ||
| def list = ['arm', 'armv7', 'arm64', 'x86'] | ||
| for (arch in list) { | ||
| for (int i = 0; i <= 1; ++i) { |
This comment has been minimized.
This comment has been minimized.
| package="com.mozilla.servo" | ||
| android:versionCode="1" | ||
| android:versionName="1.0"> | ||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" android:installLocation="preferExternal" |
This comment has been minimized.
This comment has been minimized.
fabricedesre
Mar 2, 2017
Contributor
Use auto as a value for installLocation, and move it to its own line.
| } | ||
|
|
||
| // keep the device's screen turned on and bright. | ||
| private void keepScreenOn() { |
This comment has been minimized.
This comment has been minimized.
|
|
||
| // Dim toolbar and make the view fullscreen | ||
| private void setFullScreen() | ||
| { |
This comment has been minimized.
This comment has been minimized.
fabricedesre
Mar 2, 2017
Contributor
Same here.
Also the style in this file is not consistent. Either always put the opening brace on its own line or on the same line as the method declaration, but don't mix and match.
| try { | ||
| is = getAssets().open(file); | ||
| isr = new InputStreamReader(is); | ||
| reader = new BufferedReader(isr); |
This comment has been minimized.
This comment has been minimized.
fabricedesre
Mar 2, 2017
Contributor
nit: reader = new BufferedReadre(new InputStreamReader(is)); so you won't have to deal manually with isr.
| } | ||
| finally { | ||
| try { | ||
| if (is != null) is.close(); |
This comment has been minimized.
This comment has been minimized.
| return new JSONObject(json); | ||
| } catch (JSONException e) { | ||
| Log.e(LOGTAG, Log.getStackTraceString(e)); | ||
| return null; |
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,2 @@ | |||
| buildDir=../../../../target/gradle | |||
| #android.useDeprecatedNdk=true | |||
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,6 @@ | |||
| #Mon Dec 28 10:00:20 PST 2015 | |||
This comment has been minimized.
This comment has been minimized.
b0e0497
to
356f545
|
Thanks for the review @fabricedesre I pushed all the requested changed. I'm waiting for servo/glutin#117 to be merged to update the glutin version. |
|
hm, I'm not sure why the ccache travis build is failing. |
|
It seems like a random crash. Anyway we need to merge servo/glutin#117 or travis build will fail too in another step. |
|
Not sure it's random, since I re triggered the build and it failed again. But yep, let's wait for the glutin changes first! |
356f545
to
63a600f
|
@larsbergstrom Thanks for merging glutin PR servo/glutin#117 But no version has been uploaded to crates.io (I thought that it was automatic). Can you publish the new version? I can switch to a github based dependency if required. @fabricedesre I have updated the PR with some java/rust changes related to android resources/data paths. Upgrading the target SDK breaks permissions for writing in /sdcard/. I have switched to the recommended getExternalFilesDir paths. Changed files: https://github.com/servo/servo/pull/15773/files#diff-e65ecda904892250719ab1e90cf0205f https://github.com/servo/servo/pull/15773/files#diff-f067d00c401c272948bfa3dde94d5ead and the path where copying resources in the MainActivity |
63a600f
to
f08a5c6
|
@MortimerGoro thanks for the update! Can you not squash when doing that though, it's easier to look at interdiffs when reviewing incremental changes. |
|
@mbrubeck Thanks! @fabricedesre Ok, the correct way is to squash the commits after the PR is approved. Sorry for the inconveniences. Let's do that way ;) |
a2ac654
to
6f56f88
|
Finally had some time to try this out, really nice work! |
|
Thanks @mmatyas
I've upgraded the PR to support API 18. The android version compatibility code is much simpler now as requested by @fabricedesre. Only a full screen View flag was breaking compatibility with API 18, easy to handle. |
5a4a12c
to
7a2a909
|
@aneeshusa ok, changed to use full path. |
|
servo/saltfs#639 is deployed. @bors-servo r=larsbergstrom,fabricedesre |
|
|
…trom,fabricedesre Android life cycle improvements and Gradle integration This PR includes Android life cycle Improvements and Gradle integration for android packaging. Android life cycle improvements are implemented in both the new [MainActivity](https://github.com/servo/servo/compare/master...MortimerGoro:android_improvements?expand=1#diff-f43708b102e98272c2af7454b8846927) and native code in this related PR: servo/glutin#117 - Properly handle the life cycle of the Android Activity: manage EGLContext lost & restore, and animation loop pause/resume when the app goes to background/foreground or orientation changes. In the current upstream Servo crashes when the app goes to background, activity stops or changes orientation - Handle event loop awake for Servo Animation loop - Handle screen resize & orientation events - Implement new keep_screen_on preference which keeps Android device's screen from ever going to sleep: very useful for games or WebVR - Implement a full screen mode enabled by preference: very useful for games or WebVR - Implement new shell.native-orientation preference which allows to lock the Activity to a specific orientation - Automatically sync new android assets to sdcard when the Android version code is changed. In the current upstream only the existence of the folder is check but resources are not updated ofscreen_gl_context is updated to fix this: servo/surfman#83 This PR integrates Gradle build system for android packaging. Most of the code is implemented in this [build.gradle](https://github.com/servo/servo/compare/master...MortimerGoro:android_improvements?expand=1#diff-89cdb9324addb994cdba0a158b209547) . We can get rid of [build-apk](https://github.com/servo/servo/compare/master...MortimerGoro:android_improvements?expand=1#diff-40f5a7cf22f94aad059b2c1795347f5e) and manual jar dependency copying in the [package_commands.py](https://github.com/servo/servo/compare/master...MortimerGoro:android_improvements?expand=1#diff-0d425b142c8d10ae6ac1f3711fb5c23a). The correct version of gradle is automatically downloaded using the gradlew wrapper. Some improvements: - Allows to include more complex android dependencies/SDKs like AARs, manifest auto-merging and more. - Improved packaging process: The gradle project is always in the same folder, it uses relative paths for everything (assets, native libraries) and outputs the apk into the correct target folder in servo. In the current upstream, ant/python build system copies the manifest, project, resources and jars each time so you end with multiple copies of the same files. - Improved dependency declaration. We do not have to manually copy jar dependencies in the python script anymore. The gradle build scripts itself is able to search for the dependencies in the correct servo target folder. - Supports packaging apks with different architectures: armeabi, armeabi-v7a, aarch64. We still need to fix some native servo compilation issues with armeabi-v7a, aarch64 due to dependencies which use `make`. I'll push this changes in a separate PR of the python build files but the gradle file is already ready to handle that. - We can easily create product flavors for different versions of Servo. For example a default browser, a WebVR browser with additional dependencies or a Servo android Webview component - Fixes minor.major.52 build error when blurdroid cargo dependency is compiled using java8 (the default in new Linux machines). The gradle build file enables the new Jack compiler which supports Java8 dependencies. - The project can be opened with Android Studio and run the brand new GPU debugger on any Android phone. I'll add some docs in the Wiki about this. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14568 (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15773) <!-- Reviewable:end -->
|
|
This is needed for the gradle based build system (see servo/servo#15773 (comment))
…sbergstrom Make android sdk 25.2.3 default Follow up to servo/servo#15773. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/644) <!-- Reviewable:end -->
This is needed for the gradle based build system (see servo/servo#15773 (comment))
…ergstrom Remove redundant ANDROID_SDK variables The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to #15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16566) <!-- Reviewable:end -->
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 0b6b6b2cb7e84cd3e6f492df9489a98464fa9d79
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628 UltraBlame original commit: ae809917d5db3293869402f858ce1cf5e4a72968
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628 UltraBlame original commit: ae809917d5db3293869402f858ce1cf5e4a72968
…eeshusa:remove-android-sdk-override); r=larsbergstrom The r25.2.3 Android SDK has been made the current version in saltfs, so we no longer need to override it via environment variable. Follow up to servo/servo#15773. Requires servo/saltfs#644. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 779edd7c4aaf900f12fab378a378b0fc52d4c628 UltraBlame original commit: ae809917d5db3293869402f858ce1cf5e4a72968
MortimerGoro commentedFeb 28, 2017
•
edited
This PR includes Android life cycle Improvements and Gradle integration for android packaging.
Android life cycle improvements are implemented in both the new MainActivity and native code in this related PR: servo/glutin#117
ofscreen_gl_context is updated to fix this: servo/surfman#83
This PR integrates Gradle build system for android packaging. Most of the code is implemented in this build.gradle . We can get rid of build-apk and manual jar dependency copying in the package_commands.py. The correct version of gradle is automatically downloaded using the gradlew wrapper.
Some improvements:
make. I'll push this changes in a separate PR of the python build files but the gradle file is already ready to handle that../mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is