Bug 1297836 - Don't use WindowEvent for LayerViewSupport; r=snorp
authorJim Chen <nchen@mozilla.com>
Fri, 26 Aug 2016 12:25:57 -0400
changeset 311477 59fceff95c704ed43139bb0e31fd7b28d559ea52
parent 311476 1900eaf63f0b556b4b353e40afc420ed3019981b
child 311478 db1826bc2da5ebb8984eacb7a622b376a8b1598f
push id30610
push userkwierso@gmail.com
push dateFri, 26 Aug 2016 23:20:56 +0000
treeherdermozilla-central@f38d60b049f0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1297836
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1297836 - Don't use WindowEvent for LayerViewSupport; r=snorp Now that nsWindow doesn't own LayerViewSupport, we shouldn't be using WindowEvent for LayerViewSupport calls. This patch converts the calls that dispatch to proxy to dispatch directly to Gecko. For SyncResumeResizeCompositor, it used a proxy to call OnResumedCompositor on the Gecko thread; this patch makes SyncResumeResizeCompositor post an event to call OnResumedCompositor directly, without going through the proxy.
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerView.java
widget/android/GeneratedJNIWrappers.h
widget/android/nsWindow.cpp
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerView.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerView.java
@@ -74,38 +74,38 @@ public class LayerView extends ScrollVie
     /* This is written by the Gecko thread and the UI thread, and read by the UI thread. */
     @WrapForJNI(stubName = "CompositorCreated", calledFrom = "ui")
     /* package */ volatile boolean mCompositorCreated;
 
     private class Compositor extends JNIObject {
         public Compositor() {
         }
 
-        @WrapForJNI(calledFrom = "ui", dispatchTo = "proxy")
+        @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
         @Override protected native void disposeNative();
 
         // Gecko thread sets its Java instances; does not block UI thread.
-        @WrapForJNI(calledFrom = "any", dispatchTo = "proxy")
+        @WrapForJNI(calledFrom = "any", dispatchTo = "gecko")
         /* package */ native void attachToJava(GeckoLayerClient layerClient,
                                                NativePanZoomController npzc);
 
-        @WrapForJNI(calledFrom = "any", dispatchTo = "proxy")
+        @WrapForJNI(calledFrom = "any", dispatchTo = "gecko")
         /* package */ native void onSizeChanged(int windowWidth, int windowHeight,
                                                 int screenWidth, int screenHeight);
 
         // Gecko thread creates compositor; blocks UI thread.
         @WrapForJNI(calledFrom = "ui", dispatchTo = "proxy")
         /* package */ native void createCompositor(int width, int height);
 
         // Gecko thread pauses compositor; blocks UI thread.
         @WrapForJNI(calledFrom = "ui", dispatchTo = "current")
         /* package */ native void syncPauseCompositor();
 
         // UI thread resumes compositor and notifies Gecko thread; does not block UI thread.
-        @WrapForJNI(calledFrom = "ui", dispatchTo = "proxy")
+        @WrapForJNI(calledFrom = "ui", dispatchTo = "current")
         /* package */ native void syncResumeResizeCompositor(int width, int height);
 
         @WrapForJNI(calledFrom = "any", dispatchTo = "current")
         /* package */ native void syncInvalidateAndScheduleComposite();
 
         @WrapForJNI
         private Object getSurface() {
             synchronized (LayerView.this) {
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -4619,17 +4619,17 @@ public:
         static constexpr char signature[] =
                 "(Lorg/mozilla/gecko/gfx/GeckoLayerClient;Lorg/mozilla/gecko/gfx/NativePanZoomController;)V";
         static const bool isStatic = false;
         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;
+                mozilla::jni::DispatchTarget::GECKO;
     };
 
     struct CreateCompositor_t {
         typedef Compositor Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<
                 int32_t,
@@ -4674,17 +4674,17 @@ public:
         static constexpr char signature[] =
                 "()V";
         static const bool isStatic = false;
         static const mozilla::jni::ExceptionMode exceptionMode =
                 mozilla::jni::ExceptionMode::ABORT;
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::UI;
         static const mozilla::jni::DispatchTarget dispatchTarget =
-                mozilla::jni::DispatchTarget::PROXY;
+                mozilla::jni::DispatchTarget::GECKO;
     };
 
     struct GetSurface_t {
         typedef Compositor Owner;
         typedef mozilla::jni::Object::LocalRef ReturnType;
         typedef mozilla::jni::Object::Param SetterType;
         typedef mozilla::jni::Args<> Args;
         static constexpr char name[] = "getSurface";
@@ -4714,17 +4714,17 @@ public:
         static constexpr char signature[] =
                 "(IIII)V";
         static const bool isStatic = false;
         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;
+                mozilla::jni::DispatchTarget::GECKO;
     };
 
     struct Reattach_t {
         typedef Compositor Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<> Args;
         static constexpr char name[] = "reattach";
@@ -4786,17 +4786,17 @@ public:
         static constexpr char signature[] =
                 "(II)V";
         static const bool isStatic = false;
         static const mozilla::jni::ExceptionMode exceptionMode =
                 mozilla::jni::ExceptionMode::ABORT;
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::UI;
         static const mozilla::jni::DispatchTarget dispatchTarget =
-                mozilla::jni::DispatchTarget::PROXY;
+                mozilla::jni::DispatchTarget::CURRENT;
     };
 
     static const mozilla::jni::CallingThread callingThread =
             mozilla::jni::CallingThread::ANY;
 
     template<class Impl> class Natives;
 };
 
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -982,42 +982,20 @@ class nsWindow::LayerViewSupport final
 public:
     typedef LayerView::Compositor::Natives<LayerViewSupport> Base;
 
     template<class Functor>
     static void OnNativeCall(Functor&& aCall)
     {
         if (aCall.IsTarget(&LayerViewSupport::CreateCompositor)) {
             // This call is blocking.
-            nsAppShell::SyncRunEvent(WindowEvent<Functor>(
+            nsAppShell::SyncRunEvent(nsAppShell::LambdaEvent<Functor>(
                     mozilla::Move(aCall)), &LayerViewEvent::MakeEvent);
             return;
-
-        } else if (aCall.IsTarget(
-                &LayerViewSupport::SyncResumeResizeCompositor)) {
-            // This call is synchronous. Perform the original call using a copy
-            // of the lambda. Then redirect the original lambda to
-            // OnResumedCompositor, to be run on the Gecko thread. We use
-            // Functor instead of our own lambda so that Functor can handle
-            // object lifetimes for us.
-            (Functor(aCall))();
-            aCall.SetTarget(&LayerViewSupport::OnResumedCompositor);
-            nsAppShell::PostEvent(
-                    mozilla::MakeUnique<LayerViewEvent>(
-                    mozilla::MakeUnique<WindowEvent<Functor>>(
-                    mozilla::Move(aCall))));
-            return;
         }
-
-        // LayerViewEvent (i.e. prioritized event) applies to
-        // CreateCompositor, PauseCompositor, and OnResumedCompositor. For all
-        // other events, use regular WindowEvent.
-        nsAppShell::PostEvent(
-                mozilla::MakeUnique<WindowEvent<Functor>>(
-                mozilla::Move(aCall)));
     }
 
     static LayerViewSupport*
     FromNative(const LayerView::Compositor::LocalRef& instance)
     {
         return GetNative(instance);
     }
 
@@ -1051,17 +1029,17 @@ public:
 
     jni::Object::Param GetSurface()
     {
         mSurface = mCompositor->GetSurface();
         return mSurface;
     }
 
 private:
-    void OnResumedCompositor(int32_t aWidth, int32_t aHeight)
+    void OnResumedCompositor()
     {
         MOZ_ASSERT(NS_IsMainThread());
 
         // When we receive this, the compositor has already been told to
         // resume. (It turns out that waiting till we reach here to tell
         // the compositor to resume takes too long, resulting in a black
         // flash.) This means it's now safe for layer updates to occur.
         // Since we might have prevented one or more draw events from
@@ -1074,16 +1052,19 @@ private:
 
     /**
      * Compositor methods
      */
 public:
     void AttachToJava(jni::Object::Param aClient, jni::Object::Param aNPZC)
     {
         MOZ_ASSERT(NS_IsMainThread());
+        if (!mWindow) {
+            return; // Already shut down.
+        }
 
         const auto& layerClient = GeckoLayerClient::Ref::From(aClient);
 
         AndroidBridge::Bridge()->SetLayerClient(layerClient);
 
         // If resetting is true, Android destroyed our GeckoApp activity and we
         // had to recreate it, but all the Gecko-side things were not
         // destroyed.  We therefore need to link up the new java objects to
@@ -1108,31 +1089,35 @@ public:
             }
         }
     }
 
     void OnSizeChanged(int32_t aWindowWidth, int32_t aWindowHeight,
                        int32_t aScreenWidth, int32_t aScreenHeight)
     {
         MOZ_ASSERT(NS_IsMainThread());
+        if (!mWindow) {
+            return; // Already shut down.
+        }
 
         if (aWindowWidth != mWindow->mBounds.width ||
             aWindowHeight != mWindow->mBounds.height) {
 
             mWindow->Resize(aWindowWidth, aWindowHeight, /* repaint */ false);
         }
     }
 
     void CreateCompositor(int32_t aWidth, int32_t aHeight)
     {
         MOZ_ASSERT(NS_IsMainThread());
+        MOZ_ASSERT(mWindow);
 
         mWindow->CreateLayerManager(aWidth, aHeight);
         mCompositorPaused = false;
-        OnResumedCompositor(aWidth, aHeight);
+        OnResumedCompositor();
     }
 
     void SyncPauseCompositor()
     {
         MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
 
         RefPtr<CompositorBridgeParent> bridge;
 
@@ -1156,30 +1141,58 @@ public:
             bridge = window->GetCompositorBridgeParent();
         }
 
         if (bridge && bridge->ScheduleResumeOnCompositorThread()) {
             mCompositorPaused = false;
         }
     }
 
-    void SyncResumeResizeCompositor(int32_t aWidth, int32_t aHeight)
+    void SyncResumeResizeCompositor(const LayerView::Compositor::LocalRef& aObj,
+                                    int32_t aWidth, int32_t aHeight)
     {
         MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
 
         RefPtr<CompositorBridgeParent> bridge;
 
         if (LockedWindowPtr window{mWindow}) {
             bridge = window->GetCompositorBridgeParent();
         }
 
-        if (bridge && bridge->ScheduleResumeOnCompositorThread(aWidth,
-                                                               aHeight)) {
-            mCompositorPaused = false;
+        if (!bridge || !bridge->ScheduleResumeOnCompositorThread(aWidth,
+                                                                 aHeight)) {
+            return;
         }
+
+        mCompositorPaused = false;
+
+        class OnResumedEvent : public nsAppShell::Event
+        {
+            LayerView::Compositor::GlobalRef mCompositor;
+
+        public:
+            OnResumedEvent(LayerView::Compositor::GlobalRef&& aCompositor)
+                : mCompositor(mozilla::Move(aCompositor))
+            {}
+
+            void Run() override
+            {
+                MOZ_ASSERT(NS_IsMainThread());
+
+                JNIEnv* const env = jni::GetGeckoThreadEnv();
+                LayerViewSupport* const lvs = GetNative(
+                        LayerView::Compositor::LocalRef(env, mCompositor));
+                MOZ_CATCH_JNI_EXCEPTION(env);
+
+                lvs->OnResumedCompositor();
+            }
+        };
+
+        nsAppShell::PostEvent(MakeUnique<LayerViewEvent>(
+                MakeUnique<OnResumedEvent>(aObj)));
     }
 
     void SyncInvalidateAndScheduleComposite()
     {
         RefPtr<CompositorBridgeParent> bridge;
 
         if (LockedWindowPtr window{mWindow}) {
             bridge = window->GetCompositorBridgeParent();