New issue
Advanced search Search tips

Issue 760574 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

TTS results are not spoken, when voice search is launched from today’s view

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

Issue description

App Version: 61.0.3163.72
iOS Version: 9.3.5, 10.3.3, 11.0 beta 8
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 
4. In GLIF animation, give any voice query (eg: time or list of europe countries) 

Observed results:
Notice the TTS results are not spoken

Note:
1. Open new tab, tap on voice search in today view and provide voice query, the TTS results are spoken back
2. This issue is identified after recent fix of http://crbug/753779 

Expected results:
TTS results should be spoken back

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: 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/0B8Cek8RsDbF8N1NWbVEyR0FxQ1U/view?usp=sharing

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

 

Comment 1 by sczs@chromium.org, Aug 30 2017

Labels: ReleaseBlock-Stable M-61
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
kkhorimoto@ could you PTAL. Please assess if this is a blocker or not.
Status: Started (was: Assigned)
Owner: olivierrobin@chromium.org
Status: Assigned (was: Started)
I looked into it and figured out the cause.  Not sure what the best fix is, so I'm assigning to Olivier since he introduced the mechanism to begin with:

https://chromium-review.googlesource.com/c/chromium/src/+/574537 moved the code that launches voice search to the tab added completion block.  For iPhones, this block is called after the new tab animation, so by the time they are executed, the new WebState has already been activated in the WebStateList.
iPads don't have a new tab animation, so the function previously early returned for iPads.  I added some code to call foregroundTabWasAddedCompletionBlock before the early return.  This is does not behave as expected because this is called in BrowserViewController's |-tabWadAdded:|, which is executed after the tab was added but before it's activated.  This means that the VoiceSearchController::StartRecognition() is given the previous tab rather than the newly opened one.
Status: Started (was: Assigned)
Labels: -ReleaseBlock-Stable -M-61 M-62

Comment 6 by pkl@chromium.org, Aug 31 2017

iPad only problem & affecting small number of users. Not RBS. Move to M62. Potentially a M61 Refresh candidate.
Project Member

Comment 7 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 8 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

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Verified on iPad Pro with iOS11.0
Build: 63.0.3213.0 canary
Project Member

Comment 11 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)

TTS results are spoken, when voice search is launched from today’s view with the steps mentioned in comment#0. Looks good.
Project Member

Comment 13 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 on M62.0.3202.69 beta
iPad mini2, ipad pro
iOS: 11.0.3

Sign in to add a comment