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

Issue 753779 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Voice search is not functional when launch from today view

Project Member Reported by rakurati@chromium.org, Aug 9 2017

Issue description

App Version: 61.0.3163.40
iOS Version: 9.3.5, 10.3.3, 11.0 beta 5
Device: iPad only

Steps to reproduce:
1. Launch chrome and load any web page in a tab
2. Swipe down Today's View.
3. Tap on Voice Search button under chrome in Today's View 

Observed results:
GLIF animation is not displayed.

Note:
1. Open new tab, tap on voice search in today view the GLIF animation will be displayed.
2. This issue is identified in canary and also exist from current beta first version after the branch cut.

Expected results:
GLIF animation should be displayed.

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes/No
Bug reproducible after clearing cache and cookies: Yes/No
Bug reproducible on Chrome Mobile on Android: Not tested
Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA
Bug reproducible on current stable build (App Version, iOS Version): No on M60
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M61

Link to video/image:

M61 Beta behavior:
https://drive.google.com/a/google.com/file/d/0B8Cek8RsDbF8VXBGNTlvcEx4OUk/view?usp=sharing

M60 Stable behavior:
https://drive.google.com/a/google.com/file/d/0B8Cek8RsDbF8cFVUejFFNXA5UXc/view?usp=sharing

 
Cc: kkhorimoto@chromium.org
Cc: -kkhorimoto@chromium.org
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
Cc: pinkerton@chromium.org
Labels: M-61
Did this also happen in M60? I'm tempted to make this RBS for 61 if it's a true regression. 
No this issue didn't exist in M60(stable).
Labels: ReleaseBlock-Stable
RBS per pink's comment. 
Kurt, ptal asap.
Status: Started (was: Assigned)
This was caused by some refactoring where completion blocks were added for after a tab was added to the model in the BVC.  These blocks were not executed on iPads.  I've created https://chromium-review.googlesource.com/c/chromium/src/+/625556 to execute the block before the early return.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 22 2017

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

commit ca8bd7defcb69626acaf12a1c0e05805d70caadd
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Tue Aug 22 17:42:50 2017

Call |foregroundTabWasAddedCompletionBlock| on iPad.

On iPhones, this block is executed after the new tab animation has
finished.  This early return, however, prevented it from being executed
on iPad.  One side effect of this change is that startup actions were
not getting executed on iPads after the new Tab was added.

Bug:  753779 
Change-Id: I6c2a5a9aec385905b66ad394ba7ddd0475dfe71f
Reviewed-on: https://chromium-review.googlesource.com/625556
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496357}
[modify] https://crrev.com/ca8bd7defcb69626acaf12a1c0e05805d70caadd/ios/chrome/browser/ui/browser_view_controller.mm

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

Comment 10 by sheriffbot@chromium.org, Aug 22 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Less than 10 days to go before AppStore submit on M61
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: cma...@chromium.org
Status: Verified (was: Fixed)
Verified in Build - 62.0.3195.0 Canary -  iPad iOS 10.3.3, iPad Mini iOS 11 beta 7, iPad Mini iOS 9.3.5, iPhone 6 iOS 11 beta 7
The issue “Voice search is not functional when launch from today view” has been Verified now for the fix.
Labels: -Hotlist-Merge-Review -Merge-Review-61 Merge-Approved-61
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 28 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6970d8d83abf41b05ea1546ae15ceb0e55e16846

commit 6970d8d83abf41b05ea1546ae15ceb0e55e16846
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Mon Aug 28 18:08:45 2017

Call |foregroundTabWasAddedCompletionBlock| on iPad.

On iPhones, this block is executed after the new tab animation has
finished.  This early return, however, prevented it from being executed
on iPad.  One side effect of this change is that startup actions were
not getting executed on iPads after the new Tab was added.

TBR=kkhorimoto@chromium.org

(cherry picked from commit ca8bd7defcb69626acaf12a1c0e05805d70caadd)

Bug:  753779 
Change-Id: I6c2a5a9aec385905b66ad394ba7ddd0475dfe71f
Reviewed-on: https://chromium-review.googlesource.com/625556
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#496357}
Reviewed-on: https://chromium-review.googlesource.com/638853
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#934}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/6970d8d83abf41b05ea1546ae15ceb0e55e16846/ios/chrome/browser/ui/browser_view_controller.mm

