Bug 1297836 - Get surface when creating or resuming compositor; r=snorp
authorJim Chen <nchen@mozilla.com>
Fri, 26 Aug 2016 12:25:57 -0400
changeset 311459 db1826bc2da5ebb8984eacb7a622b376a8b1598f
parent 311458 59fceff95c704ed43139bb0e31fd7b28d559ea52
child 311460 da87045db58056e2c495ec4df8da74fc1093640d
push id20400
push userkwierso@gmail.com
push dateFri, 26 Aug 2016 23:33:52 +0000
treeherderfx-team@3c4c4accb139 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1297836
milestone51.0a1
Bug 1297836 - Get surface when creating or resuming compositor; r=snorp Get rid of LayerView.Compositor.getSurface and just pass in the surface when creating or resuming the compositor. That also lets us get rid of some synchronization required for getSurface.
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerView.java
widget/android/GeneratedJNIWrappers.cpp
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
@@ -88,39 +88,29 @@ public class LayerView extends ScrollVie
                                                NativePanZoomController npzc);
 
         @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);
+        /* package */ native void createCompositor(int width, int height, Object surface);
 
         // 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 = "current")
-        /* package */ native void syncResumeResizeCompositor(int width, int height);
+        /* package */ native void syncResumeResizeCompositor(int width, int height, Object surface);
 
         @WrapForJNI(calledFrom = "any", dispatchTo = "current")
         /* package */ native void syncInvalidateAndScheduleComposite();
 
-        @WrapForJNI
-        private Object getSurface() {
-            synchronized (LayerView.this) {
-                if (LayerView.this.mServerSurfaceValid) {
-                    return LayerView.this.getSurface();
-                }
-            }
-            return null;
-        }
-
         @WrapForJNI(calledFrom = "gecko")
         private void reattach() {
             mCompositorCreated = true;
         }
 
         @WrapForJNI(calledFrom = "gecko")
         private void destroy() {
             // The nsWindow has been closed. First mark our compositor as destroyed.
@@ -454,21 +444,19 @@ public class LayerView extends ScrollVie
     @WrapForJNI(calledFrom = "ui")
     protected Object getCompositor() {
         return mCompositor;
     }
 
     void serverSurfaceChanged(int newWidth, int newHeight) {
         ThreadUtils.assertOnUiThread();
 
-        synchronized (this) {
-            mWidth = newWidth;
-            mHeight = newHeight;
-            mServerSurfaceValid = true;
-        }
+        mWidth = newWidth;
+        mHeight = newHeight;
+        mServerSurfaceValid = true;
 
         updateCompositor();
     }
 
     void updateCompositor() {
         ThreadUtils.assertOnUiThread();
 
         if (mCompositorCreated) {
@@ -479,26 +467,26 @@ public class LayerView extends ScrollVie
                 return;
             }
             // Asking Gecko to resume the compositor takes too long (see
             // https://bugzilla.mozilla.org/show_bug.cgi?id=735230#c23), so we
             // resume the compositor directly. We still need to inform Gecko about
             // the compositor resuming, so that Gecko knows that it can now draw.
             // It is important to not notify Gecko until after the compositor has
             // been resumed, otherwise Gecko may send updates that get dropped.
-            mCompositor.syncResumeResizeCompositor(mWidth, mHeight);
+            mCompositor.syncResumeResizeCompositor(mWidth, mHeight, getSurface());
             return;
         }
 
         // Only try to create the compositor if we have a valid surface and gecko is up. When these
         // two conditions are satisfied, we can be relatively sure that the compositor creation will
         // happen without needing to block anywhere.
         if (mServerSurfaceValid && getLayerClient().isGeckoReady()) {
             mCompositorCreated = true;
-            mCompositor.createCompositor(mWidth, mHeight);
+            mCompositor.createCompositor(mWidth, mHeight, getSurface());
         }
     }
 
     /* When using a SurfaceView (mSurfaceView != null), resizing happens in two
      * phases. First, the LayerView changes size, then, often some frames later,
      * the SurfaceView changes size. Because of this, we need to split the
      * resize into two phases to avoid jittering.
      *
@@ -517,17 +505,17 @@ public class LayerView extends ScrollVie
      */
     private void onSizeChanged(int width, int height) {
         if (!mServerSurfaceValid || mSurfaceView == null) {
             surfaceChanged(width, height);
             return;
         }
 
         if (mCompositorCreated) {
-            mCompositor.syncResumeResizeCompositor(width, height);
+            mCompositor.syncResumeResizeCompositor(width, height, getSurface());
         }
 
         if (mOverscroll != null) {
             mOverscroll.setSize(width, height);
         }
     }
 
     private void surfaceChanged(int width, int height) {
@@ -556,19 +544,17 @@ public class LayerView extends ScrollVie
         // serverSurfaceDestroyed notification), and to make sure that any in-flight
         // Gecko draw events have been processed.  When this returns, composition is
         // definitely paused -- it'll synchronize with the Gecko event loop, which
         // in turn will synchronize with the compositor thread.
         if (mCompositorCreated) {
             mCompositor.syncPauseCompositor();
         }
 
-        synchronized (this) {
-            mServerSurfaceValid = false;
-        }
+        mServerSurfaceValid = false;
     }
 
     private void onDestroyed() {
         serverSurfaceDestroyed();
     }
 
     public Object getNativeWindow() {
         if (mSurfaceView != null)
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -1436,24 +1436,16 @@ constexpr char LayerView::Compositor::De
 auto LayerView::Compositor::Destroy() const -> void
 {
     return mozilla::jni::Method<Destroy_t>::Call(Compositor::mCtx, nullptr);
 }
 
 constexpr char LayerView::Compositor::DisposeNative_t::name[];
 constexpr char LayerView::Compositor::DisposeNative_t::signature[];
 
-constexpr char LayerView::Compositor::GetSurface_t::name[];
-constexpr char LayerView::Compositor::GetSurface_t::signature[];
-
-auto LayerView::Compositor::GetSurface() const -> mozilla::jni::Object::LocalRef
-{
-    return mozilla::jni::Method<GetSurface_t>::Call(Compositor::mCtx, nullptr);
-}
-
 constexpr char LayerView::Compositor::OnSizeChanged_t::name[];
 constexpr char LayerView::Compositor::OnSizeChanged_t::signature[];
 
 constexpr char LayerView::Compositor::Reattach_t::name[];
 constexpr char LayerView::Compositor::Reattach_t::signature[];
 
 auto LayerView::Compositor::Reattach() const -> void
 {
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -4628,20 +4628,21 @@ public:
     };
 
     struct CreateCompositor_t {
         typedef Compositor Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<
                 int32_t,
-                int32_t> Args;
+                int32_t,
+                mozilla::jni::Object::Param> Args;
         static constexpr char name[] = "createCompositor";
         static constexpr char signature[] =
-                "(II)V";
+                "(IILjava/lang/Object;)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;
     };
@@ -4677,35 +4678,16 @@ public:
         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::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";
-        static constexpr char signature[] =
-                "()Ljava/lang/Object;";
-        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::CURRENT;
-    };
-
-    auto GetSurface() const -> mozilla::jni::Object::LocalRef;
-
     struct OnSizeChanged_t {
         typedef Compositor Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<
                 int32_t,
                 int32_t,
                 int32_t,
@@ -4776,20 +4758,21 @@ public:
     };
 
     struct SyncResumeResizeCompositor_t {
         typedef Compositor Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<
                 int32_t,
-                int32_t> Args;
+                int32_t,
+                mozilla::jni::Object::Param> Args;
         static constexpr char name[] = "syncResumeResizeCompositor";
         static constexpr char signature[] =
-                "(II)V";
+                "(IILjava/lang/Object;)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::CURRENT;
     };
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -1024,17 +1024,16 @@ public:
 
     bool CompositorPaused() const
     {
         return mCompositorPaused;
     }
 
     jni::Object::Param GetSurface()
     {
-        mSurface = mCompositor->GetSurface();
         return mSurface;
     }
 
 private:
     void OnResumedCompositor()
     {
         MOZ_ASSERT(NS_IsMainThread());
 
@@ -1100,22 +1099,25 @@ public:
 
         if (aWindowWidth != mWindow->mBounds.width ||
             aWindowHeight != mWindow->mBounds.height) {
 
             mWindow->Resize(aWindowWidth, aWindowHeight, /* repaint */ false);
         }
     }
 
