Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preliminary Android build support #31086

Merged
merged 42 commits into from Jan 22, 2024
Merged

Preliminary Android build support #31086

merged 42 commits into from Jan 22, 2024

Conversation

mukilan
Copy link
Member

@mukilan mukilan commented Jan 15, 2024

The built APK still doesn't run on device and it crashes on loading.
This is happening only after the rust compiler toolchain update. I
am still investigating this, but it is proving to be challenging as
the generated stack trace doesn't show function names even
in debug build and after ensuring Gradle doesn't strip the symbols.

The goal with this PR is to land the changes with just the CI build
passing.

Other issues that need to be resolved in future PRs

  1. Enable GStreamer support - this is disabled for now
  2. Enable bluetooth support - blurdroid is unmaintained and doesn't
    compile now.
  3. Revisit issues involving dependencies - jemalloc-sys, freetype and
    fontconfig
  4. Revisit compositing logic & event handling. Apps can lose underlying
    GL Surface when they are sent to background. I have added logic to
    pause/resume the compositor when this happens so that we bind/unbind.
    Scrolling is also now 'janky' after the move from WebRender based zoom
    to Servo handling the zoom with root pipeline.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because these changes are for preliminary support. CI sanity check of android is disabled for now as apk crashes and sanity test code is also incorrect.

mrobinson and others added 29 commits January 12, 2024 18:48
* Fixes
* More fixes
  - Still failing in the linking step
* More work on getting linking working

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
NDK requires we compile against the minSdk API level
which is 30 in our case.

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
This commit drops:
* support for google, oculusvr
* support for arm5 architecture

and also removes
* fakeld scripts
* unused java code

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Great work! I'm so glad to see this happening. I have a bunch of comments below, but this is already looking very nice.

ports/libsimpleservo/jniapi/build.rs Outdated Show resolved Hide resolved
ports/libsimpleservo/jniapi/src/lib.rs Outdated Show resolved Hide resolved
ports/libsimpleservo/jniapi/src/lib.rs Outdated Show resolved Hide resolved
python/servo/build_commands.py Outdated Show resolved Hide resolved
python/servo/command_base.py Outdated Show resolved Hide resolved
components/compositing/compositor.rs Outdated Show resolved Hide resolved
ports/libsimpleservo/jniapi/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/jniapi/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/jniapi/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/jniapi/Cargo.toml Outdated Show resolved Hide resolved
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
libservo and android_logger can use env_logger 0.10
but quickcheck is still stuck on 0.8 and has not seen
any activity in the last 2 years. This commit adds
a duplicate exception until the quickcheck dependency
can be upgraded (or replaced)

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Nice! So happy to see this finally landing. Just a couple small things before landing and also let's go ahead and disable jemalloc for Android.

.github/workflows/android.yml Outdated Show resolved Hide resolved
components/compositing/compositor.rs Outdated Show resolved Hide resolved
components/compositing/compositor.rs Outdated Show resolved Hide resolved
components/compositing/windowing.rs Outdated Show resolved Hide resolved
components/compositing/windowing.rs Outdated Show resolved Hide resolved
@@ -1091,6 +1098,9 @@ fn default_user_agent_string_for(agent: UserAgent) -> &'static str {
const DESKTOP_UA_STRING: &'static str =
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Servo/1.0 Firefox/111.0";

#[cfg(target_os = "android")]
Copy link
Member

Choose a reason for hiding this comment

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

It might be a good moment to clean this up and just conditionally define a UA_STRING and always return it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, the existing code is written so that the user can switch user agent to ios/android specific ones even on desktop at runtime using this flag, probably to allow testing.

I think implementing what you suggested would require removing this functionality from libservo (maybe we can do it to ports/servoshell). Can this be tackled in a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sure!

ports/libsimpleservo/jniapi/build.rs Outdated Show resolved Hide resolved
python/servo/command_base.py Outdated Show resolved Hide resolved
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
@mukilan
Copy link
Member Author

mukilan commented Jan 22, 2024

Thanks for the review!

@mukilan mukilan added this pull request to the merge queue Jan 22, 2024
Merged via the queue into servo:main with commit d7de206 Jan 22, 2024
11 checks passed
@mukilan mukilan deleted the android-ng branch January 22, 2024 14:01
sagudev pushed a commit to sagudev/servo that referenced this pull request Jan 24, 2024
* Android build

* Fixes
* More fixes
  - Still failing in the linking step
* More work on getting linking working

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* android: use mozjs with ndk r25c. loads servo.org

more android build fixes.

* fix ./mach run for android and make it follow logs

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* add experimental logic for compositor pause/resume

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* pass DPI from android to simpleservo

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* ci: add android workflow

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* switch to ANDROID_SDK_ROOT and ANDROID_NDK_ROOT vars

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* upgrade gradle to 4.10.1

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* upgrade to gradle 5.1.1

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* upgrade to gradle 8 and agp 8

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* make compositing work again with external present

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* android: improve mach support for non-NixOS and CI

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* fix sampler compilation bug introduced in servo#30490

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* ci: add android build to main workflow

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* gradle: set MinSdk = targetSdk = 30

NDK requires we compile against the minSdk API level
which is 30 in our case.

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* add instructions for android in README.md

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* apk: move servosurface to servoview

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* apk: uncomment the mediasession callbacks on MainActivity

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* apk: fix crash on MainAtivity.onDestroy

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* apk: drop VR, arm 5 and unused code

This commit drops:
* support for google, oculusvr
* support for arm5 architecture

and also removes
* fakeld scripts
* unused java code

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* cleanup shell.nix

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* android: add FIXMEs for gstreamer code

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* apk: remove commented code and debug logs

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* cleanup ServoView.java

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* mach: comment call to download gstreamer deps for android

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* disable bluetooth for jniapi as blurdroid is broken

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* fixup! README.md

* fixup! remove change in Cargo.toml

* fixup! move shell variables together

* fixup! cleanup jniapi/Cargo.toml comments

* delete commented gstreamer related android code

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* remove unused config variable in servbuild

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* android: more cleanup

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* force no_static_freetype only for android

* use actions to manage sdk, ndk and java

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* rename embedder event names to be more clear.

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* link to startup crash issue

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* fix lint issues

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* upgrade env_logger to 0.10 with duplicate exception

libservo and android_logger can use env_logger 0.10
but quickcheck is still stuck on 0.8 and has not seen
any activity in the last 2 years. This commit adds
a duplicate exception until the quickcheck dependency
can be upgraded (or replaced)

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* android: fix comments

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* disable jemalloc on android

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

* fixup! replace linux with android in cfg

---------

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants