Bug 1346634 - Only notify push observers from xpcom queue; r=snorp a=lizzard
authorJim Chen <nchen@mozilla.com>
Mon, 13 Mar 2017 19:55:00 +0100
changeset 379198 2000266fbab6c7567940cb982b32e0dd3cd56c18
parent 379197 96801dce9f7506139346614116dd87c0264b2d88
child 379199 aad06be43cd864b4d170cd94210d174f1279aad9
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, lizzard
bugs1346634, 1344892
milestone53.0
Bug 1346634 - Only notify push observers from xpcom queue; r=snorp a=lizzard Until bug 1344892 is fixed, only notify push observers (and not all observers) from the XPCOM nsThread event queue, to avoid regressions such as session restore breaking.
mobile/android/base/java/org/mozilla/gecko/push/PushService.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
widget/android/GeneratedJNINatives.h
widget/android/GeneratedJNIWrappers.cpp
widget/android/GeneratedJNIWrappers.h
widget/android/nsAppShell.cpp
--- a/mobile/android/base/java/org/mozilla/gecko/push/PushService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/push/PushService.java
@@ -282,26 +282,24 @@ public class PushService implements Bund
             sendMessageToGeckoService(data);
         } else {
             sendMessageToDecodeToGeckoService(data);
         }
     }
 
     protected static void sendMessageToGeckoService(final @NonNull JSONObject message) {
         Log.i(LOG_TAG, "Delivering dom/push message to Gecko!");
-        GeckoAppShell.notifyObservers("PushServiceAndroidGCM:ReceivedPushMessage",
-                                      message.toString(),
-                                      GeckoThread.State.PROFILE_READY);
+        GeckoAppShell.notifyPushObservers("PushServiceAndroidGCM:ReceivedPushMessage",
+                                          message.toString());
     }
 
     protected static void sendMessageToDecodeToGeckoService(final @NonNull JSONObject message) {
         Log.i(LOG_TAG, "Delivering dom/push message to decode to Gecko!");
-        GeckoAppShell.notifyObservers("FxAccountsPush:ReceivedPushMessageToDecode",
-                                      message.toString(),
-                                      GeckoThread.State.PROFILE_READY);
+        GeckoAppShell.notifyPushObservers("FxAccountsPush:ReceivedPushMessageToDecode",
+                                          message.toString());
     }
 
     protected void registerGeckoEventListener() {
         Log.d(LOG_TAG, "Registered Gecko event listener.");
         EventDispatcher.getInstance().registerBackgroundThreadListener(this, GECKO_EVENTS);
     }
 
     protected void unregisterGeckoEventListener() {
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -285,17 +285,17 @@ public class GeckoAppShell
 
         notifyObservers(request.getName(), request.getData());
     }
 
     // Synchronously notify a Gecko observer; must be called from Gecko thread.
     @WrapForJNI(calledFrom = "gecko")
     public static native void syncNotifyObservers(String topic, String data);
 
-    @WrapForJNI(stubName = "NotifyObservers", dispatchTo = "proxy")
+    @WrapForJNI(stubName = "NotifyObservers", dispatchTo = "gecko")
     private static native void nativeNotifyObservers(String topic, String data);
 
     @RobocopTarget
     public static void notifyObservers(final String topic, final String data) {
         notifyObservers(topic, data, GeckoThread.State.RUNNING);
     }
 
     public static void notifyObservers(final String topic, final String data, final GeckoThread.State state) {
@@ -303,16 +303,29 @@ public class GeckoAppShell
             nativeNotifyObservers(topic, data);
         } else {
             GeckoThread.queueNativeCallUntil(
                     state, GeckoAppShell.class, "nativeNotifyObservers",
                     String.class, topic, String.class, data);
         }
     }
 
+    @WrapForJNI(stubName = "NotifyPushObservers", dispatchTo = "proxy")
+    private static native void nativeNotifyPushObservers(String topic, String data);
+
+    public static void notifyPushObservers(final String topic, final String data) {
+        if (GeckoThread.isStateAtLeast(GeckoThread.State.PROFILE_READY)) {
+            nativeNotifyPushObservers(topic, data);
+        } else {
+            GeckoThread.queueNativeCallUntil(
+                    GeckoThread.State.PROFILE_READY, GeckoAppShell.class,
+                    "nativeNotifyPushObservers", String.class, topic, String.class, data);
+        }
+    }
+
     /*
      *  The Gecko-side API: API methods that Gecko calls
      */
 
     @WrapForJNI(exceptionMode = "ignore")
     private static String getExceptionStackTrace(Throwable e) {
         return CrashHandler.getExceptionStackTrace(CrashHandler.getRootException(e));
     }
