976249 - Reduce JSON exposed in our API

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

JSON is a data exchange format that's useful for communicating with Gecko. But once we've received JSON on the Java end, keeping the data in JSON form is not useful in most cases, and in fact is detrimental for several reasons:
* Using JSON too eagerly can result in JSON building/parsing where it's not even needed; at [1], for example, we parse the JSON to read the result, but this JSON is built in Java right before executing that callback [2].
* Accepting JSON strings as parameters or returning JSON strings as results muddles our API; it's less clear what methods are expecting or returning.
* If we decide to change how we communicate with Gecko, we should be able to confine those changes to just the communication points; if JSON is propogated across the API, however, these changes will require more effort.
* It is ugly to build and parse JSON in Java (especially with checked JSONExceptions).

Instead of having any methods that accept and return JSON, we should strongly consider using standard Java classes instead. Any necessary JSON parsing should be done immediately when received in handleMessage(); any JSON building should be done immediately before sending the event to Gecko. Perhaps we could even consider using another JSON library such as GSON [3], which automagically translates JSON strings to/from Java objects [4].

[1] http://hg.mozilla.org/mozilla-central/file/1422dfcd7fd8/mobile/android/base/BrowserApp.java#l2382
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/prompts/Prompt.java#414
[3] https://code.google.com/p/google-gson/
[4] https://sites.google.com/site/gson/gson-user-guide#TOC-Object-Examples

Comment 1

[Wesley Johnston (:wesj)]

The prompt examples probably aren't the best. i.e. it shouldn't be hard to adapt those to return non-JSON, but for simplicity I've always just left it in. i.e. its not performance critical.

I worry this would actually lead to us doing un-necessary parsing. i.e. we'd parse everything whether it was used or not. We'd have to start being (more) careful with what we sent across the bridge from js to make sure we went sending extraneous information...

We talked at one point about using the JSApi to convert JS Objects to Java HashMaps via JNI. I think ckitching was working on that, but I never saw anything come out of it.

Comment 2

[Brian Nicholson (:bnicholson)]

(In reply to Wesley Johnston (:wesj) from comment #1)
> I worry this would actually lead to us doing un-necessary parsing. i.e. we'd
> parse everything whether it was used or not. We'd have to start being (more)
> careful with what we sent across the bridge from js to make sure we went
> sending extraneous information...

We should avoid sending extraneous data regardless. Just as we would want to avoid any unnecessary parsing on the Java end, we would also want to avoid unnecessary stringification on the JS end.

Comment 3

[Nick Alexander :nalexander [he/him]]

From a different perspective, it's hard to track JSON data flowing through the application.  Parsing to POJOs and then tracking consumers of those instances is much easier... with an IDE.

Comment 4

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.