1202767 - Interact with Android notifications properly

Status: RESOLVED FIXED

(B2GDroid Graveyard :: General, defect)

Description

[Jacqueline Savory [:jsavory] UX]

Explore bringing Android notifications to B2G utility tray or modifying the Android utility tray.

Comment 1

[sgiles]

Attachment: bug1202767-initial.patch

Comment 2

[sgiles]

This patch will send a notification to Gaia when an Android notification is received - just the nuts and bolts so far.

Requires you to allow apps to view Notifications in Android settings:

For Android >= 5.0, enable for "Firefox OS" under Settings -> Sound and notifications -> Notification access.

For Android >= 4.3 but < 5.0 enable fore "Firefox OS" in Settings -> Security -> Notifications access.


On receiving a Notification you'll see in the logs:

```
I/B2G:NotificationObserver(21453): Android notification received
I/B2GDroid:GeckoEventReceiver(21932): Sending event (Android:Notification) to Gecko
I/GeckoJNI(21932): notifyGeckoOfEvent
I/Gecko   (21932): -*- MessagesBridge Got Android:Notification message
I/Gecko   (21932): XXX FIXME : Dispatch a mozChromeNotificationEvent: desktop-notification
E/GeckoConsole(21932): [JavaScript Error: "ReferenceError: Intl is not defined" {file: "app://system.gaiamobile.org/shared/js/moz_intl.js" line: 321}]
E/GeckoConsole(21932): global.mozIntl._gaia.RelativeDate@app://system.gaiamobile.org/shared/js/moz_intl.js:321:13
E/GeckoConsole(21932): ns_addNotification@app://system.gaiamobile.org/js/notification_screen.js:473:21
E/GeckoConsole(21932): ns_handleEvent@app://system.gaiamobile.org/js/notification_screen.js:143:13
```

So this may depend on bug 1211623 - unless there is a flag I'm missing to turn off Intl in Gaia.

Comment 3

[[:fabrice] Fabrice Desré]

You need bug 864843 to fix the Intl issues.

With this patch, do we display notifications both on Android and FxOS ? What happens when we tap on a notification?

Comment 4

[sgiles]

This is just the glue really, at the moment you'll just see the notification in the notification tray in Gaia as well as in Android - we could immediately remove the Android Notification once we've received it - but I'm not sure how it will work with sticky notifications.  

It would be fairly easy to map to an Intent when the notification is tapped.

Another avenue I've looked at is disabling the notification tray in Android.  However, the security model makes it difficult - there is a method to disable it, but you need some Reflection magic [1] to get to it because it's an internal API and then the permission model prevents third party apps anyway [2].

It would probably be easier, and, I think, better for Android users, if we posted notifications from FxOS apps as Android notifications and disabled the Gaia notification tray.  I think trying to do too much against Android will end up in a whole load of hacks and result in an imperfect UX. We'll also end up missing some Android niceties - I use the Spotify lock screen widget all the time, for example.

[1] 
```
 private void disableStatusBar() {
        try {
            Object statusBarManager = getSystemService("statusbar");
            Class<?> statusBarManagerClass = statusBarManager.getClass();
            Method disable = statusBarManagerClass.getMethod("disable", int.class);

            Field disableMaskField = statusBarManagerClass.getDeclaredField("DISABLE_MASK");
            int disableMask = disableMaskField.getInt(statusBarManager);

            try {
                disable.invoke(statusBarManager, disableMask);
            } catch(InvocationTargetException e) {
                // Outputs: F/B2G ( 9332): disable(int) threw exception StatusBarManagerService: Neither user 10286 nor current process has android.permission.STATUS_BAR.
                Log.wtf(LOGTAG, "disable(int) threw exception " + e.getCause().getMessage());
            } catch(Exception e) {
                Log.wtf(LOGTAG, "Could not invoke StatusBarManager.disable" + e.toString());
            }
        } catch(Exception e) {
            Log.wtf(LOGTAG, "Could not access android.app.StatusBarManager" + e.toString());
        }

    }
```

[2] http://developer.android.com/reference/android/Manifest.permission.html#STATUS_BAR

Comment 5

[[:fabrice] Fabrice Desré]

Jacqueline, since we can't disable the Android notification tray, what do you recommend overall for our own tray & notification area?

Comment 6

[sgiles]

Attachment: android-notifications-in-gaia.patch

This brings Android notifications into Gaia.

I couldn't see an easy way to get the blob from the Android API over to Gecko - so I base64 encoded the bitmap.

Comment 7

[[:fabrice] Fabrice Desré]

Comment on attachment 8681303 [details] [diff] [review]
android-notifications-in-gaia.patch

Review of attachment 8681303 [details] [diff] [review]:
-----------------------------------------------------------------

Mostly nits, but the main issue is that I get this error that prevents anything from actually working:

F/B2GDroid:NotificationObserver(16424): Error building android notification message Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
D/skia    (16424): --- SkImageDecoder::Factory returned null
F/B2GDroid:NotificationObserver(16424): Error building android notification message Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
D/skia    (16424): --- SkImageDecoder::Factory returned null
F/B2GDroid:NotificationObserver(16424): Error building android notification message Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
W/mpdecision(  259): type=1400 audit(0.0:13269): avc: granted { dac_override } for capability=1 scontext=u:r:mpdecision:s0 tcontext=u:r:mpdecision:s0 tclass=capability
W/mpdecision(  259): type=1400 audit(0.0:13270): avc: granted { dac_override } for capability=1 scontext=u:r:mpdecision:s0 tcontext=u:r:mpdecision:s0 tclass=capability
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
D/skia    (16424): --- SkImageDecoder::Factory returned null
F/B2GDroid:NotificationObserver(16424): Error building android notification message Attempt to invoke virtual method 'boolean android.graphics.Bitmap.compress(android.graphics.Bitmap$CompressFormat, int, java.io.OutputStream)' on a null object reference
I/GeckoNotificationHelper(19665): Send {"id":"{7370a644-dba4-4491-85ea-b712bb22b016}","handlerKey":"downloads","cookie":"\"\/storage\/emulated\/0\/Download\/b2gdroid(2).apkhttp:\/\/fabricedesre.github.io\/b2gdroid\/b2gdroid.apkFri Oct 30 2015 10:50:57 GMT-0700 (PDT)\"","eventType":"notification-closed"}
I/GeckoConsole(19665): Notification:Event {"id":"{7370a644-dba4-4491-85ea-b712bb22b016}","handlerKey":"downloads","cookie":"\"\/storage\/emulated\/0\/Download\/b2gdroid(2).apkhttp:\/\/fabricedesre.github.io\/b2gdroid\/b2gdroid.apkFri Oct 30 2015 10:50:57 GMT-0700 (PDT)\"","eventType":"notification-closed"}
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
I/B2GDroid:NotificationObserver(16424): onNotificationPosted(aNotification, aRanking)
I/B2GDroid:NotificationObserver(16424): Creating broadcast event intent
I/B2GDroid:GeckoEventReceiver(15422): Received GeckoEvent: Android:Notification

::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/GeckoEventReceiver.java
@@ +32,5 @@
> +    private static final String EXTRA_SUBJECT = "SUBJECT";
> +    private static final String EXTRA_DATA = "DATA";
> +
> +    public static Intent createBroadcastEventIntent(String subject, String data) {
> +

nit: extra blank line

@@ +40,5 @@
> +        return intent;
> +    }
> +
> +    public void registerWithContext(Context aContext) {
> +

here too

@@ +44,5 @@
> +
> +        // Register with the appropriate IntentFilter
> +        IntentFilter intentFilter = new IntentFilter();
> +        intentFilter.addAction(SEND_GECKO_EVENT);
> +        aContext.registerReceiver(this, intentFilter);

you can just do aContext.registerReceiver(this, new IntentFilter(SEND_GECKO_EVENT));

::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/Launcher.java
@@ +19,5 @@
>  import android.util.Log;
>  import android.view.View;
> +import android.view.WindowManager;
> +import android.view.Gravity;
> +import android.graphics.PixelFormat;

What are these for?

@@ +49,5 @@
>      private ScreenStateObserver mScreenStateObserver;
>      private Apps                mApps;
>      private SettingsMapper      mSettings;
> +    private GeckoEventReceiver  mGeckoEventReceiver;
> +    private RemoteGeckoEventProxy mGeckoEventProxy;

nit: re-indent everything properly.

@@ +101,5 @@
> +        mGeckoEventProxy = new RemoteGeckoEventProxy(this);
> +
> +        mGeckoEventReceiver = new GeckoEventReceiver();
> +        mGeckoEventReceiver.registerWithContext(this);
> +

extra blank line

@@ +114,5 @@
> +        // Register the RemoteGeckoEventProxy with the Notification Opened
> +        // event, Notifications are handled in a different process as a
> +        // service, so we need to forward them to the remote service
> +        EventDispatcher.getInstance().registerGeckoThreadListener(mGeckoEventProxy,
> +            "Android:NotificationOpened");

Indentation looks wrong

::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/NotificationObserver.java
@@ +19,5 @@
> +
> +import android.content.res.Resources;
> +
> +import android.service.notification.NotificationListenerService;
> +import android.service.notification.NotificationListenerService.RankingMap;

this seems unused

@@ +29,5 @@
> +import android.util.Base64;
> +import android.util.Log;
> +
> +import org.json.JSONObject;
> +import org.json.JSONArray;

this seems unused

@@ +33,5 @@
> +import org.json.JSONArray;
> +
> +import org.mozilla.gecko.AppConstants;
> +import org.mozilla.gecko.util.GeckoEventListener;
> +

extra blank line.

@@ +42,5 @@
> +    private static final String LOGTAG = "B2GDroid:NotificationObserver";
> +
> +    private static final String ACTION_REGISTER_FOR_NOTIFICATIONS = AppConstants.ANDROID_PACKAGE_NAME + ".ACTION_REGISTER_FOR_NOTIFICATIONS";
> +
> +    private Map<String, PendingIntent> mNotificationIntents;

Add a comment to describe what the key is.

@@ +105,5 @@
> +        } catch(NameNotFoundException ex) {
> +            Log.i(LOGTAG, "Could not find package name: " + ex.getMessage());
> +            return "";
> +        }
> +

