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

Issue 680474 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Blank snapshots are displayed in tab switcher mode for normal and incognito tabs.

Project Member Reported by pmadalla@chromium.org, Jan 12 2017

Issue description

App Version: 56.0.2924.59 dev
iOS Version: 9.3.5, iOS 10
Device: iPad

Steps to reproduce:
 1. Launch chrome 
 2. Tap on Menu > New Incognito tab.
 3. Tap on Tabswitcher icon.
 
Observed results:
Blank snapshot is seen in tab switcher mode.

Expected results:
You’ve gone incognito text should be seen.

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: NA
Bug reproducible on Dolphin/Safari/Firefox: NA
Bug reproducible on current stable build (App Version, iOS Version): No in M55.0.2883.79
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes in 56.0.2924.59 dev

Good Build : 56.0.2924.57 dev
Bad Build : 56.0.2924.58 dev

Link to Image : 

Observed :
https://drive.google.com/a/google.com/file/d/0B--UpU2GW2EpamxLd1R0X1pOYTQ/view?usp=sharing

Expected :
https://drive.google.com/a/google.com/file/d/0B--UpU2GW2EpQjhVQkpwSDc3cGM/view?usp=sharing

 
Cc: linds...@chromium.org
Labels: -Pri-2 M-56 Pri-1
Owner: jif@chromium.org
Status: Assigned (was: Untriaged)
Hi Jif,
Can you ptal?
Thanks,

Comment 2 by jif@chromium.org, Jan 16 2017

Status: Started (was: Assigned)

Comment 3 by jif@chromium.org, Jan 16 2017

Cc: noyau@chromium.org
Broken by https://bugs.chromium.org/p/chromium/issues/detail?id=677374.
The CL from 677374 was cherry picked to M56, which explains why the blank snapshots occurs on M56.

Here's a quick explanation of the problem:
The cells of the Tab Switcher request the snapshots using their delegate.
Unfortunately, the CL from 677374 changed the order in which the cells were initialized:
The CL made it so that the delegate was set on the cells _after_ the cells requested the snapshots.

The fix is to restore the order in which the cells are initialized.
In addition to the fix, I've added a DCHECK checking that the delegate is initialized before requesting the snapshots.

Long term fix is to have an integration test that checks the pixels of the snapshots are roughly correct. Filed crbug.com/681615
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 17 2017

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

commit bc8c881127daf08a8c857e55f465699ed32bcb44
Author: jif <jif@chromium.org>
Date: Tue Jan 17 09:39:42 2017

Fix the order in which the iOS Tab Switcher cells are initialized.

The delegate of the cell is used in |setAppearanceForTab:cellSize:| to obtain the
snapshot.
https://codereview.chromium.org/2615883002 incorrectly moved the initialization of
the delegate after the call to |setAppearanceForTab:cellSize:|. This CL fixes that.

BUG= 680474 

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

[modify] https://crrev.com/bc8c881127daf08a8c857e55f465699ed32bcb44/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_cell.h
[modify] https://crrev.com/bc8c881127daf08a8c857e55f465699ed32bcb44/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_cell.mm
[modify] https://crrev.com/bc8c881127daf08a8c857e55f465699ed32bcb44/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_controller.mm

Comment 6 by jif@chromium.org, Jan 17 2017

Labels: Merge-Request-56
Status: Fixed (was: Started)
Project Member

Comment 7 by sheriffbot@chromium.org, Jan 17 2017

Labels: -Merge-Request-56 Merge-Review-56 Hotlist-Merge-Review
This bug requires manual review: Less than 2 weeks to go before AppStore submit on M56
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

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

Comment 8 by cma...@chromium.org, Jan 17 2017

pmadalla@, please verify this bug whenever possible. Thanks!
Status: Verified (was: Fixed)
Verified the issue on the latest canary 57.0.2985.0 canary tested on iPadMini(9.3.5 & iOS 10),iPad2(9.3.5).
Thumbnails are displayed in tab-switcher both in normal and incognito mode,works fine.

Comment 10 by jif@chromium.org, Jan 19 2017

Labels: ReleaseBlock-Stable

Comment 11 by jif@chromium.org, Jan 19 2017

Cc: cma...@chromium.org
Note that there are 2 options to fix this bug in M56:
A/ Either we cherry pick the -1 +2 fix ( https://codereview.chromium.org/2631233002/diff/40001/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_controller.mm )
B/ Either we revert https://bugs.chromium.org/p/chromium/issues/detail?id=677374 in M56.

Labels: -Merge-Review-56 Merge-Approved-56
I am not very comfortable with option B. A seems more reasonable while looking at the CL. Let me know if you think otherwise.

Comment 13 by jif@chromium.org, Jan 20 2017

I agree.
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 23 2017

Labels: -merge-approved-56 Merge-Merged-2924
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/a74d819badc7407440b1518c84abe6181af36ad3

commit a74d819badc7407440b1518c84abe6181af36ad3
Author: Jean-François Geyelin <jif@google.com>
Date: Mon Jan 23 10:21:09 2017

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 23 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/a74d819badc7407440b1518c84abe6181af36ad3

commit a74d819badc7407440b1518c84abe6181af36ad3
Author: Jean-François Geyelin <jif@google.com>
Date: Mon Jan 23 10:21:09 2017

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 23 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/a74d819badc7407440b1518c84abe6181af36ad3

commit a74d819badc7407440b1518c84abe6181af36ad3
Author: Jean-François Geyelin <jif@google.com>
Date: Mon Jan 23 10:21:09 2017

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 23 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/a74d819badc7407440b1518c84abe6181af36ad3

commit a74d819badc7407440b1518c84abe6181af36ad3
Author: Jean-François Geyelin <jif@google.com>
Date: Mon Jan 23 10:21:09 2017

Verified the issue on the latest version 58.0.2991.0 canary tested on iPad(iOS 10).
Thumbnails are displayed in tab-switcher both in normal and incognito mode,works fine.
Verified on chrome beta version 56.0.2924.75 on iPad Air with iOS 10.1.1 following the steps mentioned in comment #0.  Looks good.

Comment 20 by jif@chromium.org, Jan 25 2017

Thank you Venu!

Sign in to add a comment