Bug 1359618 - Prevent LayerView from accessing the compositor until UiCompositorControllerChild is open r=jchen
MozReview-Commit-ID: 3SmdJx0QXW8
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/DynamicToolbarAnimator.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/DynamicToolbarAnimator.java
@@ -48,17 +48,16 @@ public class DynamicToolbarAnimator {
private final Set<PinReason> mPinFlags = Collections.synchronizedSet(EnumSet.noneOf(PinReason.class));
private final GeckoLayerClient mTarget;
private LayerView.Compositor mCompositor;
private final List<MetricsListener> mListeners;
private ToolbarChromeProxy mToolbarChromeProxy;
private int mMaxToolbarHeight;
- private boolean mCompositorControllerOpen;
public DynamicToolbarAnimator(GeckoLayerClient aTarget) {
mTarget = aTarget;
mListeners = new ArrayList<MetricsListener>();
}
public void addMetricsListener(MetricsListener aListener) {
mListeners.add(aListener);
@@ -82,17 +81,17 @@ public class DynamicToolbarAnimator {
for (MetricsListener listener : mListeners) {
listener.onMetricsChanged(aMetrics);
}
}
public void setMaxToolbarHeight(int maxToolbarHeight) {
ThreadUtils.assertOnUiThread();
mMaxToolbarHeight = maxToolbarHeight;
- if (mCompositor != null) {
+ if (isCompositorReady()) {
mCompositor.setMaxToolbarHeight(mMaxToolbarHeight);
}
}
public int getCurrentToolbarHeight() {
if ((mToolbarChromeProxy != null) && mToolbarChromeProxy.isToolbarChromeVisible()) {
return mMaxToolbarHeight;
}
@@ -105,37 +104,53 @@ public class DynamicToolbarAnimator {
public boolean isPinned() {
return !mPinFlags.isEmpty();
}
public boolean isPinnedBy(PinReason reason) {
return mPinFlags.contains(reason);
}
- public void setPinned(boolean pinned, PinReason reason) {
- if ((mCompositor != null) && (pinned != mPinFlags.contains(reason))) {
+ /* package */ void sendPinValueToCompositor(final boolean pinned, final PinReason reason) {
+ if (isCompositorReady()) {
mCompositor.setPinned(pinned, reason.value);
}
+ }
+
+ public void setPinned(final boolean pinned, final PinReason reason) {
+ // setPinned may be called from the main thread but compositor can only be accessed on UI thread
+ if (pinned != mPinFlags.contains(reason)) {
+ if (ThreadUtils.isOnUiThread() == true) {
+ sendPinValueToCompositor(pinned, reason);
+ } else {
+ ThreadUtils.postToUiThread(new Runnable() {
+ @Override
+ public void run() {
+ sendPinValueToCompositor(pinned, reason);
+ }
+ });
+ }
+ }
if (pinned) {
mPinFlags.add(reason);
} else {
mPinFlags.remove(reason);
}
}
public void showToolbar(boolean immediately) {
- if (mCompositor != null) {
+ if (isCompositorReady()) {
mCompositor.sendToolbarAnimatorMessage(immediately ?
LayerView.REQUEST_SHOW_TOOLBAR_IMMEDIATELY : LayerView.REQUEST_SHOW_TOOLBAR_ANIMATED);
}
}
public void hideToolbar(boolean immediately) {
- if (mCompositor != null) {
+ if (isCompositorReady()) {
mCompositor.sendToolbarAnimatorMessage(immediately ?
LayerView.REQUEST_HIDE_TOOLBAR_IMMEDIATELY : LayerView.REQUEST_HIDE_TOOLBAR_ANIMATED);
}
}
/* package-private */ IntSize getViewportSize() {
ThreadUtils.assertOnUiThread();
@@ -147,48 +162,37 @@ public class DynamicToolbarAnimator {
return new IntSize(viewWidth, viewHeight);
}
public PointF getVisibleEndOfLayerView() {
return new PointF(mTarget.getView().getWidth(),
mTarget.getView().getHeight());
}
- private void dumpStateToCompositor() {
- if ((mCompositor != null) && mCompositorControllerOpen) {
+ /* package-private */ void updateCompositor() {
+ if (isCompositorReady()) {
mCompositor.setMaxToolbarHeight(mMaxToolbarHeight);
if ((mToolbarChromeProxy != null) && mToolbarChromeProxy.isToolbarChromeVisible()) {
mCompositor.sendToolbarAnimatorMessage(LayerView.REQUEST_SHOW_TOOLBAR_IMMEDIATELY);
} else {
mCompositor.sendToolbarAnimatorMessage(LayerView.REQUEST_HIDE_TOOLBAR_IMMEDIATELY);
}
for (PinReason reason : PinReason.values()) {
- mCompositor.setPinned(mPinFlags.contains(reason), reason.value);
+ mCompositor.setPinned(mPinFlags.contains(reason), reason.value);
}
- } else if ((mCompositor != null) && !mCompositorControllerOpen) {
- // Ask the UiCompositorControllerChild if it is open since the open message can
- // sometimes be sent to a different instance of the LayerView such as when
- // Fennec is being used in custom tabs.
- mCompositor.sendToolbarAnimatorMessage(LayerView.IS_COMPOSITOR_CONTROLLER_OPEN);
}
}
/* package-private */ void notifyCompositorCreated(LayerView.Compositor aCompositor) {
ThreadUtils.assertOnUiThread();
mCompositor = aCompositor;
- dumpStateToCompositor();
}
- /* package-private */ void notifyCompositorControllerOpen() {
- mCompositorControllerOpen = true;
- dumpStateToCompositor();
- }
-
- /* package-private */ void notifyCompositorDestroyed() {
- mCompositor = null;
+ private boolean isCompositorReady() {
+ return ((mCompositor != null) && (mCompositor.isReady()));
}
/* package-private */ Bitmap getBitmapOfToolbarChrome() {
if (mToolbarChromeProxy != null) {
return mToolbarChromeProxy.getBitmapOfToolbarChrome();
}
return null;
--- 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
@@ -66,18 +66,18 @@ public class LayerView extends FrameLayo
private int mWidth, mHeight;
private boolean onAttachedToWindowCalled;
private int mDefaultClearColor = Color.WHITE;
/* package */ GetPixelsResult mGetPixelsResult;
private final List<DrawListener> mDrawListeners;
/* 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;
+ /* package */ volatile boolean mCompositorControllerOpen;
//
// NOTE: These values are also defined in gfx/layers/ipc/UiCompositorControllerMessageTypes.h
// and must be kept in sync. Any new AnimatorMessageType added here must also be added there.
//
/* package */ final static int STATIC_TOOLBAR_NEEDS_UPDATE = 0; // Sent from compositor when the static toolbar wants to hide.
/* package */ final static int STATIC_TOOLBAR_READY = 1; // Sent from compositor when the static toolbar image has been updated and is ready to animate.
/* package */ final static int TOOLBAR_HIDDEN = 2; // Sent to compositor when the real toolbar has been hidden.
@@ -97,20 +97,29 @@ public class LayerView extends FrameLayo
ThreadUtils.postToUiThread(new Runnable() {
@Override
public void run() {
mCompositor.sendToolbarAnimatorMessage(message);
}
});
}
+ @WrapForJNI(calledFrom = "ui")
+ /* package */ boolean isCompositorReady() {
+ ThreadUtils.assertOnUiThread();
+ return mCompositorCreated && mCompositorControllerOpen;
+ }
+
/* package */ class Compositor extends JNIObject {
public Compositor() {
}
+ /* package */ boolean isReady() {
+ return isCompositorReady();
+ }
@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 = "gecko")
/* package */ native void attachToJava(GeckoLayerClient layerClient,
NativePanZoomController npzc);
@@ -164,42 +173,36 @@ public class LayerView extends FrameLayo
@WrapForJNI(calledFrom = "ui", dispatchTo = "current")
/* package */ native void enableLayerUpdateNotifications(boolean enable);
@WrapForJNI(calledFrom = "ui", dispatchTo = "current")
/* package */ native void sendToolbarPixelsToCompositor(final int width, final int height, final int[] pixels);
@WrapForJNI(calledFrom = "gecko")
private void reattach() {
- mCompositorCreated = true;
ThreadUtils.postToUiThread(new Runnable() {
@Override
public void run() {
- // Make sure it is still valid
- if (mCompositorCreated) {
- mCompositor.setDefaultClearColor(mDefaultClearColor);
- mCompositor.enableLayerUpdateNotifications(!mDrawListeners.isEmpty());
- mToolbarAnimator.notifyCompositorCreated(mCompositor);
- }
+ updateCompositor();
}
});
}
@WrapForJNI(calledFrom = "gecko")
private void destroy() {
// The nsWindow has been closed. First mark our compositor as destroyed.
LayerView.this.mCompositorCreated = false;
+ LayerView.this.mCompositorControllerOpen = false;
LayerView.this.mLayerClient.setGeckoReady(false);
// Then clear out any pending calls on the UI thread by disposing on the UI thread.
ThreadUtils.postToUiThread(new Runnable() {
@Override
public void run() {
- LayerView.this.mToolbarAnimator.notifyCompositorDestroyed();
LayerView.this.mDrawListeners.clear();
disposeNative();
}
});
}
}
/* package */ void handleToolbarAnimatorMessage(int message) {
@@ -237,17 +240,31 @@ public class LayerView extends FrameLayo
setSurfaceBackgroundColor(Color.TRANSPARENT);
break;
case LAYERS_UPDATED:
for (DrawListener listener : mDrawListeners) {
listener.drawFinished();
}
break;
case COMPOSITOR_CONTROLLER_OPEN:
- mToolbarAnimator.notifyCompositorControllerOpen();
+ // It is possible to get this message multiple times. Only act on it if we didn't know the compositor controller was open
+ if (mCompositorControllerOpen) {
+ break;
+ }
+ mCompositorControllerOpen = true;
+ // updateCompositor makes a synchronous call to the compositor which will dead lock if called directly from here
+ ThreadUtils.postToUiThread(new Runnable() {
+ @Override
+ public void run() {
+ mCompositor.setDefaultClearColor(mDefaultClearColor);
+ mCompositor.enableLayerUpdateNotifications(!mDrawListeners.isEmpty());
+ mToolbarAnimator.updateCompositor();
+ updateCompositor();
+ }
+ });
break;
default:
Log.e(LOGTAG, "Unhandled Toolbar Animator Message: " + message);
break;
}
}
private final Compositor mCompositor = new Compositor();
@@ -291,16 +308,17 @@ public class LayerView extends FrameLayo
public void initializeView() {
mLayerClient = new GeckoLayerClient(getContext(), this);
if (mOverscroll != null) {
mLayerClient.setOverscrollHandler(mOverscroll);
}
mPanZoomController = mLayerClient.getPanZoomController();
mToolbarAnimator = mLayerClient.getDynamicToolbarAnimator();
+ mToolbarAnimator.notifyCompositorCreated(mCompositor);
setFocusable(true);
setFocusableInTouchMode(true);
GeckoAccessibility.setDelegate(this);
}
/**
@@ -477,17 +495,17 @@ public class LayerView extends FrameLayo
public void setSurfaceBackgroundColor(int newColor) {
if (mSurfaceView != null) {
mSurfaceView.setBackgroundColor(newColor);
}
}
public void requestRender() {
- if (mCompositorCreated) {
+ if (isCompositorReady()) {
mCompositor.syncInvalidateAndScheduleComposite();
}
}
public interface GetPixelsResult {
public void onPixelsResult(int width, int height, IntBuffer pixels);
}
@@ -498,17 +516,17 @@ public class LayerView extends FrameLayo
@Override
public void run() {
getPixels(getPixelsResult);
}
});
return;
}
- if (mCompositorCreated) {
+ if (isCompositorReady()) {
mGetPixelsResult = getPixelsResult;
mCompositor.requestScreenPixels();
} else {
getPixelsResult.onPixelsResult(0, 0, null);
}
}
public void setListener(Listener listener) {
@@ -545,17 +563,17 @@ public class LayerView extends FrameLayo
mServerSurfaceValid = true;
updateCompositor();
}
void updateCompositor() {
ThreadUtils.assertOnUiThread();
- if (mCompositorCreated) {
+ if (isCompositorReady()) {
// If the compositor has already been created, just resume it instead. We don't need
// to block here because if the surface is destroyed before the compositor grabs it,
// we can handle that gracefully (i.e. the compositor will remain paused).
if (!mServerSurfaceValid) {
return;
}
// Asking Gecko to resume the compositor takes too long (see
// https://bugzilla.mozilla.org/show_bug.cgi?id=735230#c23), so we
@@ -565,22 +583,23 @@ public class LayerView extends FrameLayo
// been resumed, otherwise Gecko may send updates that get dropped.
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()) {
+ if (!mCompositorCreated && mServerSurfaceValid && getLayerClient().isGeckoReady()) {
mCompositorCreated = true;
mCompositor.createCompositor(mWidth, mHeight, getSurface());
- mCompositor.setDefaultClearColor(mDefaultClearColor);
- mCompositor.enableLayerUpdateNotifications(!mDrawListeners.isEmpty());
- mToolbarAnimator.notifyCompositorCreated(mCompositor);
+ }
+
+ if (mCompositorCreated && !mCompositorControllerOpen) {
+ mCompositor.sendToolbarAnimatorMessage(IS_COMPOSITOR_CONTROLLER_OPEN);
}
}
/* 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.
*
@@ -598,17 +617,17 @@ public class LayerView extends FrameLayo
* TextureView instead of a SurfaceView, the first phase is skipped.
*/
private void onSizeChanged(int width, int height) {
if (!mServerSurfaceValid || mSurfaceView == null) {
surfaceChanged(width, height);
return;
}
- if (mCompositorCreated) {
+ if (isCompositorReady()) {
mCompositor.syncResumeResizeCompositor(width, height, getSurface());
}
if (mOverscroll != null) {
mOverscroll.setSize(width, height);
}
}
@@ -634,17 +653,17 @@ public class LayerView extends FrameLayo
// We need to coordinate with Gecko when pausing composition, to ensure
// that Gecko never executes a draw event while the compositor is paused.
// This is sent synchronously to make sure that we don't attempt to use
// any outstanding Surfaces after we call this (such as from a
// 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) {
+ if (isCompositorReady()) {
mCompositor.syncPauseCompositor();
}
mServerSurfaceValid = false;
}
private void onDestroyed() {
serverSurfaceDestroyed();
@@ -739,17 +758,17 @@ public class LayerView extends FrameLayo
addDrawListener(listener);
}
});
return;
}
boolean wasEmpty = mDrawListeners.isEmpty();
mDrawListeners.add(listener);
- if (mCompositorCreated && wasEmpty) {
+ if (isCompositorReady() && wasEmpty) {
mCompositor.enableLayerUpdateNotifications(true);
}
}
@RobocopTarget
public void removeDrawListener(final DrawListener listener) {
if (!ThreadUtils.isOnUiThread()) {
ThreadUtils.postToUiThread(new Runnable() {
@@ -758,17 +777,17 @@ public class LayerView extends FrameLayo
removeDrawListener(listener);
}
});
return;
}
boolean notEmpty = mDrawListeners.isEmpty();
mDrawListeners.remove(listener);
- if (mCompositorCreated && notEmpty && mDrawListeners.isEmpty()) {
+ if (isCompositorReady() && notEmpty && mDrawListeners.isEmpty()) {
mCompositor.enableLayerUpdateNotifications(false);
}
}
@RobocopTarget
public static interface DrawListener {
public void drawFinished();
}
@@ -816,13 +835,13 @@ public class LayerView extends FrameLayo
public void run() {
setClearColor(color);
}
});
return;
}
mDefaultClearColor = color;
- if (mCompositorCreated) {
+ if (isCompositorReady()) {
mCompositor.setDefaultClearColor(mDefaultClearColor);
}
}
}
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -1142,27 +1142,22 @@ const char LayerView::name[] =
constexpr char LayerView::GetCompositor_t::name[];
constexpr char LayerView::GetCompositor_t::signature[];
auto LayerView::GetCompositor() const -> mozilla::jni::Object::LocalRef
{
return mozilla::jni::Method<GetCompositor_t>::Call(LayerView::mCtx, nullptr);
}
-constexpr char LayerView::CompositorCreated_t::name[];
-constexpr char LayerView::CompositorCreated_t::signature[];
-
-auto LayerView::CompositorCreated() const -> bool
+constexpr char LayerView::IsCompositorReady_t::name[];
+constexpr char LayerView::IsCompositorReady_t::signature[];
+
+auto LayerView::IsCompositorReady() const -> bool
{
- return mozilla::jni::Field<CompositorCreated_t>::Get(LayerView::mCtx, nullptr);
-}
-
-auto LayerView::CompositorCreated(bool a0) const -> void
-{
- return mozilla::jni::Field<CompositorCreated_t>::Set(LayerView::mCtx, nullptr, a0);
+ return mozilla::jni::Method<IsCompositorReady_t>::Call(LayerView::mCtx, nullptr);
}
const char LayerView::Compositor::name[] =
"org/mozilla/gecko/gfx/LayerView$Compositor";
constexpr char LayerView::Compositor::AttachToJava_t::name[];
constexpr char LayerView::Compositor::AttachToJava_t::signature[];
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -3557,36 +3557,34 @@ public:
static const mozilla::jni::CallingThread callingThread =
mozilla::jni::CallingThread::UI;
static const mozilla::jni::DispatchTarget dispatchTarget =
mozilla::jni::DispatchTarget::CURRENT;
};
auto GetCompositor() const -> mozilla::jni::Object::LocalRef;
- struct CompositorCreated_t {
+ struct IsCompositorReady_t {
typedef LayerView Owner;
typedef bool ReturnType;
typedef bool SetterType;
typedef mozilla::jni::Args<> Args;
- static constexpr char name[] = "mCompositorCreated";
- static constexpr char signature[] =
- "Z";
+ static constexpr char name[] = "isCompositorReady";
+ static constexpr char signature[] =
+ "()Z";
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;
};
- auto CompositorCreated() const -> bool;
-
- auto CompositorCreated(bool) const -> void;
+ auto IsCompositorReady() const -> bool;
static const mozilla::jni::CallingThread callingThread =
mozilla::jni::CallingThread::UI;
};
class LayerView::Compositor : public mozilla::jni::ObjectBase<Compositor>
{
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -1158,17 +1158,17 @@ class nsWindow::PMPMSupport final
{
PMPMSupport() = delete;
static LayerViewSupport* GetLayerViewSupport(jni::Object::Param aView)
{
const auto& layerView = LayerView::Ref::From(aView);
LayerView::Compositor::LocalRef compositor = layerView->GetCompositor();
- if (!layerView->CompositorCreated() || !compositor) {
+ if (!layerView->IsCompositorReady() || !compositor) {
return nullptr;
}
LayerViewSupport* const lvs = LayerViewSupport::FromNative(compositor);
if (!lvs) {
// There is a pending exception whenever FromNative returns nullptr.
compositor.Env()->ExceptionClear();
}