New issue
Advanced search Search tips

Issue 900427 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

[Magic Signature] app_list::PagedViewStructure::LoadFromMetadata

Project Member Reported by mkarkada@chromium.org, Oct 31

Issue description

Chrome OS Version: 11151.17.0, 71.0.3578.27 beta channel caroline

What steps will reproduce the problem?
No exact repro steps
(1) Create a folder in launcher having apps spanning atleast 2 pages
(2) Try drag & move an app in 2nd page to 1st page within this folder

Note that app installation was also in progress in background.

What happens instead?
Random chrome browser crash observed. Issue is not 100% reproducible.

Crash report:
https://crash.corp.google.com/browse?stbtiq=4dd6d4bffdb84301

Crash info:
	0x00005dcf13d1dbee	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ui/views/view_model.h:119 )	app_list::PagedViewStructure::LoadFromMetadata()
0x00005dcf13cf8e5a	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/views/apps_grid_view.cc:2591 )	app_list::AppsGridView::OnListItemMoved(unsigned long, unsigned long, app_list::AppListItem*)
0x00005dcf13d3128b	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/model/app_list_item_list.cc:312 )	app_list::AppListItemList::FixItemPosition(unsigned long)
0x00005dcf13d32701	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/model/app_list_item_list.cc:147 )	app_list::AppListItemList::AddPageBreakItemAfter(app_list::AppListItem const*)
0x00005dcf13d34b07	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/model/app_list_model.cc:79 )	app_list::AppListModel::AddPageBreakItemAfter(app_list::AppListItem const*)
0x00005dcf13d1dd81	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/paged_view_structure.cc:99 )	app_list::PagedViewStructure::SaveToMetadata()
0x00005dcf13cf63e7	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/views/apps_grid_view.cc:1857 )	app_list::AppsGridView::EndDragFromReparentItemInRootLevel(bool, bool)
0x00005dcf13d226a8	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/views/app_list_folder_view.cc:764 )	non-virtual thunk to app_list::AppListFolderView::DispatchEndDragEventForReparent(bool, bool)
0x00005dcf13ceddcd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/views/apps_grid_view.cc:685 )	app_list::AppsGridView::EndDrag(bool)
0x00005dcf13cede42	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/views/app_list_main_view.cc:169 )	app_list::AppsGridView::EndDrag(bool)
0x00005dcf13cf865e	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/views/apps_grid_view.cc:2534 )	app_list::AppsGridView::OnListItemAdded(unsigned long, app_list::AppListItem*)
0x00005dcf13d3358b	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/model/app_list_item_list.cc:218 )	app_list::AppListItemList::AddItem(std::__1::unique_ptr<app_list::AppListItem, std::__1::default_delete<app_list::AppListItem> >)
0x00005dcf13d34789	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/model/app_list_model.cc:321 )	app_list::AppListModel::AddItemToItemListAndNotify(std::__1::unique_ptr<app_list::AppListItem, std::__1::default_delete<app_list::AppListItem> >)
0x00005dcf13d34ec3	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/model/app_list_model.cc:74 )	app_list::AppListModel::AddItemToFolder(std::__1::unique_ptr<app_list::AppListItem, std::__1::default_delete<app_list::AppListItem> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
0x00005dcf1399cae2	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ash/app_list/app_list_controller_impl.cc:117 )	ash::AppListControllerImpl::AddItemToFolder(mojo::StructPtr<ash::mojom::AppListItemMetadata>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
0x00005dcf0fd0e848	(chrome -./gen/ash/public/interfaces/app_list.mojom.cc:2179 )	ash::mojom::AppListControllerStubDispatch::Accept(ash::mojom::AppListController*, mojo::Message*)
0x00005dcf0e3a4384	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:423 )	mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept(mojo::Message*)
0x00005dcf1150a104	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:306 )	mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*)
0x00005dcf0e3a4675	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/mojo/public/cpp/bindings/lib/multiplex_router.cc:590 )	mojo::internal::MultiplexRouter::Accept(mojo::Message*)
0x00005dcf0e3a3d25	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/mojo/public/cpp/bindings/lib/connector.cc:476 )	mojo::Connector::ReadAllAvailableMessages()
0x00005dcf0e3a5753	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/callback.h:129 )	mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&)
0x00005dcf0e393b16	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/callback.h:99 )	base::MessageLoop::DoWork()
0x00005dcf0e3a0f20	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/message_loop/message_pump_libevent.cc:210 )	base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)
0x00005dcf11444553	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/run_loop.cc:102 )	<name omitted>
0x00005dcf10fab71f	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/chrome_browser_main.cc:2028 )	ChromeBrowserMainParts::MainMessageLoopRun(int*)
0x00005dcf0f37a0a9	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/browser/browser_main_loop.cc:998 )	content::BrowserMainLoop::RunMainMessageLoopParts()
0x00005dcf0f37df51	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/browser/browser_main_runner_impl.cc:165 )	content::BrowserMainRunnerImpl::Run()
0x00005dcf0f372931	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/browser/browser_main.cc:47 )	content::BrowserMain(content::MainFunctionParams const&)
0x00005dcf10f974e7	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main_runner_impl.cc:541 )	content::ContentMainRunnerImpl::Run(bool)
0x00005dcf10f9f3fd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/services/service_manager/embedder/main.cc:472 )	service_manager::Main(service_manager::MainParams const&)
0x00005dcf0e4d47f4	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main.cc:19 )	ChromeMain
0x000078f23d168735	(libc-2.23.so -libc-start.c:289 )	__libc_start_main
0x00005dcf0e4b6278	(chrome + 0x00398278 )	_start
0x00007ffd6803d317
 
