Bug 1453501 - Immediately create Compositor on Android r=jchen,rbarker draft
authorJames Willcox <snorp@snorp.net>
Wed, 25 Apr 2018 15:04:14 -0500
changeset 790778 102d62b895db1b41c85c1c14270e567fbb799e01
parent 790777 8651412da8b0b6890280d167f323d332c85c0d79
child 790779 22c1fdc56e895fd595cf0e5b44a80cbf0141ca9b
push id108584
push userbmo:snorp@snorp.net
push dateWed, 02 May 2018 19:07:44 +0000
reviewersjchen, rbarker
bugs1453501
milestone61.0a1
Bug 1453501 - Immediately create Compositor on Android r=jchen,rbarker Currently we don't create the Compositor until we have a valid surface to render into. This causes a race that can result in us not being able to display anything at all once a surface is provided and the compositor is started. It seems the easiest thing to do right now is to avoid the race by starting the Compositor immediately. MozReview-Commit-ID: HkdVL3LBNZB
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
widget/android/GeneratedJNINatives.h
widget/android/GeneratedJNIWrappers.cpp
widget/android/GeneratedJNIWrappers.h
widget/android/nsWindow.cpp
widget/android/nsWindow.h
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
@@ -77,20 +77,16 @@ public class LayerSession {
         @Override protected native void disposeNative();
 
         @WrapForJNI(calledFrom = "any", dispatchTo = "gecko")
         public native void attachNPZC(PanZoomController npzc);
 
         @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
         public native void onBoundsChanged(int left, int top, int width, int height);
 
-        // Gecko thread creates compositor; blocks UI thread.
-        @WrapForJNI(calledFrom = "ui", dispatchTo = "proxy")
-        public native void createCompositor(int width, int height, Object surface);
-
         // Gecko thread pauses compositor; blocks UI thread.
         @WrapForJNI(calledFrom = "ui", dispatchTo = "current")
         public native void syncPauseCompositor();
 
         // UI thread resumes compositor and notifies Gecko thread; does not block UI thread.
         @WrapForJNI(calledFrom = "ui", dispatchTo = "current")
         public native void syncResumeResizeCompositor(int width, int height, Object surface);
 
@@ -158,17 +154,16 @@ public class LayerSession {
     // All fields are accessed on UI thread only.
     private final GeckoDisplay mDisplay = new GeckoDisplay(this);
     private PanZoomController mNPZC;
     private OverscrollEdgeEffect mOverscroll;
     private DynamicToolbarAnimator mToolbar;
     private CompositorController mController;
 
     private boolean mAttachedCompositor;
-    private boolean mCalledCreateCompositor;
     private boolean mCompositorReady;
     private Surface mSurface;
 
     // All fields of coordinates are in screen units.
     private int mLeft;
     private int mTop; // Top of the surface (including toolbar);
     private int mClientTop; // Top of the client area (i.e. excluding toolbar);
     private int mWidth;
@@ -356,29 +351,30 @@ public class LayerSession {
             mCompositor.attachNPZC(mNPZC);
         }
 
         if (mSurface != null) {
             // If we have a valid surface, create the compositor now that we're attached.
             // Leave mSurface alone because we'll need it later for onCompositorReady.
             onSurfaceChanged(mSurface, mWidth, mHeight);
         }
+
+        mCompositor.sendToolbarAnimatorMessage(IS_COMPOSITOR_CONTROLLER_OPEN);
     }
 
     /* package */ void onCompositorDetached() {
         if (DEBUG) {
             ThreadUtils.assertOnUiThread();
         }
 
         if (mController != null) {
             mController.onCompositorDetached();
         }
 
         mAttachedCompositor = false;
-        mCalledCreateCompositor = false;
         mCompositorReady = false;
     }
 
     /* package */ void handleCompositorMessage(final int message) {
         if (DEBUG) {
             ThreadUtils.assertOnUiThread();
         }
 
@@ -547,22 +543,16 @@ public class LayerSession {
         mHeight = height;
 
         if (mCompositorReady) {
             mCompositor.syncResumeResizeCompositor(width, height, surface);
             onWindowBoundsChanged();
             return;
         }
 
-        if (mAttachedCompositor && !mCalledCreateCompositor) {
-            mCompositor.createCompositor(width, height, surface);
-            mCompositor.sendToolbarAnimatorMessage(IS_COMPOSITOR_CONTROLLER_OPEN);
-            mCalledCreateCompositor = true;
-        }
-
         // We have a valid surface but we're not attached or the compositor
         // is not ready; save the surface for later when we're ready.
         mSurface = surface;
 
         // Adjust bounds as the last step.
         onWindowBoundsChanged();
     }
 
--- a/widget/android/GeneratedJNINatives.h
+++ b/widget/android/GeneratedJNINatives.h
@@ -383,30 +383,26 @@ const JNINativeMethod SurfaceTextureList
             mozilla::jni::NativeStub<SurfaceTextureListener::OnFrameAvailable_t, Impl>
             ::template Wrap<&Impl::OnFrameAvailable>)
 };
 
 template<class Impl>
 class LayerSession::Compositor::Natives : public mozilla::jni::NativeImpl<Compositor, Impl>
 {
 public:
-    static const JNINativeMethod methods[13];
+    static const JNINativeMethod methods[12];
 };
 
 template<class Impl>
 const JNINativeMethod LayerSession::Compositor::Natives<Impl>::methods[] = {
 
     mozilla::jni::MakeNativeMethod<LayerSession::Compositor::AttachNPZC_t>(
             mozilla::jni::NativeStub<LayerSession::Compositor::AttachNPZC_t, Impl>
             ::template Wrap<&Impl::AttachNPZC>),
 
-    mozilla::jni::MakeNativeMethod<LayerSession::Compositor::CreateCompositor_t>(
-            mozilla::jni::NativeStub<LayerSession::Compositor::CreateCompositor_t, Impl>
-            ::template Wrap<&Impl::CreateCompositor>),
-
     mozilla::jni::MakeNativeMethod<LayerSession::Compositor::DisposeNative_t>(
             mozilla::jni::NativeStub<LayerSession::Compositor::DisposeNative_t, Impl>
             ::template Wrap<&Impl::DisposeNative>),
 
     mozilla::jni::MakeNativeMethod<LayerSession::Compositor::EnableLayerUpdateNotifications_t>(
             mozilla::jni::NativeStub<LayerSession::Compositor::EnableLayerUpdateNotifications_t, Impl>
             ::template Wrap<&Impl::EnableLayerUpdateNotifications>),
 
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -1243,19 +1243,16 @@ auto LayerSession::GetCompositor() const
 }
 
 const char LayerSession::Compositor::name[] =
         "org/mozilla/gecko/gfx/LayerSession$Compositor";
 
 constexpr char LayerSession::Compositor::AttachNPZC_t::name[];
 constexpr char LayerSession::Compositor::AttachNPZC_t::signature[];
 