extra blank line.

@@ +112,5 @@
> +
> +        int resourceId = notification.icon;
> +        Bitmap bitmap = BitmapFactory.decodeResource(notifyingPackageResources, resourceId);
> +
> +

extra blank line

::: mobile/android/b2gdroid/components/MessagesBridge.jsm
@@ +13,5 @@
> +Cu.import("resource://gre/modules/AppsUtils.jsm");
> +
> +XPCOMUtils.defineLazyServiceGetter(this, "appsService",
> +                                    "@mozilla.org/AppsService;1",
> +                                    "nsIAppsService");

nit: indent " at the same level as `this`

@@ +151,4 @@
>        Services.obs.removeObserver(this.onAndroidMessage, "Android:Launcher");
>        Services.obs.removeObserver(this.onAndroidSetting, "Android:Setting");
>        Services.obs.removeObserver(this.onSettingChange, "mozsettings-changed");
>        Services.obs.removeObserver(this, "xpcom-shutdown");

you need to remove your new observer here.

@@ +156,5 @@
>    }
>  }
>  
> +function showNotification(id, title, text, appIcon, manifestURL, timestamp,
> +                             behavior) {

nit: s/id/aId etc..

Comment 8

[sgiles]

Attachment: android-notifications-in-gaia.patch

Comment 9

[sgiles]

Comment on attachment 8682050 [details] [diff] [review]
android-notifications-in-gaia.patch

I've addressed most of the nits. There was a question on an import that was used. I've removed some more imports that were unused too.

I've also fixed the bug you came across I think.  It seems some notifications are created by services rather than an app and the Icon for these can't be found by referencing their resources.  It now uses a couple of fallbacks: first it looks in the `largeIcon` field of the Notification, and then falls back to an empty URL.

Comment 10

[[:fabrice] Fabrice Desré]

Attachment: logcat-download.txt

I still can't get notifications when for instance downloading a large file with fennec.

Comment 11

[sgiles]

Attachment: android-notifications-in-gaia-v2.patch

This adds a bit more than before:

It uses the flags from the notification to set some behaviours - so ongoing notifications don't re-notify.
When a notification is cancelled in Gaia or in Android it is reflected in both.
Also fixes a bug where FxOS notifications caused a failure because the NotificationHandler isn't initialised in the GeckoAppShell.

Comment 12

[sgiles]

Attachment: android-notifications-in-gaia-v2.patch

rebased on m-c (apply before bug 1202752)

Comment 13

[[:fabrice] Fabrice Desré]

Comment on attachment 8682767 [details] [diff] [review]
android-notifications-in-gaia-v2.patch

Review of attachment 8682767 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits fixed

::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/Launcher.java
@@ +136,5 @@
> +        // event, Notifications are handled in a different process as a
> +        // service, so we need to forward them to the remote service
> +        EventDispatcher
> +            .getInstance()
> +            .registerGeckoThreadListener(mGeckoEventProxy, "Android:NotificationOpened");

nit: indent like the previous EventDispatcher.getInstance()...

::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/NotificationObserver.java
@@ +79,5 @@
> +        try {
> +            notificationData.put("_action", "post");
> +            notificationData.put("id",    notificationKey);
> +            notificationData.put("title", title.toString());
> +            notificationData.put("text",  extraText.toString());

either pad all the put("foo",   bar) or none of them, but don't mix styles.

@@ +168,5 @@
> +            if (notification.largeIcon != null) {
> +                bitmap = notification.largeIcon;
> +            } else {
> +                Log.i(LOGTAG, "No image found for notification");
> +                return "";

Maybe we should return a default icon instead? That's fine to do that in a follow up.

::: mobile/android/b2gdroid/components/MessagesBridge.jsm
@@ +168,5 @@
>    }
>  }
>  
> +function removeNotification(aDetail) {
> +

nit: extra blank line

Comment 14

[sgiles]

Attachment: android-notifications-in-gaia-v2.patch

Obsoletes r+d attachment 8682135 [details] with nits fixed.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/515f1bf1eb54

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/515f1bf1eb54