Commit 5192ab32 authored by Piotr Swigon's avatar Piotr Swigon Committed by Commit Bot

[Installed PWA] Waits for a frame swap before hiding a splash screen.

This patch fixes the flash of white between the splash screen and the
web content in installed web apps. This is achieved with waiting for
a compositor frame swap before starting splash screen hiding animation.

Adding callbacks for next frame swap in CompositorViewHolder might not
be at the right level of abstraction - feedback welcome. I introduced
a generic mechanism as we might need it different places as well.
E.g. In CustomTabActivity the same issue is solved with a time delay,
which I plan to change to this mechanism in a follow up patch.

Existing tests for a webapp splash screen cover showing/hiding it in
various circumstances. Writing a specific test to assert flash of white
is fixed would be more work than it's worth it.

FYI: On Google Pixel this causes the splash screen to be shown about
50-100 later, which seems like the time time when flash of white was
visible.

Bug: 734500
Change-Id: I70f8bde54f251c283e260d17a291685738f9e080
Reviewed-on: https://chromium-review.googlesource.com/569552
Commit-Queue: Piotr Swigon <piotrs@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487336}
parent 53d57824
......@@ -127,6 +127,9 @@ public class CompositorViewHolder extends FrameLayout
private int mOverlayContentWidthMeasureSpec = ContentView.DEFAULT_MEASURE_SPEC;
private int mOverlayContentHeightMeasureSpec = ContentView.DEFAULT_MEASURE_SPEC;
// List of callbacks to be called on nearest frame swap.
private List<Runnable> mNextFrameSwapCallbacks;
/**
* This view is created on demand to display debugging information.
*/
......@@ -598,6 +601,24 @@ public class CompositorViewHolder extends FrameLayout
if (!mSkipInvalidation || pendingFrameCount == 0) flushInvalidation();
mSkipInvalidation = !mSkipInvalidation;
runNextFrameSwapCallbacks();
}
/**
* @param nextFrameSwapCallback Callback to run on a nearest compositor frame swap.
*/
public void addNextFrameSwapCallback(Runnable nextFrameSwapCallback) {
if (mNextFrameSwapCallbacks == null) mNextFrameSwapCallbacks = new ArrayList<>();
mNextFrameSwapCallbacks.add(nextFrameSwapCallback);
}
private void runNextFrameSwapCallbacks() {
if (mNextFrameSwapCallbacks == null) return;
for (Runnable r : mNextFrameSwapCallbacks) {
post(r);
}
mNextFrameSwapCallbacks = null;
}
@Override
......
......@@ -179,7 +179,7 @@ public class WebappActivity extends SingleTabActivity {
controlContainer);
if (getFullscreenManager() != null) getFullscreenManager().setTab(getActivityTab());
mSplashController.onFinishedNativeInit(getActivityTab());
mSplashController.onFinishedNativeInit(getActivityTab(), getCompositorViewHolder());
super.finishNativeInitialization();
mIsInitialized = true;
}
......
......@@ -17,6 +17,7 @@ import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.ContextUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.metrics.WebappUma;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
......@@ -24,6 +25,9 @@ import org.chromium.chrome.browser.util.ColorUtils;
/** Shows and hides splash screen. */
class WebappSplashScreenController extends EmptyTabObserver {
/** Used to schedule splash screen hiding. */
private CompositorViewHolder mCompositorViewHolder;
/** View to which the splash screen is added. */
private ViewGroup mParentView;
......@@ -76,8 +80,9 @@ class WebappSplashScreenController extends EmptyTabObserver {
}
/** Should be called once native has loaded. */
public void onFinishedNativeInit(Tab tab) {
public void onFinishedNativeInit(Tab tab, CompositorViewHolder compositorViewHolder) {
mNativeLoaded = true;
mCompositorViewHolder = compositorViewHolder;
tab.addObserver(this);
if (mInitializedLayout) {
mWebappUma.commitMetrics();
......@@ -92,27 +97,27 @@ class WebappSplashScreenController extends EmptyTabObserver {
@Override
public void didFirstVisuallyNonEmptyPaint(Tab tab) {
if (canHideSplashScreen()) {
hideSplashScreen(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_PAINT);
hideSplashScreenOnNextFrameSwap(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_PAINT);
}
}
@Override
public void onPageLoadFinished(Tab tab) {
if (canHideSplashScreen()) {
hideSplashScreen(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_LOAD_FINISHED);
hideSplashScreenOnNextFrameSwap(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_LOAD_FINISHED);
}
}
@Override
public void onPageLoadFailed(Tab tab, int errorCode) {
if (canHideSplashScreen()) {
hideSplashScreen(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_LOAD_FAILED);
hideSplashScreenOnNextFrameSwap(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_LOAD_FAILED);
}
}
@Override
public void onCrash(Tab tab, boolean sadTabShown) {
hideSplashScreen(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_CRASH);
hideSplashScreenOnNextFrameSwap(tab, WebappUma.SPLASHSCREEN_HIDES_REASON_CRASH);
}
protected boolean canHideSplashScreen() {
......@@ -181,8 +186,25 @@ class WebappSplashScreenController extends EmptyTabObserver {
}
}
/** Hides the splash screen. */
private void hideSplashScreen(final Tab tab, final int reason) {
/**
* Schedules the splash screen hiding animation once the compositor frame has been swapped.
*
* Without this callback we were seeing a short flash of white between the splash screen and
* the web content (crbug.com/734500).
* */
private void hideSplashScreenOnNextFrameSwap(final Tab tab, final int reason) {
if (mSplashScreen == null || mCompositorViewHolder == null) return;
mCompositorViewHolder.addNextFrameSwapCallback(new Runnable() {
@Override
public void run() {
animateHidingSplashScreen(tab, reason);
}
});
}
/** Performs the splash screen hiding animation. */
private void animateHidingSplashScreen(final Tab tab, final int reason) {
if (mSplashScreen == null) return;
mSplashScreen.animate().alpha(0f).withEndAction(new Runnable() {
......@@ -192,6 +214,7 @@ class WebappSplashScreenController extends EmptyTabObserver {
mParentView.removeView(mSplashScreen);
tab.removeObserver(WebappSplashScreenController.this);
mSplashScreen = null;
mCompositorViewHolder = null;
mWebappUma.splashscreenHidden(reason);
}
});
......
......@@ -117,7 +117,7 @@ public class WebApkIntegrationTest {
public void testLaunchAndNavigateOffOrigin() throws Exception {
startWebApkActivity("org.chromium.webapk.test",
mTestServer.getURL("/chrome/test/data/android/test.html"));
waitUntilSplashscreenHides();
InstrumentationRegistry.getInstrumentation().waitForIdleSync();
// We navigate outside origin and expect Custom Tab to open on top of WebApkActivity.
mActivityTestRule.runJavaScriptCodeInCurrentTab(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment