New issue
Advanced search Search tips

Issue 855888 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Splash screen glitch for Maps Go on Android Go device

Project Member Reported by pkotw...@chromium.org, Jun 23 2018

Issue description

Repro steps:
On an Android Go Device
1) Install Chrome Dev
2) Install Maps Go
3) Launch Maps Go, select "Chrome Dev" as the browser

Expected:
Show splash screen, no flashes
Actual:
Flash when splash screen is shown
 
chrome_dev_maps_go.mp4
23.0 MB Download
Owner: mheikal@chromium.org
Status: Assigned (was: Untriaged)
Summary: Splash screen glitch for Maps Go on Android Go device (was: Splash screen glitch)
This bug is not present when using "Chrome Beta" as the browser. I did a bisect and the culprit CL is https://chromium-review.googlesource.com/1072607

The issue is that the splash screen is shown as part of WebappActivity#postInflationStartup() and the CL delays when the splash screen is shown. (The CL also delays when the other views are inflated)

I tried fixing the obvious bug - show the splash screen synchronously from WebappActivity#doLayoutInflation() and I still get flickering. It seems that adding a view at a z order behind the splash screen causes flickering. That means that I don't know how to fix this bug :(

As an aside, the code in WebappActivity#doLayoutInflation() seems unoptimal. For instance, there are redundant calls to WebappActivity.this.isActivityFinishing()
Cc: pkotw...@chromium.org yfried...@chromium.org
The flickering of adding things behind the splashscreen only occurs on older android versions. I am not sure about what the limit is but occurs on kitkat but not on Oreo.

I think it might be because we are creating a transparent surface view and android changes the transparency hint on our window and recreates it causing the flicker.
I created a cl to fix the first version of the flickering (delaying the splashscreen). The other version, I think would require some help from a UI clanker.
On low end devices, we create a non-transparent surface view. (EGL565). The flicker might go away if we created a ARGB8888 surface view
I think this is a regression of a previous bug, and the way they solved it [1] (I think) is by creating the surface (with the correct opacity) during the first draw event (in the current model, that would be in either preInflationStartup or doLayoutInflation). 

[1] https://chromium.googlesource.com/chromium/src/+/468454d5badc826207b382cf10f5eb2ee124c3e1%5E%21/
I have isolated this (through debugging and stepping through the code to see when the flicker happens). The flicker happens when the surface view is attached to the view hierarchy (even if the rest of the view hierarchy was attached previously, I tested this by delaying just the creation of the surface views). Setting the compositor view visibility to be invisible or gone, as discussed offline did not work to remove the flicker.
I solved the flicker by creating a SurfaceView and attaching it to the UI hierarchy in preinflationstartup (I didnt actually use the SurfaceView in any way, just adding it to the view hierarchy early on before the first draw event solves the flickering issue. I will talk to someone on the UI team on Tuesday to see if there is a cleaner way to do things.
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bd469df1f8dcbab5fa4db7a86f6a9b83cbd8849

commit 5bd469df1f8dcbab5fa4db7a86f6a9b83cbd8849
Author: Mohamed Heikal <mheikal@google.com>
Date: Wed Jul 04 02:30:33 2018

[Android] Moves Splashscreen showing to WebappActivity#preInflationStartup

Since now layout inflation is delayed for WebappActivity, the
splashscreen should be shown in preInflationStartup instead of
postInflationStartup.

This should fix flickering that may occur on some android devices.

Bug:  855888 
Change-Id: Ica4ef8431b67a15ded1c769a50501323dc545bf3
Reviewed-on: https://chromium-review.googlesource.com/1119260
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572453}
[modify] https://crrev.com/5bd469df1f8dcbab5fa4db7a86f6a9b83cbd8849/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/5bd469df1f8dcbab5fa4db7a86f6a9b83cbd8849/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappSplashScreenController.java

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0535832c309af14bfbd26d689bb1d9a0b94617fe

commit 0535832c309af14bfbd26d689bb1d9a0b94617fe
Author: Mohamed Heikal <mheikal@google.com>
Date: Wed Jul 04 17:59:52 2018

[Android] Fix flicker due to delayed attachment of SurfaceView

SurfaceView(s) used by the CompositorSurfaceManagerImpl are normally
attached during the inflation of the CompositorView. Normally that is
before the first draw of the application window happens (in
Activity#OnCreate). However, if the inflation is delayed (if it happens
on a background thread), this means that the SurfaceView is attached
after a draw event has occurred.

At the time of the first attach of a SurfaceView to the view hierarchy
(regardless of the SurfaceView's actual opacity), the window
transparency hint changes (because the window creates a transparent hole
and attaches the SurfaceView to that hole). This may cause older android
versions to destroy the window and redraw it causing a flicker. This one
line CL sets the window transparency hint early so that when the
SurfaceView gets attached later, the transparency hint need not change
and no flickering occurs.

Also removes misleading comments added earlier when this bug was not understood
well enough.

Bug:  855888 ,704866
Change-Id: If213b238a708fe1da5fce7808b0ec2e8cec4998a
Reviewed-on: https://chromium-review.googlesource.com/1125288
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572618}
[modify] https://crrev.com/0535832c309af14bfbd26d689bb1d9a0b94617fe/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/0535832c309af14bfbd26d689bb1d9a0b94617fe/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java

Labels: Merge-Request-68
Requesting to merge the two fixes here for flickering and perceived performance regression. Both regressed in M68.
Project Member

Comment 13 by sheriffbot@chromium.org, Jul 5

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -RegressedIn-68 -Merge-Review-68
Status: Fixed (was: Assigned)
yfriedman@ you are correct. I the bug was tagged regressed in 68 and I did not actually check. I am removing the regressed labels then, and closing the bug since we checked it was fixed by manual testing.
Status: Verified (was: Fixed)
Verified fix in 69.0.3493.4. No white screen / flash seen before the Maps Go splash screen on launch.

Sign in to add a comment