Closed Bug 684727 Opened 8 years ago Closed 8 years ago

Don't call into JNI from ExtensionsView.init

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 9

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
In an effort to hide any orphaned Android system notifications, Fennec fires code to clear any notifications on startup. This happens all the time, whether we have orphaned notifications or not. This means we call into the Android nsIAlertsService impl and that means calling JNI too.

This patch uses a preference to identify situations where we might have an orphaned notification. We remove the extra code called during startup and removes calling into JNI unless we really need to.

Note: We need to make sure this patch does not regress bug 620975. Bug 620975 was the reason we attempt to always clear the notification.
Attachment #558298 - Flags: review?(mbrubeck)
Attachment #558298 - Flags: feedback?(alexp)
Comment on attachment 558298 [details] [diff] [review]
patch

Looks like a regression. I've installed AdBlock Plus, when the notification appeared killed the running Fennec process with a task manager. On restart the notification did not hide.
Attached patch patch 2 (obsolete) — Splinter Review
This patch fixes a missing identifier problem in BrowserStartup.js

Thanks Alex!
Assignee: nobody → mark.finkle
Attachment #558298 - Attachment is obsolete: true
Attachment #558298 - Flags: review?(mbrubeck)
Attachment #558298 - Flags: feedback?(alexp)
Attachment #558529 - Flags: review?(mbrubeck)
Attachment #558529 - Flags: feedback?(alexp)
ADDONS_NOTIFICATION_NAME wasn't defined
OS: Linux → Android
Hardware: x86 → All
Attachment #558529 - Flags: review?(mbrubeck) → review+
Attached patch patch 3Splinter Review
Alex found a case where we don't clear the notification after a crash. I couldn't reproduce, but I think saving the prefs to disk ASAP is probably a good thing.
Attachment #558529 - Attachment is obsolete: true
Attachment #558529 - Flags: feedback?(alexp)
Attachment #558589 - Flags: review?(mbrubeck)
Attachment #558589 - Flags: review?(mbrubeck) → review+
(In reply to Mark Finkle (:mfinkle) from comment #4)
> Alex found a case where we don't clear the notification after a crash. I
> couldn't reproduce, but I think saving the prefs to disk ASAP is probably a
> good thing.

The latest patch finally works in my case too!
Thank you Mark!
OS: Android → Linux
Hardware: All → x86
http://hg.mozilla.org/mozilla-central/rev/1ddde977131a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 9
Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110926
Firefox/9.0a1 Fennec/9.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.