New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 729278 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Black flicker on cold start

Project Member Reported by aelias@chromium.org, Jun 3 2017

Issue description

Repro steps:

1) Install Chrome and visit http://theonion.com/
2) Reinstall the same version of Chrome using adb, killing it
3) Tap on the Chrome homescreen icon to start again.  50% of the time, observe black flicker before the page loads.

(Reinstalling Chrome seems to repro the bug more reliably than killing it from the task switcher.)

New in M59.  Reproed on NKG47L as well as O.

Exactly bisected to http://crrev.com/462646 "Revert of cc: Always log shader compiler errors" which is a GPU process startup performance improvement unlikely to have side effects.  So I suspect that startup performance improving sufficiently exposed a preexisting race.
 
Cc: aelias@chromium.org
Owner: khushals...@chromium.org
Khushal offered to take a look.
Video of symptom:
screenrecord-462645-20170606_142202.mp4
3.0 MB View Download
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 7 2017

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

commit 481b7bd458ec703fbe99d111424a7a260658f185
Author: khushalsagar <khushalsagar@chromium.org>
Date: Wed Jun 07 23:24:11 2017

chrome/android: Fix black flash on cold start.

We currently see a black flash on cold start with some pages because
the logic uses the SwapBuffers notification to hide the background color
on the View to make the underlying Surface visible.

Since the SwapBuffers notification doesn't ensure that the rendering
has finished, it may not actually be used by the SurfaceFlinger which
causes a black flash.

Since we have similar logic for the tab strip which is also drawn in
native, merge this logic in one place and remove the background only
after the second frame is drawn.

BUG= 729278 

Review-Url: https://codereview.chromium.org/2926803003
Cr-Commit-Position: refs/heads/master@{#477774}

[modify] https://crrev.com/481b7bd458ec703fbe99d111424a7a260658f185/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
[modify] https://crrev.com/481b7bd458ec703fbe99d111424a7a260658f185/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java

Labels: Merge-Request-60
Project Member

Comment 5 by sheriffbot@chromium.org, Jun 7 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 8 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d317f0434ffd4eacbe8f4f412da254a411cf34ea

commit d317f0434ffd4eacbe8f4f412da254a411cf34ea
Author: Khushal <khushalsagar@chromium.org>
Date: Thu Jun 08 01:22:14 2017

chrome/android: Fix black flash on cold start.

We currently see a black flash on cold start with some pages because
the logic uses the SwapBuffers notification to hide the background color
on the View to make the underlying Surface visible.

Since the SwapBuffers notification doesn't ensure that the rendering
has finished, it may not actually be used by the SurfaceFlinger which
causes a black flash.

Since we have similar logic for the tab strip which is also drawn in
native, merge this logic in one place and remove the background only
after the second frame is drawn.

BUG= 729278 

Review-Url: https://codereview.chromium.org/2926803003
Cr-Original-Commit-Position: refs/heads/master@{#477774}
Review-Url: https://codereview.chromium.org/2928703006 .
Cr-Commit-Position: refs/branch-heads/3112@{#244}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/d317f0434ffd4eacbe8f4f412da254a411cf34ea/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
[modify] https://crrev.com/d317f0434ffd4eacbe8f4f412da254a411cf34ea/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java

Status: Fixed (was: Assigned)
Thanks for the quick fix, I appreciate it!
No problem! :)

Comment 10 by aluo@chromium.org, Jun 16 2017

Cc: amineer@chromium.org
Labels: Merge-Request-59
Noticed this was not merged to M59 yet, we've had respins in the past due to screen flicker issues.
Labels: -Merge-Request-59
This flicker is pretty rare and the fix has some risk of introducing other flickers so I'd like to give it time to bake.  I don't think it calls for a last-minute merge.

Sign in to add a comment