--- a/widget/android/GeneratedJNINatives.h
+++ b/widget/android/GeneratedJNINatives.h
@@ -96,26 +96,30 @@ const JNINativeMethod EventDispatcher::N
             mozilla::jni::NativeStub<EventDispatcher::NativeCallbackDelegate::SendSuccess_t, Impl>
             ::template Wrap<&Impl::SendSuccess>)
 };
 
 template<class Impl>
 class GeckoAppShell::Natives : public mozilla::jni::NativeImpl<GeckoAppShell, Impl>
 {
 public:
-    static const JNINativeMethod methods[8];
+    static const JNINativeMethod methods[9];
 };
 
 template<class Impl>
 const JNINativeMethod GeckoAppShell::Natives<Impl>::methods[] = {
 
     mozilla::jni::MakeNativeMethod<GeckoAppShell::NotifyObservers_t>(
             mozilla::jni::NativeStub<GeckoAppShell::NotifyObservers_t, Impl>
             ::template Wrap<&Impl::NotifyObservers>),
 
+    mozilla::jni::MakeNativeMethod<GeckoAppShell::NotifyPushObservers_t>(
+            mozilla::jni::NativeStub<GeckoAppShell::NotifyPushObservers_t, Impl>
+            ::template Wrap<&Impl::NotifyPushObservers>),
+
     mozilla::jni::MakeNativeMethod<GeckoAppShell::NotifyAlertListener_t>(
             mozilla::jni::NativeStub<GeckoAppShell::NotifyAlertListener_t, Impl>
             ::template Wrap<&Impl::NotifyAlertListener>),
 
     mozilla::jni::MakeNativeMethod<GeckoAppShell::NotifyUriVisited_t>(
             mozilla::jni::NativeStub<GeckoAppShell::NotifyUriVisited_t, Impl>
             ::template Wrap<&Impl::NotifyUriVisited>),
 
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -565,16 +565,19 @@ constexpr char GeckoAppShell::MoveTaskTo
 auto GeckoAppShell::MoveTaskToBack() -> void
 {
     return mozilla::jni::Method<MoveTaskToBack_t>::Call(GeckoAppShell::Context(), nullptr);
 }
 
 constexpr char GeckoAppShell::NotifyObservers_t::name[];
 constexpr char GeckoAppShell::NotifyObservers_t::signature[];
 
+constexpr char GeckoAppShell::NotifyPushObservers_t::name[];
+constexpr char GeckoAppShell::NotifyPushObservers_t::signature[];
+
 constexpr char GeckoAppShell::NotifyAlertListener_t::name[];
 constexpr char GeckoAppShell::NotifyAlertListener_t::signature[];
 
 constexpr char GeckoAppShell::NotifyUriVisited_t::name[];
 constexpr char GeckoAppShell::NotifyUriVisited_t::signature[];
 
 constexpr char GeckoAppShell::NotifyWakeLockChanged_t::name[];
 constexpr char GeckoAppShell::NotifyWakeLockChanged_t::signature[];
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -1536,16 +1536,35 @@ public:
         static constexpr char signature[] =
                 "(Ljava/lang/String;Ljava/lang/String;)V";
         static const bool isStatic = true;
         static const mozilla::jni::ExceptionMode exceptionMode =
                 mozilla::jni::ExceptionMode::ABORT;
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::ANY;
         static const mozilla::jni::DispatchTarget dispatchTarget =
+                mozilla::jni::DispatchTarget::GECKO;
+    };
+
+    struct NotifyPushObservers_t {
+        typedef GeckoAppShell Owner;
+        typedef void ReturnType;
+        typedef void SetterType;
+        typedef mozilla::jni::Args<
+                mozilla::jni::String::Param,
+                mozilla::jni::String::Param> Args;
+        static constexpr char name[] = "nativeNotifyPushObservers";
+        static constexpr char signature[] =
+                "(Ljava/lang/String;Ljava/lang/String;)V";
+        static const bool isStatic = true;
+        static const mozilla::jni::ExceptionMode exceptionMode =
+                mozilla::jni::ExceptionMode::ABORT;
+        static const mozilla::jni::CallingThread callingThread =
+                mozilla::jni::CallingThread::ANY;
+        static const mozilla::jni::DispatchTarget dispatchTarget =
                 mozilla::jni::DispatchTarget::PROXY;
     };
 
     struct NotifyAlertListener_t {
         typedef GeckoAppShell Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -254,20 +254,26 @@ public:
     {
         MOZ_RELEASE_ASSERT(NS_IsMainThread());
         NotifyObservers(aTopic, aData);
     }
 
     template<typename Functor>
     static void OnNativeCall(Functor&& aCall)
     {
-        MOZ_ASSERT(aCall.IsTarget(&NotifyObservers));
+        MOZ_ASSERT(aCall.IsTarget(&NotifyPushObservers));
         NS_DispatchToMainThread(NS_NewRunnableFunction(mozilla::Move(aCall)));
     }
 
+    static void NotifyPushObservers(jni::String::Param aTopic,
+                                    jni::String::Param aData)
+    {
+        NotifyObservers(aTopic, aData);
+    }
+
     static void NotifyObservers(jni::String::Param aTopic,
                                 jni::String::Param aData)
     {
         MOZ_ASSERT(NS_IsMainThread());
         MOZ_ASSERT(aTopic);
 
         nsCOMPtr<nsIObserverService> obsServ = services::GetObserverService();
         if (!obsServ) {