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

Issue 661358 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

DCHECK when selecting the bottommost voice search language in settings

Project Member Reported by rohitrao@chromium.org, Nov 1 2016

Issue description

Open up voice search settings and scroll to the bottom of the list.  On tapping the bottommost item, you'll hit:

DCHECK_LT(static_cast<size_t>(index),
              localeConfig->GetAvailableLocales().size());
 
Summary: DCHECK when selecting the bottommost voice search language in settings (was: OOB crash when selecting the bottommost voice search language in settings)
I think the DCHECK just needs to be updated to use (index - 1).

There is no OOB array access because we already use (index - 1) when indexing into the array.
An alternative fix would be to create a separate item type for DefaultLanguageOption (vs LanguageOption).  Then we could handle that separately based on item type, rather than checking index == 0.
Oh whoops, good catch.  I like the new item type approach the best, but I'm okay if you just update the DCHECK to (index - 1) to keep changes minimal.

Comment 4 by cma...@chromium.org, Nov 21 2016

Friendly ping on this blocker for the current release.

Comment 5 by pkl@chromium.org, Nov 22 2016

Is someone fixing this? Rohit? Kurt? Sounds like it should be fixed. 

It's a DCHECK, so won't affect release builds. Does this really need to be a ReleaseBlock-Stable?
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 22 2016

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

commit 3746bdbc5b5d232fb2e0b81a5a1c150d4ffaa280
Author: rohitrao <rohitrao@google.com>
Date: Tue Nov 22 21:15:02 2016

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

Comment 8 by bugdroid1@chromium.org, Nov 22 2016

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

commit 3746bdbc5b5d232fb2e0b81a5a1c150d4ffaa280
Author: rohitrao <rohitrao@google.com>
Date: Tue Nov 22 21:15:02 2016

Comment 9 by dimu@chromium.org, Nov 22 2016

Labels: -Merge-Request-55 Merge-Review-55 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before AppStore submit on M55, manual review required.
Labels: -Hotlist-Merge-Review -Merge-Review-55 Merge-Approved-55
Labels: Merge-Request-56
Also needs to be on M56.

Comment 12 by dimu@chromium.org, Nov 23 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 23 2016

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

commit bc88874acf1ad47ec7fadceb34aff288916ba942
Author: rohitrao <rohitrao@google.com>
Date: Tue Nov 22 21:15:02 2016

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 23 2016

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

commit bc88874acf1ad47ec7fadceb34aff288916ba942
Author: rohitrao <rohitrao@google.com>
Date: Tue Nov 22 21:15:02 2016

Labels: -M-55 -Merge-Approved-55 M-56
Moving out of 55 because I'm having trouble cherry-picking and it's only a DCHECK, so it won't affect production builds.

Sign in to add a comment