New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment
link

Issue 914891: Answer card crashes the browser

Reported by kaznacheev@chromium.org, Dec 13 Project Member

Issue description

Chrome Version: 72.0.3626.15

What steps will reproduce the problem?
(1) Type "time" in the launcher search
(2) Observe the answer card
(3) Type backspace
(4) Observe the answer card disappear
(4) Type "e" 

What is the expected result?
Answer card re-appears

What happens instead?
Answer card does not re-appear, browser crashes.



Please use labels and text to provide additional information.

If this is a regression (i.e., worked before), please consider using the
bisect tool (https://www.chromium.org/developers/bisect-builds-py) to help
us identify the root cause and more rapidly triage the issue.

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.
 

Comment 1 by newcomer@chromium.org, Dec 13

Cc: newcomer@chromium.org
 Issue 914605  has been merged into this issue.

Comment 2 by newcomer@chromium.org, Dec 13

Cc: rockot@google.com
Components: UI>Shell>Launcher
+rockot who worked on a refactor recently (shot in the dark).

Comment 3 by rockot@google.com, Dec 13

Hmm. I am unable to repro this on ToT.

Comment 4 by kaznacheev@chromium.org, Dec 13

rockot@, are you testing on device?
for me this crashes reliably on eve R73-11396.0.0 (Chrome 73.0.3639.0)
does not crash for me on ToT Linux

Comment 5 by manucornet@chromium.org, Dec 14

Owner: rockot@google.com
Assigning to rockot@ assuming it might be related to the refactor -- I actually never worked on client-side answer cards.

Comment 6 by rockot@google.com, Dec 14

I have no way to test on a device right now, unfortunately. Are you able to get a crash stack when you repro?

Comment 7 by kaznacheev@chromium.org, Dec 14

Don't know if this is relevant, but I was able to get this stacktrace once (Linux, dcheck_always_on = true):

Received signal 11 SEGV_MAPERR 000000000002
#0 0x7fdca86aa81d base::debug::StackTrace::StackTrace()
#1 0x7fdca83a561a base::debug::StackTrace::StackTrace()
#2 0x7fdca86aa29f base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7fdc779a00c0 <unknown>
#4 0x7fdc6e7155c0 base::internal::UncheckedObserverAdapter::IsEqual()
#5 0x7fdc6e7155a0 _ZZN4base12ObserverListIN8app_list22SearchBoxModelObserverELb0ELb1ENS_8internal24UncheckedObserverAdapterEE14RemoveObserverEPKS2_ENKUlRKT_E_clIS4_EEDaSA_
#6 0x7fdc6e724270 base::ObserverList<>::RemoveObserver()
#7 0x7fdc6e72372c app_list::SearchResult::RemoveObserver()
#8 0x7fdc6fa949bb app_list::SearchResultView::ClearResultNoRepaint()
#9 0x7fdc6fa94c60 app_list::SearchResultView::SetResult()
#10 0x7fdc6fa82cc6 app_list::SearchResultListView::DoUpdate()
#11 0x7fdc6fa804cd app_list::SearchResultContainerView::Update()
#12 0x7fdc6fa35e6f _ZN4base8internal13FunctorTraitsIMN8app_list15AppListItemViewEFvvEvE6InvokeIS5_NS_7WeakPtrIS3_EEJEEEvT_OT0_DpOT1_
#13 0x7fdc6fa819da _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMN8app_list25SearchResultContainerViewEFvvENS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#14 0x7fdc6fa81970 _ZN4base8internal7InvokerINS0_9BindStateIMN8app_list25SearchResultContainerViewEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIS6_NSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSD_16integer_sequenceImJXspT1_EEEE
#15 0x7fdc6fa818b9 _ZN4base8internal7InvokerINS0_9BindStateIMN8app_list25SearchResultContainerViewEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#16 0x7fdca83573fe _ZNO4base12OnceCallbackIFvvEE3RunEv
#17 0x7fdca83a6aca base::debug::TaskAnnotator::RunTask()
#18 0x7fdca843791c base::MessageLoopImpl::RunTask()
#19 0x7fdca8437c1b base::MessageLoopImpl::DeferOrRunPendingTask()
#20 0x7fdca8438454 base::MessageLoopImpl::DoWork()
#21 0x7fdca86fd859 base::MessagePumpLibevent::Run()
#22 0x7fdca8437136 base::MessageLoopImpl::Run()
#23 0x7fdca84e2522 base::RunLoop::Run()
#24 0x55d2e58911ae ChromeBrowserMainParts::MainMessageLoopRun()
#25 0x7fdca0913213 content::BrowserMainLoop::RunMainMessageLoopParts()
#26 0x7fdca091b8d0 content::BrowserMainRunnerImpl::Run()
#27 0x7fdca09066ee content::BrowserMain()
#28 0x7fdca2bf4520 content::RunBrowserProcessMain()
#29 0x7fdca2bf7955 content::ContentMainRunnerImpl::RunServiceManager()
#30 0x7fdca2bf6559 content::ContentMainRunnerImpl::Run()
#31 0x7fdca2bed31c content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#32 0x7fdc740614d1 service_manager::Main()
#33 0x7fdca2bf1c35 content::ContentMain()
#34 0x55d2e15fa216 ChromeMain
#35 0x55d2e15fa122 main
#36 0x7fdc760c62b1 __libc_start_main
#37 0x55d2e15f9ffa _start

Comment 8 by rockot@google.com, Dec 14

Cc: xiy...@chromium.org
Thank you, that looks like it's almost certainly relevant. I am not sure it's necessarily related to my refactor though. I'll see if I can hunt down any bad observer lifetime issues in this code.

+xiyuan@ who may have more immediate insights though.

Comment 9 by xiy...@chromium.org, Dec 14

The stack in #7 seems suggesting this is not answer card related because the crash is from SearchResultListView, which is a container that does not handle answer card.

The crash is likely to be caused by a SearchResult being released without notifying/updating its SearchResultView.

Comment 10 by xiy...@chromium.org, Dec 14

Cc: -xiy...@chromium.org
Owner: xiy...@chromium.org
Let me take this.

Comment 11 by xiy...@chromium.org, Dec 14

It turns out this is caused by my CL that deletes the search-what-user-typed result when showing answer card [1]. This changes the search results mode without going the regular update code path. Will fix.

[1] https://cs.chromium.org/chromium/src/ash/app_list/views/search_result_answer_card_view.cc?rcl=c0fd077dcd55a26ea05eedc16cad0d3693ac33cf&l=281

Comment 12 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/36323f0591ce6cceb8749c5e7d9d876e4eb8eae7

commit 36323f0591ce6cceb8749c5e7d9d876e4eb8eae7
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri Dec 14 19:25:01 2018

app_list: Fix SearchResultView crash

Fix the crash introduced in http://crrev.com/c/1351714 that deletes
the search-what-you-type result when showing the answer card. The
change to SearchResults model does not go through regular search
results update code path. Hence not handled properly. The CL fixes
the issue by making SearchResultView handle OnResultDestroying so
that its internal state is updated whenever underlying SearchResult
goes away.

Bug:  914891 
Change-Id: Idc17516865333114e47b218d17d3629f50657f4a
Reviewed-on: https://chromium-review.googlesource.com/c/1378487
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616782}
[modify] https://crrev.com/36323f0591ce6cceb8749c5e7d9d876e4eb8eae7/ash/app_list/views/search_result_view.cc
[modify] https://crrev.com/36323f0591ce6cceb8749c5e7d9d876e4eb8eae7/ash/app_list/views/search_result_view.h

Comment 13 by xiy...@chromium.org, Dec 14

Labels: Merge-Request-72

Comment 14 by kaznacheev@chromium.org, Dec 14

Tried the fixed on Caroline, no crash, yay!

Comment 15 by gov...@chromium.org, Dec 14

Pls apply appropriate OSs label.

Comment 16 by xiy...@chromium.org, Dec 14

Labels: OS-Chrome

Comment 17 by sheriffbot@chromium.org, Dec 15

Project Member
Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 18 by dgagnon@google.com, Dec 17

Labels: -Hotlist-Merge-Review -Merge-Review-72 Merge-Approved-72

Comment 19 by bugdroid1@chromium.org, Dec 17

Project Member
Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c5d8d08929cb27e23e0f755e6325d7dce764713

commit 1c5d8d08929cb27e23e0f755e6325d7dce764713
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Mon Dec 17 16:44:11 2018

Merge M72 "app_list: Fix SearchResultView crash"

> Fix the crash introduced in http://crrev.com/c/1351714 that deletes
> the search-what-you-type result when showing the answer card. The
> change to SearchResults model does not go through regular search
> results update code path. Hence not handled properly. The CL fixes
> the issue by making SearchResultView handle OnResultDestroying so
> that its internal state is updated whenever underlying SearchResult
> goes away.
> 
> Bug:  914891 
> Change-Id: Idc17516865333114e47b218d17d3629f50657f4a
> Reviewed-on: https://chromium-review.googlesource.com/c/1378487
> Reviewed-by: Alex Newcomer <newcomer@chromium.org>
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616782}
> (cherry picked from commit 36323f0591ce6cceb8749c5e7d9d876e4eb8eae7)

Change-Id: Ib43143f966a79955a2fc541b007bf4a2e74bfd50
Reviewed-on: https://chromium-review.googlesource.com/c/1379494
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#387}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/1c5d8d08929cb27e23e0f755e6325d7dce764713/ash/app_list/views/search_result_view.cc
[modify] https://crrev.com/1c5d8d08929cb27e23e0f755e6325d7dce764713/ash/app_list/views/search_result_view.h

Comment 20 by xiy...@chromium.org, Dec 17

Status: Fixed (was: Assigned)

Sign in to add a comment