Attaching debug logs from caroline.
debug-logs_20181030-172641.tgz
4.3 MB Download
Owner: weidongg@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 2

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

commit ea17936a5fe1e1c254aa63c20b08ccfab05b6258
Author: Weidong Guo <weidongg@chromium.org>
Date: Fri Nov 02 17:21:33 2018

Speculative fix for crash in paged view structure

The crash may occur when dragging an item for reparant in root level
while we need to add a page break item between two items in root level.
The two items happened to have the same position which triggers a
position fix causing a LoadFromMetadata() call during SaveToMetadata().

Changes:
Remove observer when add page break item.

Bug:  900427 
Change-Id: Ie416eeb81de47cd531a6c25a16761ce1d141847f
Reviewed-on: https://chromium-review.googlesource.com/c/1313054
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604970}
[modify] https://crrev.com/ea17936a5fe1e1c254aa63c20b08ccfab05b6258/ash/app_list/paged_view_structure.cc

Labels: Merge-Request-71
Merge request for CL in #3. It is safe to merge because it basically has the same fix as [1]. Both DeleteItem() and AddPageBreakItemAfter() might cause loading metadata during saving metadata, thus removing the observer temporarily to fix it.

[1] https://cs.chromium.org/chromium/src/ash/app_list/paged_view_structure.cc?dr=CSs&q=paged_view&g=0&l=85
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 3

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Approved-70
Merge approved for ChromeOS M71
Status: Assigned (was: Untriaged)
Is "Merge-Approved-70" the correct label? The original merge request was for 71
Labels: -Merge-Approved-70 Merge-Approved-71
Fixed, thanks for catching that.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 6

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b347ce9c8144675d8ef2ec18300124c3836c03d

commit 5b347ce9c8144675d8ef2ec18300124c3836c03d
Author: Weidong Guo <weidongg@chromium.org>
Date: Tue Nov 06 17:57:17 2018

Speculative fix for crash in paged view structure

The crash may occur when dragging an item for reparant in root level
while we need to add a page break item between two items in root level.
The two items happened to have the same position which triggers a
position fix causing a LoadFromMetadata() call during SaveToMetadata().

Changes:
Remove observer when add page break item.

Bug:  900427 
Change-Id: Ie416eeb81de47cd531a6c25a16761ce1d141847f
Reviewed-on: https://chromium-review.googlesource.com/c/1313054
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604970}(cherry picked from commit ea17936a5fe1e1c254aa63c20b08ccfab05b6258)
Reviewed-on: https://chromium-review.googlesource.com/c/1320197
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#541}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/5b347ce9c8144675d8ef2ec18300124c3836c03d/ash/app_list/paged_view_structure.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/5b347ce9c8144675d8ef2ec18300124c3836c03d

Commit: 5b347ce9c8144675d8ef2ec18300124c3836c03d
Author: weidongg@chromium.org
Commiter: weidongg@chromium.org
Date: 2018-11-06 17:57:17 +0000 UTC

Speculative fix for crash in paged view structure

The crash may occur when dragging an item for reparant in root level
while we need to add a page break item between two items in root level.
The two items happened to have the same position which triggers a
position fix causing a LoadFromMetadata() call during SaveToMetadata().

Changes:
Remove observer when add page break item.

Bug:  900427 
Change-Id: Ie416eeb81de47cd531a6c25a16761ce1d141847f
Reviewed-on: https://chromium-review.googlesource.com/c/1313054
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604970}(cherry picked from commit ea17936a5fe1e1c254aa63c20b08ccfab05b6258)
Reviewed-on: https://chromium-review.googlesource.com/c/1320197
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#541}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Assigned)

Sign in to add a comment