-    void CreateCompositor(int32_t aWidth, int32_t aHeight)
+    void CreateCompositor(int32_t aWidth, int32_t aHeight,
+                          jni::Object::Param aSurface)
     {
         MOZ_ASSERT(NS_IsMainThread());
         MOZ_ASSERT(mWindow);
 
+        mSurface = aSurface;
         mWindow->CreateLayerManager(aWidth, aHeight);
+
         mCompositorPaused = false;
         OnResumedCompositor();
     }
 
     void SyncPauseCompositor()
     {
         MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
 
@@ -1142,26 +1144,29 @@ public:
         }
 
         if (bridge && bridge->ScheduleResumeOnCompositorThread()) {
             mCompositorPaused = false;
         }
     }
 
     void SyncResumeResizeCompositor(const LayerView::Compositor::LocalRef& aObj,
-                                    int32_t aWidth, int32_t aHeight)
+                                    int32_t aWidth, int32_t aHeight,
+                                    jni::Object::Param aSurface)
     {
         MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
 
         RefPtr<CompositorBridgeParent> bridge;
 
         if (LockedWindowPtr window{mWindow}) {
             bridge = window->GetCompositorBridgeParent();
         }
 
+        mSurface = aSurface;
+
         if (!bridge || !bridge->ScheduleResumeOnCompositorThread(aWidth,
                                                                  aHeight)) {
             return;
         }
 
         mCompositorPaused = false;
 
         class OnResumedEvent : public nsAppShell::Event