-constexpr char LayerSession::Compositor::CreateCompositor_t::name[];
-constexpr char LayerSession::Compositor::CreateCompositor_t::signature[];
-
 constexpr char LayerSession::Compositor::DisposeNative_t::name[];
 constexpr char LayerSession::Compositor::DisposeNative_t::signature[];
 
 constexpr char LayerSession::Compositor::EnableLayerUpdateNotifications_t::name[];
 constexpr char LayerSession::Compositor::EnableLayerUpdateNotifications_t::signature[];
 
 constexpr char LayerSession::Compositor::OnBoundsChanged_t::name[];
 constexpr char LayerSession::Compositor::OnBoundsChanged_t::signature[];
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -3773,36 +3773,16 @@ public:
         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 CreateCompositor_t {
-        typedef Compositor Owner;
-        typedef void ReturnType;
-        typedef void SetterType;
-        typedef mozilla::jni::Args<
-                int32_t,
-                int32_t,
-                mozilla::jni::Object::Param> Args;
-        static constexpr char name[] = "createCompositor";
-        static constexpr char signature[] =
-                "(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;
-    };
-
     struct DisposeNative_t {
         typedef Compositor Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<> Args;
         static constexpr char name[] = "disposeNative";
         static constexpr char signature[] =
                 "()V";
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -820,29 +820,16 @@ class nsWindow::LayerViewSupport final
                 queue.insertBack(this);
             }
         }
     };
 
 public:
     typedef LayerSession::Compositor::Natives<LayerViewSupport> Base;
 
