Closed Bug 1081276 Opened 7 years ago Closed 7 years ago

Don't use Java assertions

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 35

People

(Reporter: ckitching, Assigned: ckitching)

Details

Attachments

(1 file, 2 obsolete files)

Java assertions should not be used on Android.

We have our own Assert class which (... hopefully...) gets stripped by Proguard in release builds.

Let's fix this.
Attached patch Don't use Java assertions (obsolete) — Splinter Review
Attachment #8503358 - Flags: review?(rnewman)
Assignee: nobody → chriskitching
Status: NEW → ASSIGNED
Attachment #8503358 - Flags: review?(rnewman) → review+
OS: Linux → Android
Hardware: x86_64 → All
... bloody third party code...
Attachment #8503477 - Flags: review?(rnewman)
Attachment #8503358 - Attachment is obsolete: true
Attachment #8503477 - Flags: review?(rnewman) → review+
Attachment #8503619 - Flags: review?(rnewman)
Comment on attachment 8503619 [details] [diff] [review]
Don't notify() without holding the monitor

Whoops!
Attachment #8503619 - Attachment is obsolete: true
Attachment #8503619 - Flags: review?(rnewman)
https://hg.mozilla.org/mozilla-central/rev/1ef3c935199a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
(In reply to Chris Kitching [:ckitching] from comment #0)
> Java assertions should not be used on Android.

To elaborate:
  1) it looks like they can't be turned on easily because you don't control the JVM: http://softwarecave.org/2014/01/14/assertions-on-android/
  2) Even if you could use them, the assertions stay in the class files and can bloat the release code: https://docs.oracle.com/javase/7/docs/technotes/guides/language/assert.html#usage-adv-removing
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.