Verified in 61.0.3163.72 Beta build on iPad Air (iOS 11.0 beta8), iPad mini (iOS 10.3.3), iPad Air (iOS 9.3.5)

As per steps mentioned in comment#0 the GLIF animation is now displayed when voice search launches from todays view
This fix was landed and merged very late in the 61 branch. It appears to have resulted in a (different) regression in 61 now:  issue 760574 

Kurt please review 760574 asap.
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 6 2017

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

commit c7e46245f70f972913eade944b2b5be227cccfc7
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Wed Sep 06 07:55:43 2017

Call tab added completion asynchronously on iPad

tabWasAdded is called before the webState is activated. Calling the
completion synchronously can lead to running it on wrong webState.

Bug:  753779 ,  760574 
Change-Id: Iaa83650b1cacd58613d092a63a3bd6915bfb1233
Reviewed-on: https://chromium-review.googlesource.com/645347
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499900}
[modify] https://crrev.com/c7e46245f70f972913eade944b2b5be227cccfc7/ios/chrome/browser/ui/browser_view_controller.mm

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 6 2017

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

commit 8964797e09ca547c2cbd2616e838cd54a8fa5d69
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Wed Sep 06 12:41:01 2017

Test foregroundTabWasAddedCompletionBlock existence before call.

Async call means block may not exist anymore.
Follow up of https://chromium-review.googlesource.com/c/chromium/src/+/645347.

Bug:  753779 ,  760574 
Change-Id: I2542d4dc7737905eacfab1ccf508a397c20b3c43
Reviewed-on: https://chromium-review.googlesource.com/652446
Reviewed-by: Elodie Banel <lod@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Elodie Banel <lod@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499941}
[modify] https://crrev.com/8964797e09ca547c2cbd2616e838cd54a8fa5d69/ios/chrome/browser/ui/browser_view_controller.mm

Verified in 63.0.3224.0 Canary, iPhone 6 plus iOS 10.3.3, iPhone 7 iOS11, iPad Pro iOS11
Voice search is not functional when launch from today view, Looks good.
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 5 2017

Labels: merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5d2a0bb3c8a3140bd991552ea9f9c310076d13c

commit c5d2a0bb3c8a3140bd991552ea9f9c310076d13c
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Thu Oct 05 09:10:35 2017

Call tab added completion asynchronously on iPad

tabWasAdded is called before the webState is activated. Calling the
completion synchronously can lead to running it on wrong webState.

Bug:  753779 ,  760574 
Change-Id: Iaa83650b1cacd58613d092a63a3bd6915bfb1233
Reviewed-on: https://chromium-review.googlesource.com/645347
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499900}(cherry picked from commit c7e46245f70f972913eade944b2b5be227cccfc7)
Reviewed-on: https://chromium-review.googlesource.com/702335
Reviewed-by: Elodie Banel <lod@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#588}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/c5d2a0bb3c8a3140bd991552ea9f9c310076d13c/ios/chrome/browser/ui/browser_view_controller.mm

Verified in 62.0.3202.52 Beta on iPad Mini(iOS 10.3.3), iPad Air(iOS 9.3.5) and iPad Air2(iOS 11.0)

Voice search is functional when launch from today view with steps followed in comment#0, Looks good.
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 18 2017

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

commit 8bb80efb322a98197c3744ccc787206fd004e37f
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Wed Oct 18 16:49:01 2017

Test foregroundTabWasAddedCompletionBlock existence before call.

Async call means block may not exist anymore.
Follow up of https://chromium-review.googlesource.com/c/chromium/src/+/645347.

Bug:  753779 ,  760574 
Change-Id: I2542d4dc7737905eacfab1ccf508a397c20b3c43
Reviewed-on: https://chromium-review.googlesource.com/652446
Reviewed-by: Elodie Banel <lod@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Elodie Banel <lod@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499941}(cherry picked from commit 8964797e09ca547c2cbd2616e838cd54a8fa5d69)
Reviewed-on: https://chromium-review.googlesource.com/726279
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#713}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/8bb80efb322a98197c3744ccc787206fd004e37f/ios/chrome/browser/ui/browser_view_controller.mm

Verified in 62.0.3202.69 beta, iPad Air iOS 10.3.3, iPad Pro iOS11
Voice search is functional when launch from today view
Looks good

Sign in to add a comment