839622 - Various JNI-related functions are not used and can be removed

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1/3 - Clean up temp files

See patches

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2/3 - Remove unused functions/implementations

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3/3 - Don't include websms JNI goop if we're not building websms

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3/3 - Don't include websms JNI goop if we're not building websms

Whoops, wrong patch file.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 711948 [details] [diff] [review]
Part 3/3 - Don't include websms JNI goop if we're not building websms

Review of attachment 711948 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/android/jni-stubs.inc
@@ -455,5 @@
> -  xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_notifyReadingMessageListFailed", &f_Java_org_mozilla_gecko_GeckoAppShell_notifyReadingMessageListFailed);
> -#endif
> -
> -#ifdef JNI_STUBS
> -

rather than deleting all of this, can you just put it in an ifdef MOZ_WEBSMS_BACKEND

Comment 5

[Kartikaya Gupta (email:[e-mail])]

(In reply to Brad Lassey [:blassey] from comment #4)
> 
> rather than deleting all of this, can you just put it in an ifdef
> MOZ_WEBSMS_BACKEND

Not without significantly changing how that file is autogenerated. (One of the changes would require building GeckoSmsManager.java even though it is not used).

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #5)
> (In reply to Brad Lassey [:blassey] from comment #4)
> > 
> > rather than deleting all of this, can you just put it in an ifdef
> > MOZ_WEBSMS_BACKEND
> 
> Not without significantly changing how that file is autogenerated. (One of
> the changes would require building GeckoSmsManager.java even though it is
> not used).

Right, this is the autogenerted file. And this problem will go away when we make the autogeneration part of the build process, right?

Comment 7

[Chris Peterson [:cpeterson]]

Comment on attachment 711944 [details] [diff] [review]
Part 1/3 - Clean up temp files

Review of attachment 711944 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 8

[Chris Peterson [:cpeterson]]

Comment on attachment 711945 [details] [diff] [review]
Part 2/3 - Remove unused functions/implementations

Review of attachment 711945 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Comment 9

[Kartikaya Gupta (email:[e-mail])]

(In reply to Brad Lassey [:blassey] from comment #6)
> Right, this is the autogenerted file. And this problem will go away when we
> make the autogeneration part of the build process, right?

Right.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e89a686b546e
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a0f174d19f5
https://hg.mozilla.org/integration/mozilla-inbound/rev/15c4a08c03d7

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e89a686b546e
https://hg.mozilla.org/mozilla-central/rev/2a0f174d19f5
https://hg.mozilla.org/mozilla-central/rev/15c4a08c03d7