1826428 - Research removing third_party/libwebrtc/modules/utility/source/jvm_android.cc from build

Status: RESOLVED FIXED

(Core :: WebRTC, task, P2)

Description

[Michael Froman [:mjf]]

We have custom changes in our github patch stack for third_party/libwebrtc/modules/utility/source/jvm_android.cc some of which are no longer necessary, but in examining those and other changes in jvm_android.cc, I'm wondering if we need this included in the build.

I setup a build without it here and then ran tests for it here.

Based on those try results, it appears we could stop building this file altogether, but I'd like some additional confirmation from others before we try pulling it from the build.

Comment 1

[Michael Froman [:mjf]]

Randell, John, any thoughts on whether we can remove this from the libwebrtc build?

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Honestly I don't know enough about Android to know what this might impact, though if things build and pass tests without it, that's a pretty strong signal. You could ask someone from the android team, if John doesn't feel they can answer on this.

Comment 3

[John Lin [:jhlin][:jolin]]

I think it's okay to remove this. AFAIK, the only Android-specific libwebrtc module that Gecko currently uses is video capture, and it looks like we already get the needed JVM object[1][2] ourselves when initializing Gecko on Android[3][4].

[1] https://searchfox.org/mozilla-central/source/dom/media/systemservices/VideoEngine.cpp#31-32
[2] https://searchfox.org/mozilla-central/source/dom/media/systemservices/android_video_capture/video_capture_android.cc#82
[3] https://searchfox.org/mozilla-central/source/toolkit/xre/nsAndroidStartup.cpp#28
[4] https://searchfox.org/mozilla-central/source/widget/android/jni/Utils.cpp#143-144

Comment 4

[Michael Froman [:mjf]]

(In reply to John Lin [:jhlin][:jolin] from comment #3)

I think it's okay to remove this. AFAIK, the only Android-specific libwebrtc module that Gecko currently uses is video capture, and it looks like we already get the needed JVM object[1][2] ourselves when initializing Gecko on Android[3][4].

[1] https://searchfox.org/mozilla-central/source/dom/media/systemservices/VideoEngine.cpp#31-32
[2] https://searchfox.org/mozilla-central/source/dom/media/systemservices/android_video_capture/video_capture_android.cc#82
[3] https://searchfox.org/mozilla-central/source/toolkit/xre/nsAndroidStartup.cpp#28
[4] https://searchfox.org/mozilla-central/source/widget/android/jni/Utils.cpp#143-144

Thank you - I'll put together an official patch set removing it from the build and we'll land it after the next libwebrtc update (next week). If it needs to backed out, that's easy enough to do with such a small change.

Comment 5

[Michael Froman [:mjf]]

Attachment: Bug 1826428 - remove libwebrtc's jvm_android.cc from build r?ng!

Based on info from John Lin and previous try runs, we're almost
certainly not using this. Let's try removing it from the build
and landing it. If no problems emerge, we'll be able to remove
our custom changes to upstream code in jvm_android.cc.

Comment 6

[Michael Froman [:mjf]]

Attachment: Bug 1826428 - remove libwebrtc's jvm_android.cc from build - moz.build file updates r?ng!

Depends on D174793

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dca1b9752548
remove libwebrtc's jvm_android.cc from build r=ng,webrtc-reviewers
https://hg.mozilla.org/integration/autoland/rev/c572834c6dcf
remove libwebrtc's jvm_android.cc from build - moz.build file updates r=ng,webrtc-reviewers

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/dca1b9752548
https://hg.mozilla.org/mozilla-central/rev/c572834c6dcf