-    template<class Functor>
-    static void OnNativeCall(Functor&& aCall)
-    {
-        if (aCall.IsTarget(&LayerViewSupport::CreateCompositor)) {
-            // This call is blocking.
-            nsAppShell::SyncRunEvent(nsAppShell::LambdaEvent<Functor>(
-                    mozilla::Move(aCall)), &LayerViewEvent::MakeEvent);
-            return;
-        }
-
-        MOZ_CRASH("Unexpected call");
-    }
-
     static LayerViewSupport*
     FromNative(const LayerSession::Compositor::LocalRef& instance)
     {
         return GetNative(instance);
     }
 
     LayerViewSupport(NativePtr<LayerViewSupport>* aPtr, nsWindow* aWindow,
                      const LayerSession::Compositor::LocalRef& aInstance)
@@ -927,30 +914,16 @@ public:
         MOZ_ASSERT(NS_IsMainThread());
         if (!mWindow) {
             return; // Already shut down.
         }
 
         mWindow->Resize(aLeft, aTop, aWidth, aHeight, /* repaint */ false);
     }
 
-    void CreateCompositor(int32_t aWidth, int32_t aHeight,
-                          jni::Object::Param aSurface)
-    {
-        MOZ_ASSERT(NS_IsMainThread());
-        if (!mWindow) {
-            return; // Already shut down.
-        }
-
-        mSurface = aSurface;
-        mWindow->CreateLayerManager(aWidth, aHeight);
-
-        mCompositorPaused = false;
-    }
-
     void SyncPauseCompositor()
     {
         MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
 
         if (RefPtr<UiCompositorControllerChild> child = GetUiCompositorControllerChild()) {
           mCompositorPaused = true;
           child->Pause();
         }
@@ -1416,16 +1389,18 @@ nsWindow::Create(nsIWidget* aParent,
     if (IsTopLevel()) {
         gTopLevelWindows.AppendElement(this);
 
     } else if (parent) {
         parent->mChildren.AppendElement(this);
         mParent = parent;
     }
 
+    CreateLayerManager();
+
 #ifdef DEBUG_ANDROID_WIDGET
     DumpWindows();
 #endif
 
     return NS_OK;
 }
 
 void
@@ -1852,33 +1827,34 @@ nsWindow::GetLayerManager(PLayerTransact
 {
     if (mLayerManager) {
         return mLayerManager;
     }
     return nullptr;
 }
 
 void
-nsWindow::CreateLayerManager(int aCompositorWidth, int aCompositorHeight)
+nsWindow::CreateLayerManager()
 {
     if (mLayerManager) {
         return;
     }
 
     nsWindow *topLevelWindow = FindTopLevel();
     if (!topLevelWindow || topLevelWindow->mWindowType == eWindowType_invisible) {
         // don't create a layer manager for an invisible top-level window
         return;
     }
 
     // Ensure that gfxPlatform is initialized first.
     gfxPlatform::GetPlatform();
 
     if (ShouldUseOffMainThreadCompositing()) {
-        CreateCompositor(aCompositorWidth, aCompositorHeight);
+        LayoutDeviceIntRect rect = GetBounds();
+        CreateCompositor(rect.Width(), rect.Height());
         if (mLayerManager) {
             return;
         }
 
         // If we get here, then off main thread compositing failed to initialize.
         sFailedToCreateGLContext = true;
     }
 
--- a/widget/android/nsWindow.h
+++ b/widget/android/nsWindow.h
@@ -336,17 +336,17 @@ protected:
       return true;
     }
 
     static void DumpWindows();
     static void DumpWindows(const nsTArray<nsWindow*>& wins, int indent = 0);
     static void LogWindow(nsWindow *win, int index, int indent);
 
 private:
-    void CreateLayerManager(int aCompositorWidth, int aCompositorHeight);
+    void CreateLayerManager();
     void RedrawAll();
 
     mozilla::layers::LayersId GetRootLayerId() const;
     RefPtr<mozilla::layers::UiCompositorControllerChild> GetUiCompositorControllerChild();
 };
 
 // Explicit template declarations to make clang be quiet.
 template<> const char nsWindow::NativePtr<nsWindow::LayerViewSupport>::sName[];