Skip to content

Commit

Permalink
[WebLayer] Fix black flash when keyboard shows
Browse files Browse the repository at this point in the history
This was happening because compositor_->SetSurface() was being called
when the height changed.

Before: https://screenshot.googleplex.com/E5dJumgTkUo
After: https://screenshot.googleplex.com/0O6V5coCCEd

There's still a bit of an ugly black square when the keyboard hides, but
this is better than before.

(cherry picked from commit b6ce364)

Bug: 1033632
Change-Id: I4a96079df50f1c53cbd0948dff0af37da601f68e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1965995
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#724427}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1986910
Reviewed-by: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/branch-heads/3945@{#1005}
Cr-Branched-From: e4635ff-refs/heads/master@{#706915}
  • Loading branch information
clarkduvall authored and Ben Mason committed Jan 3, 2020
1 parent 22b4e69 commit 1a5ed49
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
9 changes: 7 additions & 2 deletions weblayer/browser/content_view_render_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,23 +83,28 @@ void ContentViewRenderView::OnPhysicalBackingSizeChanged(

void ContentViewRenderView::SurfaceCreated(JNIEnv* env) {
InitCompositor();
current_surface_format_ = 0;
}

void ContentViewRenderView::SurfaceDestroyed(JNIEnv* env,
jboolean cache_back_buffer) {
if (cache_back_buffer)
compositor_->CacheBackBufferForCurrentSurface();
compositor_->SetSurface(nullptr, false);
current_surface_format_ = 0;
}

void ContentViewRenderView::SurfaceChanged(
JNIEnv* env,
jboolean can_be_used_with_surface_control,
jint format,
jint width,
jint height,
const JavaParamRef<jobject>& surface) {
// TODO(boliu): Avoid SetSurface if only size changed.
compositor_->SetSurface(surface, can_be_used_with_surface_control);
if (current_surface_format_ != format) {
current_surface_format_ = format;
compositor_->SetSurface(surface, can_be_used_with_surface_control);
}
compositor_->SetWindowBounds(gfx::Size(width, height));
}

Expand Down
3 changes: 3 additions & 0 deletions weblayer/browser/content_view_render_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ContentViewRenderView : public content::CompositorClient {
void SurfaceDestroyed(JNIEnv* env, jboolean cache_back_buffer);
void SurfaceChanged(JNIEnv* env,
jboolean can_be_used_with_surface_control,
jint format,
jint width,
jint height,
const base::android::JavaParamRef<jobject>& surface);
Expand All @@ -75,6 +76,8 @@ class ContentViewRenderView : public content::CompositorClient {
scoped_refptr<cc::Layer> root_container_layer_;
scoped_refptr<cc::Layer> web_contents_layer_;

int current_surface_format_ = 0;

DISALLOW_COPY_AND_ASSIGN(ContentViewRenderView);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public class ContentViewRenderView extends FrameLayout {
// Common interface to listen to surface related events.
private interface SurfaceEventListener {
void surfaceCreated();
void surfaceChanged(
Surface surface, boolean canBeUsedWithSurfaceControl, int width, int height);
void surfaceChanged(Surface surface, boolean canBeUsedWithSurfaceControl, int format,
int width, int height);
// |cacheBackBuffer| will delay destroying the EGLSurface until after the next swap.
void surfaceDestroyed(boolean cacheBackBuffer);
}
Expand Down Expand Up @@ -125,12 +125,12 @@ public void surfaceCreated() {
}

@Override
public void surfaceChanged(
Surface surface, boolean canBeUsedWithSurfaceControl, int width, int height) {
public void surfaceChanged(Surface surface, boolean canBeUsedWithSurfaceControl, int format,
int width, int height) {
assert mNativeContentViewRenderView != 0;
assert mSurfaceData == ContentViewRenderView.this.mCurrent;
ContentViewRenderViewJni.get().surfaceChanged(mNativeContentViewRenderView,
canBeUsedWithSurfaceControl, width, height, surface);
canBeUsedWithSurfaceControl, format, width, height, surface);
if (mWebContents != null) {
ContentViewRenderViewJni.get().onPhysicalBackingSizeChanged(
mNativeContentViewRenderView, mWebContents, width, height);
Expand Down Expand Up @@ -416,10 +416,10 @@ public void surfaceCreated() {
}

@Override
public void surfaceChanged(
Surface surface, boolean canBeUsedWithSurfaceControl, int width, int height) {
public void surfaceChanged(Surface surface, boolean canBeUsedWithSurfaceControl, int format,
int width, int height) {
if (mMarkedForDestroy) return;
mListener.surfaceChanged(surface, canBeUsedWithSurfaceControl, width, height);
mListener.surfaceChanged(surface, canBeUsedWithSurfaceControl, format, width, height);
mNumSurfaceViewSwapsUntilVisible = 2;
}

Expand Down Expand Up @@ -468,7 +468,7 @@ public void surfaceCreated(SurfaceHolder holder) {

@Override
public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
mListener.surfaceChanged(holder.getSurface(), true, width, height);
mListener.surfaceChanged(holder.getSurface(), true, format, width, height);
}

@Override
Expand Down Expand Up @@ -509,7 +509,7 @@ public void onSurfaceTextureSizeChanged(
mCurrentSurfaceTexture = surfaceTexture;
mCurrentSurface = new Surface(mCurrentSurfaceTexture);
}
mListener.surfaceChanged(mCurrentSurface, false, width, height);
mListener.surfaceChanged(mCurrentSurface, false, PixelFormat.OPAQUE, width, height);
}

@Override
Expand Down Expand Up @@ -688,7 +688,7 @@ void onPhysicalBackingSizeChanged(
void surfaceCreated(long nativeContentViewRenderView);
void surfaceDestroyed(long nativeContentViewRenderView, boolean cacheBackBuffer);
void surfaceChanged(long nativeContentViewRenderView, boolean canBeUsedWithSurfaceControl,
int width, int height, Surface surface);
int format, int width, int height, Surface surface);
void evictCachedSurface(long nativeContentViewRenderView);
ResourceManager getResourceManager(long nativeContentViewRenderView);
}
Expand Down

0 comments on commit 1a5ed49

Please sign in to comment.