Bug 1296757 - Correctly resume compositor when reattaching; r=snorp

We need to set the compositor-created flag in LayerView correctly when
reattaching to a new LayerView, so that we resume the compositor instead
of trying to create the compositor again, which we already did
previously.

This patch consists of general code cleanup and a new reattach method in
LayerView.Compositor that we call in order to set the compositor-created
flag correctly.
This commit is contained in:
Jim Chen 2016-08-23 18:55:12 -04:00
parent adde79eb82
commit 2672917d6f
5 changed files with 67 additions and 79 deletions

View File

@ -180,6 +180,7 @@ class GeckoLayerClient implements LayerView.Listener, PanZoomTarget
mPanZoomController.destroy();
mToolbarAnimator.destroy();
mDrawListeners.clear();
mGeckoIsReady = false;
}
/**
@ -898,23 +899,6 @@ class GeckoLayerClient implements LayerView.Listener, PanZoomTarget
sendResizeEventIfNecessary(false, null);
}
/** Implementation of LayerView.Listener */
@Override
public void renderRequested() {
if (mView != null) {
mView.invalidateAndScheduleComposite();
}
}
/** Implementation of LayerView.Listener */
@Override
public void sizeChanged(int width, int height) {
// We need to make sure a draw happens synchronously at this point,
// but resizing the surface before the SurfaceView has resized will
// cause a visible jump.
mView.resumeCompositor(width, height);
}
/** Implementation of LayerView.Listener */
@Override
public void surfaceChanged(int width, int height) {

View File

@ -69,14 +69,12 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
/* This should only be modified on the Java UI thread. */
private final Overscroll mOverscroll;
private boolean mServerSurfaceValid;
private int mWidth, mHeight;
/* This is written by the compositor thread (while the UI thread
* is blocked on it) and read by the UI thread. */
/* This is written by the Gecko thread and the UI thread, and read by the UI thread. */
@WrapForJNI(stubName = "CompositorCreated", calledFrom = "ui")
private volatile boolean mCompositorCreated;
/* package */ volatile boolean mCompositorCreated;
private class Compositor extends JNIObject {
public Compositor() {
@ -110,13 +108,20 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
/* package */ native void syncInvalidateAndScheduleComposite();
@WrapForJNI
private synchronized Object getSurface() {
if (LayerView.this.mServerSurfaceValid) {
return LayerView.this.getSurface();
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.
@ -132,9 +137,7 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
}
}
Compositor mCompositor;
private final Compositor mCompositor = new Compositor();
/* Flags used to determine when to show the painted surface. */
public static final int PAINT_START = 0;
@ -177,8 +180,6 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
mOverscroll = null;
}
Tabs.registerOnTabsChangedListener(this);
mCompositor = new Compositor();
}
public LayerView(Context context) {
@ -233,9 +234,6 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
if (mRenderer != null) {
mRenderer.destroy();
}
if (mCompositor != null) {
mCompositor = null;
}
Tabs.unregisterOnTabsChangedListener(this);
}
@ -431,8 +429,8 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
}
public void requestRender() {
if (mListener != null) {
mListener.renderRequested();
if (mCompositorCreated) {
mCompositor.syncInvalidateAndScheduleComposite();
}
}
@ -512,42 +510,25 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
// 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).
resumeCompositor(mWidth, mHeight);
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
// 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);
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. Do it with a synchronous Gecko event so that the
// Android doesn't have a chance to destroy our surface in between.
// happen without needing to block anywhere.
if (mServerSurfaceValid && getLayerClient().isGeckoReady()) {
mCompositorCreated = true;
mCompositor.createCompositor(mWidth, mHeight);
compositorCreated();
}
}
void compositorCreated() {
// This is invoked on the compositor thread, while the java UI thread
// is blocked on the gecko sync event in updateCompositor() above
mCompositorCreated = true;
}
void resumeCompositor(int width, int height) {
// 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.
if (mServerSurfaceValid && mCompositorCreated) {
mCompositor.syncResumeResizeCompositor(width, height);
requestRender();
}
}
/* package */ void invalidateAndScheduleComposite() {
if (mCompositorCreated) {
mCompositor.syncInvalidateAndScheduleComposite();
}
}
@ -575,8 +556,8 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
return;
}
if (mListener != null) {
mListener.sizeChanged(width, height);
if (mCompositorCreated) {
mCompositor.syncResumeResizeCompositor(width, height);
}
if (mOverscroll != null) {
@ -597,10 +578,7 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
}
void notifySizeChanged(int windowWidth, int windowHeight, int screenWidth, int screenHeight) {
final Compositor compositor = mCompositor;
if (compositor != null) {
compositor.onSizeChanged(windowWidth, windowHeight, screenWidth, screenHeight);
}
mCompositor.onSizeChanged(windowWidth, windowHeight, screenWidth, screenHeight);
}
void serverSurfaceDestroyed() {
@ -654,8 +632,6 @@ public class LayerView extends ScrollView implements Tabs.OnTabsChangedListener
}
public interface Listener {
void renderRequested();
void sizeChanged(int width, int height);
void surfaceChanged(int width, int height);
}

View File

@ -1460,6 +1460,14 @@ auto LayerView::Compositor::GetSurface() const -> mozilla::jni::Object::LocalRef
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
{
return mozilla::jni::Method<Reattach_t>::Call(Compositor::mCtx, nullptr);
}
constexpr char LayerView::Compositor::SyncInvalidateAndScheduleComposite_t::name[];
constexpr char LayerView::Compositor::SyncInvalidateAndScheduleComposite_t::signature[];

View File

@ -4747,6 +4747,25 @@ public:
mozilla::jni::DispatchTarget::PROXY;
};
struct Reattach_t {
typedef Compositor Owner;
typedef void ReturnType;
typedef void SetterType;
typedef mozilla::jni::Args<> Args;
static constexpr char name[] = "reattach";
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::GECKO;
static const mozilla::jni::DispatchTarget dispatchTarget =
mozilla::jni::DispatchTarget::CURRENT;
};
auto Reattach() const -> void;
struct SyncInvalidateAndScheduleComposite_t {
typedef Compositor Owner;
typedef void ReturnType;

View File

@ -1110,6 +1110,14 @@ public:
const bool resetting = !!mLayerClient;
mLayerClient = layerClient;
MOZ_ASSERT(aNPZC);
auto npzc = NativePanZoomController::LocalRef(
jni::GetGeckoThreadEnv(),
NativePanZoomController::Ref::From(aNPZC));
mWindow->mNPZCSupport.Attach(npzc, mWindow, npzc);
layerClient->OnGeckoReady();
if (resetting) {
// Since we are re-linking the new java objects to Gecko, we need
// to get the viewport from the compositor (since the Java copy was
@ -1118,14 +1126,6 @@ public:
bridge->ForceIsFirstPaint();
}
}
MOZ_ASSERT(aNPZC);
auto npzc = NativePanZoomController::LocalRef(
jni::GetGeckoThreadEnv(),
NativePanZoomController::Ref::From(aNPZC));
mWindow->mNPZCSupport.Attach(npzc, mWindow, npzc);
layerClient->OnGeckoReady();
}
void OnSizeChanged(int32_t aWindowWidth, int32_t aWindowHeight,
@ -1424,6 +1424,7 @@ nsWindow::GeckoViewSupport::Reattach(const GeckoView::Window::LocalRef& inst,
auto compositor = LayerView::Compositor::LocalRef(
inst.Env(), LayerView::Compositor::Ref::From(aCompositor));
window.mLayerViewSupport.Attach(compositor, &window, compositor);
compositor->Reattach();
}
void