Closed Bug 1168407 Opened 5 years ago Closed 5 years ago

Make JavaAddonManager interface bi-directional

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
p11 + ---
firefox41 --- fixed
relnote-firefox --- 41+

People

(Reporter: nalexander, Assigned: nalexander)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(3 files, 5 obsolete files)

39 bytes, text/x-review-board-request
gbrown
: review+
Details
40 bytes, text/x-review-board-request
nalexander
: review+
mfinkle
: review+
Details
40 bytes, text/x-review-board-request
nalexander
: review+
Details
For a long time, Fennec has supported loading arbitrary Java code (in the form of DEX files): Bug 794479.  There are additional tickets (such as Bug 799631) to flesh out an interface for Java Addons.  This ticket tracks implementing one such interface: making the JavaAddonManager interface bi-directional.

That means that a Java Addon will be able to both receive and respond to messages from Gecko, but also send a message to Gecko (and get a response).

One possible use case: an addon wishes to integrate with an existing APK on device (such as the barcode scanning software ZXing).  The Gecko -> Java interface is used to respond to user requests to scan a barcode.  The Java -> Gecko interface might be used to notify Fennec when ZXing settings are modified.
This will need developer docs on MDN, anchored at https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API.  If time allows, documenting Messaging.jsm would be extremely valuable as well.
Keywords: dev-doc-needed
Bug 1168407 - Implement a bidirectional Java addon interface. r=kats,rnewman

There are several parts to this ticket:

1) Produce javaaddons-1.0.jar, a standalone JAR defining a (versioned)
Java interface suitable for consumption by third-party Java addon
implementations.

2) Support the new V1 interface in the JavaAddonManager.

3) Add JUnit 3 tests of the class-finding functionality.  This
includes building a test DEX file that implements the V1 interface
(consuming the built JAR).  This is both a functional test and living
documentation of how to produce such an addon.

4) Add Robocop JavascriptTests testing the JavaScript message passing
interface to and from Java.
Attachment #8614305 - Flags: review?(rnewman)
(In reply to Nick Alexander :nalexander from comment #2)
> Created attachment 8614305 [details]
> MozReview Request: Bug 1168407 - Implement a bidirectional Java addon
> interface. r=kats,rnewman
> 
> Bug 1168407 - Implement a bidirectional Java addon interface. r=kats,rnewman
> 
> There are several parts to this ticket:
> 
> 1) Produce javaaddons-1.0.jar, a standalone JAR defining a (versioned)
> Java interface suitable for consumption by third-party Java addon
> implementations.
> 
> 2) Support the new V1 interface in the JavaAddonManager.
> 
> 3) Add JUnit 3 tests of the class-finding functionality.  This
> includes building a test DEX file that implements the V1 interface
> (consuming the built JAR).  This is both a functional test and living
> documentation of how to produce such an addon.
> 
> 4) Add Robocop JavascriptTests testing the JavaScript message passing
> interface to and from Java.

rnewman: preliminary pass, when you get a moment.  You too, mfinkle.  As discussed with rnewman, this sandboxes each Java addon behind a unique GUID.  The motivation is to make all Gecko interactions explicit, so that reviewers can see the flow between Gecko and the Java addon.  (Previous approaches let Java addons sip the Gecko event stream.  They still can, they just need to be explicit in the JavaScript that does it.)

Known issues:

* Commenting is minimal.  So is MDN-style whole feature documentation.  It'll come.

* The JUnit 3 test is out of date.  There is some automated testing in place; it seems to work.

* It's hard to access classes.dex in an addon right now.  I'll add URI resolution and GeckoJarReader support into the Java side next, but I want to get a demo out before adding this polish.

* I'm not providing a Context to the addon constructor yet.  We could also expose this via the event dispatcher.  In some way, the Java code will want one.

* I've largely ignored thread considerations.  I'm not really certain what the right model is; I don't think it wouldn't be bad to assert that Java code invocations always happen on the main thread, and to act accordingly, because lots of Android code needs to happen on the main thread.
Flags: needinfo?(mark.finkle)
tracking-p11: --- → +
Comment on attachment 8614305 [details]
MozReview Request: Bug 1168407 - Pre: Move roboextender under mobile/android/tests. r?gbrown

https://reviewboard.mozilla.org/r/9879/#review8775

::: mobile/android/base/EventDispatcher.java:164
(Diff revision 1)
> +                // There were native listeners, and they're gone.  Dispatch an error rather than

Get a rubberstamp from nchen.

::: mobile/android/base/JavaAddonManagerV1.java:6
(Diff revision 1)
> +package org.mozilla.gecko;

I wish this and JAM.java were in a subpackage/subdir…

::: mobile/android/base/JavaAddonManagerV1.java:40
(Diff revision 1)
> +    private final Map<String, JavaAddonEventDispatcherV1Impl> mGuidToDispatcherMap = new HashMap<>();

s/Guid/GUID/g

::: mobile/android/base/JavaAddonManagerV1.java:55
(Diff revision 1)
> +            // we've already done this registration. don't do it again

Nit: capitals and punctuation.

::: mobile/android/base/JavaAddonManagerV1.java:54
(Diff revision 1)
> +        if (mApplicationContext != null) {

You haven't defined a threading policy for `init`, so you can get double inits (or worse).

Either impose a thread policy for init, or you need to make mApplicationContext an AtomicReference.

::: mobile/android/base/JavaAddonManagerV1.java:71
(Diff revision 1)
> +                .class);

Remove the unnecessary linebreak.

