Closed Bug 976249 Opened 8 years ago Closed 10 months ago

Reduce JSON exposed in our API

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: bnicholson, Unassigned)

Details

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
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.
(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.
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.
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.
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.