::: mobile/android/base/JavaAddonManagerV1.java:82
(Diff revision 1)
> +            if (event.equals("JavaAddonManagerV1:Load")) {

Extract constants, use string switch.

::: mobile/android/base/JavaAddonManagerV1.java:111
(Diff revision 1)
> +     * Curiously, the dispatcher does not hold a direct reference to its instance.  It will

It would help to clarify here: "add-on instance".

::: mobile/android/base/JavaAddonManagerV1.java:125
(Diff revision 1)
> +            this.dexFileName = dexFileName;

This is never used.

::: mobile/android/base/JavaAddonManagerV1.java:116
(Diff revision 1)
> +    private class JavaAddonEventDispatcherV1Impl implements JavaAddonEventDispatcherV1 {

There's nothing about this that is specific to JavaAddonEventListenerV1, really. You could make this

```
private class PrefixedEventDispatcher<T: NativeEventListener> ...
```

right?

::: mobile/android/base/JavaAddonManagerV1.java:120
(Diff revision 1)
> +        // Both protected by synchronized(this) blocks.

Both?

::: mobile/android/base/JavaAddonManagerV1.java:135
(Diff revision 1)
> +            final NativeEventListener listenerWrapper = new NativeEventListener() {

I'd lift this out and make it static, with explicit constructor parameters. You're probably capturing a big closure here.

::: mobile/android/base/JavaAddonManagerV1.java:192
(Diff revision 1)
> +                it.remove();

Why bother? You're unregistering everything in a synchronized block. Just call .clear once!

::: mobile/android/base/JavaAddonManagerV1.java:202
(Diff revision 1)
> +                    if (callback == null) {

Do this check *outside* your anonymous class!

::: mobile/android/config/proguard/proguard.cfg:214
(Diff revision 1)
> +-keep class org.mozilla.javaddons.* {

Typo: javaddons -> javaaddons.

::: mobile/android/modules/JavaAddonManager.jsm:12
(Diff revision 1)
> +Cu.import("resource://gre/modules/Messaging.jsm"); /*global Messaging */

Kill /*these useless comments*/

::: mobile/android/modules/JavaAddonManager.jsm:44
(Diff revision 1)
> +};

No semicolon after top-level functions.

::: mobile/android/modules/JavaAddonManager.jsm:66
(Diff revision 1)
> +  _prefix: function(aMessage) {

What is this, 1990?  :P

  _prefix: function (message) {

or be modern:

  _prefix(message) {

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions

::: mobile/android/modules/JavaAddonManager.jsm:41
(Diff revision 1)
> +  this._guid = options.guid;

Check options.guid for sanity.

::: mobile/android/tests/javaaddons/AndroidManifest.xml.in:8
(Diff revision 1)
> +    <uses-sdk android:minSdkVersion="8"

9?

::: mobile/android/tests/browser/junit3/src/TestJavaAddonManager.java:23
(Diff revision 1)
> +//    public void testJavaAddonManager() throws Exception {

Do you plan to uncomment these?
Attachment #8614305 - Flags: review?(rnewman)
Comment on attachment 8614305 [details]
MozReview Request: Bug 1168407 - Pre: Move roboextender under mobile/android/tests. r?gbrown

Bug 1168407 - Implement a bidirectional Java addon interface. r=kats,rnewman

There are several parts to this ticket:

1) Produce javaaddons-1.0.jar, a standalone JAR defining a (versioned)
Java interface suitable for consumption by third-party Java addon
implementations.

2) Support the new V1 interface in the JavaAddonManager.

3) Add JUnit 3 tests of the class-finding functionality.  This
includes building a test DEX file that implements the V1 interface
(consuming the built JAR).  This is both a functional test and living
documentation of how to produce such an addon.

4) Add Robocop JavascriptTests testing the JavaScript message passing
interface to and from Java.
Bug 1168407 - Review comment: Remove outdated test.

I would like to have JUnit 3 tests but it's not easy to load a
classes.dex file in JUnit 3 tests.  I guess I could put a classes.dex
in robocop.apk/assets.  Future work.
Bug 1168407 - Move everything into JavaAddonInterfaceV1. r=rnewman

This patch is motivated by mfinkle's desire to not have version
information appear in consuming code.  This gets most of the way
there: consumers can import a single JavaAddonInterfaceV1 interface,
with internal sub-interfaces that are not versioned.
Attachment #8620458 - Flags: review?(rnewman)
Bug 1168407 - Pre: Add GeckoJarReader.extractStream. r=rnewman

The use case is to ship a classes.dex file in a Gecko add-on.  This
makes it easy to extract such a file from an add-on to a temporary
location.  (Sadly, the Android Dex loading classes expect files, not
streams.)
Attachment #8620459 - Flags: review?(rnewman)
Bug 1168407 - Load classes.dex files from more general Gecko URIs. r=rnewman

This makes it easy to ship a classes.dex file from within a Gecko
add-on.  We use this mechanism, including a classes.dex in the
Roboextender add-on, to test this code path.
Attachment #8620460 - Flags: review?(rnewman)
https://reviewboard.mozilla.org/r/9877/#review9631

::: mobile/android/base/EventDispatcher.java:161
(Diff revision 2)
>              if (listeners.size() == 0) {

isEmpty()

::: mobile/android/base/javaaddons/JavaAddonManager.java:85
(Diff revision 2)
> +            if (event.equals("Dex:Load")) {

String switch(), please.

::: mobile/android/base/javaaddons/JavaAddonManagerV1.java:39
(Diff revision 2)
> +    private final org.mozilla.gecko.EventDispatcher mDispatcher;

Move mDispatcher so it doesn't look like it's affected by the "Protected by..." comment.

::: mobile/android/base/BrowserApp.java:45
(Diff revision 2)
> +import org.mozilla.gecko.javaaddons.JavaAddonManager;

This doesn't seem to be used.

::: mobile/android/base/util/GeckoJarReader.java:116
(Diff revision 2)
> +        OutputStream output = new FileOutputStream(file);

You should do this *after* you've made the decision to read (i.e., line 140), and you should close it in a try..finally.

::: mobile/android/base/util/GeckoJarReader.java:144
(Diff revision 2)
> +                output.write(buf, 0, len);

There has gotta be a utility in the tree that copies between two streams, right?

::: mobile/android/base/util/GeckoJarReader.java:113
(Diff revision 2)
> +    public static File extractStream(Context context, String url, File dir) throws IOException {

/**
 * Extracts the provided streaming resource from our jar into a temporary file. The caller is responsible for deleting the file after use, or copying it to a stable location.
 */

What happens if this is called with varying invalid inputs?
Attachment #8614305 - Attachment description: MozReview Request: Bug 1168407 - Implement a bidirectional Java addon interface. r=kats,rnewman → MozReview Request: Bug 1168407 - Pre: Move roboextender under mobile/android/tests. r?gbrown
Attachment #8614305 - Flags: review?(rnewman) → review?(gbrown)
Comment on attachment 8614305 [details]
MozReview Request: Bug 1168407 - Pre: Move roboextender under mobile/android/tests. r?gbrown

Bug 1168407 - Pre: Move roboextender under mobile/android/tests. r?gbrown

I want to include a file built during the Fennec build to the
roboextender extension; this is an easy way to arrange it.

I took the opportunity to make the extension file tree mirror the XPI
file tree, which always used base/.
Comment on attachment 8620459 [details]
MozReview Request: Bug 1168407 - Pre: Add GeckoJarReader.extractStream. r=rnewman

Bug 1168407 - Pre: Add GeckoJarReader.extractStream. r=rnewman

The use case is to ship a classes.dex file in a Gecko add-on.  This
makes it easy to extract such a file from an add-on to a temporary
location.  (Sadly, the Android Dex loading classes expect files, not
streams.)
Comment on attachment 8620454 [details]
MozReview Request: Bug 1168407 - Implement a bidirectional Java addon interface. f=jchen,r=rnewman r?mfinkle

Bug 1168407 - Implement a bidirectional Java addon interface. f=jchen,r=rnewman r?mfinkle

There are several parts to this ticket:

1) Produce javaaddons-1.0.jar, a standalone JAR defining a (versioned)
Java interface suitable for consumption by third-party Java addon
implementations.

2) Support the new V1 interface in the JavaAddonManager.

3) Add Robocop JavascriptTests testing the JavaScript message passing
interface to and from Java.

This patch can be read as "not in tests/" and "everything in tests/".
Attachment #8620454 - Attachment description: MozReview Request: Bug 1168407 - Review comment: add org.mozilla.gecko.javaaddons. → MozReview Request: Bug 1168407 - Implement a bidirectional Java addon interface. f=jchen,r=rnewman r?mfinkle
Attachment #8620454 - Flags: review?(rnewman)
Attachment #8620454 - Flags: review?(mark.finkle)
Attachment #8620458 - Attachment is obsolete: true
Attachment #8620458 - Flags: review?(rnewman)
Attachment #8620460 - Attachment is obsolete: true
Attachment #8620460 - Flags: review?(rnewman)
Comment on attachment 8620454 [details]
MozReview Request: Bug 1168407 - Implement a bidirectional Java addon interface. f=jchen,r=rnewman r?mfinkle

This incorporates rnewman's review.  Thanks, Richard!
Attachment #8620454 - Flags: review?(rnewman) → review+
https://reviewboard.mozilla.org/r/9877/#review9807

> This doesn't seem to be used.

This was because JAM moved packages.

> String switch(), please.

This existed before me.  (I did the one I added.)

> /**
>  * Extracts the provided streaming resource from our jar into a temporary file. The caller is responsible for deleting the file after use, or copying it to a stable location.
>  */
> 
> What happens if this is called with varying invalid inputs?

Funny story -- https://bugzilla.mozilla.org/show_bug.cgi?id=1174922 happens :)

> There has gotta be a utility in the tree that copies between two streams, right?

Sadly no, and I'm not blocking this Q2 goal on that :)
Comment on attachment 8620459 [details]
MozReview Request: Bug 1168407 - Pre: Add GeckoJarReader.extractStream. r=rnewman

rnewman reviewed this already.
Attachment #8620459 - Flags: review?(rnewman) → review+
Comment on attachment 8614305 [details]
MozReview Request: Bug 1168407 - Pre: Move roboextender under mobile/android/tests. r?gbrown

https://reviewboard.mozilla.org/r/9879/#review9813

Ship It!
Attachment #8614305 - Flags: review?(gbrown) → review+
(In reply to Nick Alexander :nalexander from comment #19)
> https://reviewboard.mozilla.org/r/9877/#review9807
> 
> > This doesn't seem to be used.
> 
> This was because JAM moved packages.
> 
> > String switch(), please.
> 
> This existed before me.  (I did the one I added.)

We avoid using switch() because it breaks when building GeckoView apps (or did).
Flags: needinfo?(mark.finkle)
We use source level 7 now, for quite a while, and elsewhere in the codebase. If we were going to break GV consumers, we would already have done so… but I expect most of them were using SL7 long before us.
(In reply to Richard Newman [:rnewman] from comment #24)
> We use source level 7 now, for quite a while, and elsewhere in the codebase.
> If we were going to break GV consumers, we would already have done so… but I
> expect most of them were using SL7 long before us.

... and my memory is clearing: We had to avoid switch() with R.id.xxx stuff, not plain-old-strings.
I started working on documentation for this.  It's very much Not Finished, since it doesn't describe the Java side of the interface, and it is not hyper-linked: https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/JavaAddonManager.jsm
(In reply to Nick Alexander :nalexander from comment #27)
> mfinkle: latest try build looks good.  testJavaAddons is passing in
> http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/nalexander@mozilla.
> com-d9ddeee1bb90/try-android-api-11/try_panda_android_test-robocop-7-bm102-
> tests1-panda-build1578.txt.gz.

For reasons I don't understand, I have consistent failures in rc3 on Android 9.  I'll hunt it down tomorrow.
(In reply to Nick Alexander :nalexander from comment #29)
> (In reply to Nick Alexander :nalexander from comment #27)
> > mfinkle: latest try build looks good.  testJavaAddons is passing in
> > http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/nalexander@mozilla.
> > com-d9ddeee1bb90/try-android-api-11/try_panda_android_test-robocop-7-bm102-
> > tests1-panda-build1578.txt.gz.
> 
> For reasons I don't understand, I have consistent failures in rc3 on Android
> 9.  I'll hunt it down tomorrow.

This turned out to be simple: older Android versions don't support loading from .DEX directly, see [1] and [2].  They can still load from .APK and .JAR files.

It would be possible to work around this by producing a .JAR file at run time, and this is even potentially attractive, since we extract .DEX files and write them to temporary locations in the patch already.  However, in the interest of time, we're going to test against .APK files and instead document the restrictions for add-on consumers to be aware of.

[1] http://androidxref.com/4.4.4_r1/xref/libcore/dalvik/src/main/java/dalvik/system/DexPathList.java
[2] http://androidxref.com/2.3.6/xref/libcore/dalvik/src/main/java/dalvik/system/DexClassLoader.java
(In reply to Nick Alexander :nalexander from comment #30)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=74da3c2f5888

gps: This try build appears to be busted due to config/recurse.mk not doing what I expect.  Specifically, if you look at the two relevant build logs [1] and [2], you can see that we try to copy javaaddons-test.apk into roboextender@mozilla.org before it's built, see [3].

I have added a dependency in https://hg.mozilla.org/try/file/74da3c2f5888/config/recurse.mk#l152 like

mobile/android/tests/browser/robocop/roboextender/tools: mobile/android/tests/javaaddons/tools

I have a tools:: rule in https://hg.mozilla.org/try/file/74da3c2f5888/mobile/android/tests/javaaddons/Makefile.in#l14

I have a tools:: rule in https://hg.mozilla.org/try/file/74da3c2f5888/mobile/android/tests/browser/robocop/roboextender/Makefile.in#l22

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nalexander@mozilla.com-08350b798b7d/try-android-api-9/t[y-android-api-9-bm76-try1-build245.txt.gz

[2] http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nalexander@mozilla.com-08350b798b7d/try-android-api-11/try-android-api-11-bm78-try1-build451.txt.gz

[3] 11:56:43     INFO -  Verification succesful
11:56:43     INFO -  gmake[5]: Leaving directory `/builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/mobile/android/tests/browser/junit3'
11:56:43     INFO -  gmake[5]: Entering directory `/builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/mobile/android/tests/browser/robocop/roboextender'
11:56:43     INFO -  mkdir -p ../../../../../../_tests/testing/mochitest/extensions/roboextender@mozilla.org/base
11:56:43     INFO -  cp /builds/slave/try-and-api-9-0000000000000000/build/src/mobile/android/tests/browser/robocop/roboextender/base/* ../../../../../../_tests/testing/mochitest/extensions/roboextender@mozilla.org/base
11:56:43     INFO -  cp ../../../../../../mobile/android/tests/javaaddons/javaaddons-test.apk ../../../../../../_tests/testing/mochitest/extensions/roboextender@mozilla.org/base
11:56:43     INFO -  cp: cannot stat `../../../../../../mobile/android/tests/javaaddons/javaaddons-test.apk': No such file or directory
11:56:43     INFO -  gmake[5]: [tools] Error 1 (ignored)
11:56:43     INFO -  gmake[5]: Leaving directory `/builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/mobile/android/tests/browser/robocop/roboextender'
11:56:43     INFO -  gmake[5]: Entering directory `/builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/mobile/android/tests/javaaddons'
11:56:43     INFO -  javaaddons-test.jar
11:56:43     INFO -  /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/build-tools/22.0.1/aapt package -f -M /builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/mobile/android/tests/javaaddons/AndroidManifest.xml -I /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/platforms/android-22/android.jar -S /builds/slave/try-and-api-9-0000000000000000/build/src/mobile/android/tests/javaaddons/res  \
11:56:43     INFO -  	-J . \
11:56:43     INFO -  	-F javaaddons-test.ap_
11:56:43     INFO -  /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/build-tools/22.0.1/aapt: /lib/libz.so.1: no version information available (required by /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/build-tools/22.0.1/aapt)
11:56:43     INFO -  /usr/bin/javac -target 1.7 -source 1.7 -classpath /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/platforms/android-22/android.jar -bootclasspath /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/platforms/android-22/android.jar -encoding UTF8 -g:source,lines -Werror  -Xlint:all -d javaaddons-test-classes -classpath /builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/mobile/android/javaaddons/javaaddons-1.0.jar /builds/slave/try-and-api-9-0000000000000000/build/src/mobile/android/tests/javaaddons/src/org/mozilla/javaaddons/test/ClassWithNoRecognizedConstructors.java /builds/slave/try-and-api-9-0000000000000000/build/src/mobile/android/tests/javaaddons/src/org/mozilla/javaaddons/test/JavaAddonV0.java /builds/slave/try-and-api-9-0000000000000000/build/src/mobile/android/tests/javaaddons/src/org/mozilla/javaaddons/test/JavaAddonV1.java
11:56:44     INFO -  /usr/bin/jar cMf javaaddons-test.jar -C javaaddons-test-classes .
11:56:44     INFO -  /usr/bin/javac -target 1.7 -source 1.7 -classpath /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/platforms/android-22/android.jar -bootclasspath /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/platforms/android-22/android.jar -encoding UTF8 -g:source,lines -Werror  -d classes R.java \
11:56:44     INFO -  	-classpath javaaddons-test.jar
11:56:44     INFO -  /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/build-tools/22.0.1/dx --dex --output=classes.dex classes javaaddons-test.jar
11:56:45     INFO -  cp javaaddons-test.ap_ javaaddons-test-unsigned-unaligned.apk
11:56:45     INFO -  /usr/bin/zip -0 javaaddons-test-unsigned-unaligned.apk classes.dex
11:56:45     INFO -    adding: classes.dex (stored 0%)
11:56:45     INFO -  cp javaaddons-test-unsigned-unaligned.apk javaaddons-test-unaligned.apk
11:56:45     INFO -  /builds/slave/try-and-api-9-0000000000000000/build/src/obj-firefox/_virtualenv/bin/python /builds/slave/try-and-api-9-0000000000000000/build/src/mobile/android/debug_sign_tool.py --keytool=/usr/bin/keytool --jarsigner=/usr/bin/jarsigner  javaaddons-test-unaligned.apk
11:56:46     INFO -  /builds/slave/try-and-api-9-0000000000000000/build/src/android-sdk-linux/build-tools/22.0.1/zipalign -f -v 4 javaaddons-test-unaligned.apk javaaddons-test.apk
11:56:46     INFO -  Verifying alignment of javaaddons-test.apk (4)...
11:56:46     INFO -        50 META-INF/MANIFEST.MF (OK - compressed)
11:56:46     INFO -       326 META-INF/ANDROIDD.SF (OK - compressed)
11:56:46     INFO -       669 META-INF/ANDROIDD.RSA (OK - compressed)
11:56:46     INFO -      1787 AndroidManifest.xml (OK - compressed)
11:56:46     INFO -      2264 resources.arsc (OK)
11:56:46     INFO -      2916 classes.dex (OK)
11:56:46     INFO -  Verification succesful
Flags: needinfo?(gps)
I would grep for "tools" in <objdir>/root-deps.mk and <objdir>/root.mk and verify that it has the directories you added to recurse.mk. The "recurse_tools" rule from root-deps.mk is what get executed when you do `make tools`. So it should be easy to debug this...

Watch out for "$(MAKE) -C <dir> tools" invocations in Makefile.in files: I think there are still a few.
Flags: needinfo?(gps)
(In reply to Nick Alexander :nalexander from comment #36)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=99d3ff2d0db2

mfinkle: this try is very green.  (I used a hammer to make the build ordering work.)

There are a bunch of tiny patches to fold into the main one but the meat is in place.  Can you review (or delegate) the main patch?
Flags: needinfo?(mark.finkle)
Comment on attachment 8620454 [details]
MozReview Request: Bug 1168407 - Implement a bidirectional Java addon interface. f=jchen,r=rnewman r?mfinkle

https://reviewboard.mozilla.org/r/10757/#review10169

::: mobile/android/base/javaaddons/JavaAddonManagerV1.java:109
(Diff revision 2)
> +                    dispatcher.unregisterAllEventListeners();

Do we need a sendSuccess or sendError call in the MESSAGE_UNLOAD handler?
Attachment #8620454 - Flags: review?(mark.finkle)
Comment on attachment 8620454 [details]
MozReview Request: Bug 1168407 - Implement a bidirectional Java addon interface. f=jchen,r=rnewman r?mfinkle

https://reviewboard.mozilla.org/r/10757/#review10171

Everything looks OK. Just check on the need for the success or error callback.
Attachment #8620454 - Flags: review+
Bombs away.  I added the callback call as mfinkle suggested.
https://hg.mozilla.org/mozilla-central/rev/c30f0b5c6e50
https://hg.mozilla.org/mozilla-central/rev/d99a590f2524
https://hg.mozilla.org/mozilla-central/rev/60db000f3acb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Flags: needinfo?(mark.finkle)
Release Note Request (optional, but appreciated)
[Why is this notable]: Improvement to Java use in Fennec add-ons
[Suggested wording]: JavaAddonManager interface can now send, receive and respond to messages from Gecko
[Links (documentation, blog post, etc)]:
https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/JavaAddonManager.jsm
Blocks: 1363843
You need to log in before you can comment on or make changes to this bug.