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

Issue 894742 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression
Proj-Servicification



Sign in to add a comment

Regression: [Network] Browser gets crashed after adding shortcut on NTP.

Reported by db...@etouch.net, Oct 12

Issue description

Chrome Version: 71.0.3578.0 Revision 7e93e3362fc959ad993d7ea26da37c9ae09cc794-refs/branch-heads/3578@{#1}(32/64 bit)
OS: Win(7,8,8.1,10)

Pre-condition: Enabled 'Enable network service' flag from chrome://flags

What steps will reproduce the problem?
(1) Launch chrome, open NTP and click on 'Add to shortcut' button.
(2) Type text into Name and URL text fields, now click on Done and observe. 

Actual: Browser gets crashed.
Crash ID: Uploaded Crash Report ID e5c7762ee235264f (Local Crash ID: ed324bc6-b9bf-4686-a0e3-c68e4c97bf40)

Expected: Browser should not crash.

This is Regression issue seen in M-71, and will soon update other info.
Good Build: 71.0.3575.0
Bad Build: 71.0.3576.0

Kindly refer attached screen-cast for reference.


 
Actual_Video.mp4
759 KB View Download
Expected_Video.mp4
342 KB View Download
Components: Internals>Services>Network
Labels: -T-MUM-Reported ET-MUM-Reported hasbisect OS-Linux OS-Mac
You are probably looking for a change made after 598149 (known good), but no later than 598170 (first known bad).
CHANGELOG URL:

https://chromium.googlesource.com/chromium/src/+log/55f9e8ca5566a029751b7b00341ebb1e0babcd15..f4c67a8992d2ced65731b383ac475b66a5f09a10

Suspect: https://chromium.googlesource.com/chromium/src/+/a5141b9635cb0545ee6814bd3769c0377159b8f2

@kristipark: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note:
1. Unable to provide 'per-revision' bisect as it shows "We don't have enough builds to bisect. revlist: []" error message.
2. Tried on other machines but still getting the same error again.
3. Hence provided suspect through 'Chromium bisect'
4. Issue is also seen on Mac(10.13.6,10.13.1,10.14.1) and Linux(14.04 LTS) OS.

Just to add some information:

Did this work with Enable network service flag disable?
Yes.

Thank you.
Owner: kristip...@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: ReleaseBlock-Stable
stack trace for the crash id:
-----------------------------
Thread 0 (id: 0x19b4) CRASHED [EXCEPTION_ACCESS_VIOLATION_READ @ 0x0000000f ] MAGIC SIGNATURE THREAD
Stack Quality100%Show frame trust levels
0x000007fee138f644	(chrome.dll -instant_service.cc:392 )	InstantService::DoesUrlResolve(GURL const &,base::OnceCallback<void >)
0x000007fee19f4e69	(chrome.dll -search_tab_helper.cc:328 )	SearchTabHelper::OnDoesUrlResolve(GURL const &,base::OnceCallback<void >)
0x000007fee20347d8	(chrome.dll -search_ipc_router.cc:255 )	SearchIPCRouter::DoesUrlResolve(int,GURL const &,base::OnceCallback<void >)
0x000007fee0e413c5	(chrome.dll -search.mojom.cc:2570 )	chrome::mojom::EmbeddedSearchStubDispatch::AcceptWithResponder(chrome::mojom::EmbeddedSearch *,mojo::Message *,std::unique_ptr<mojo::MessageReceiverWithStatus,std::default_delete<mojo::MessageReceiverWithStatus> >)
0x000007fee2034d7f	(chrome.dll -search.mojom.h:550 )	chrome::mojom::EmbeddedSearchStub<mojo::RawPtrImplRefTraits<chrome::mojom::EmbeddedSearch> >::AcceptWithResponder(mojo::Message *,std::unique_ptr<mojo::MessageReceiverWithStatus,std::default_delete<mojo::MessageReceiverWithStatus> >)
0x000007fedfd1a1d5	(chrome.dll -interface_endpoint_client.cc:398 )	mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message *)
0x000007fee14b76b0	(chrome.dll -ipc_mojo_bootstrap.cc:877 )	IPC::`anonymous namespace'::ChannelAssociatedGroupController::AcceptOnProxyThread
0x000007fee14b5b85	(chrome.dll -bind_internal.h:671 )	base::internal::Invoker<base::internal::BindState<void (IPC::(anonymous namespace)::ChannelAssociatedGroupController::*)(mojo::Message),scoped_refptr<IPC::(anonymous namespace)::ChannelAssociatedGroupController>,base::internal::PassedWrapper<mojo::Message> >,void ()>::Run
0x000007fedfc462ef	(chrome.dll -task_annotator.cc:99 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *)
0x000007fedfc45f7e	(chrome.dll -message_loop.cc:434 )	base::MessageLoop::RunTask(base::PendingTask *)
0x000007fedfc3de24	(chrome.dll -message_loop.cc:517 )	base::MessageLoop::DoWork()
0x000007fedfd441a8	(chrome.dll -message_pump_win.cc:179 )	base::MessagePumpForUI::DoRunLoop()
0x000007fedfc3dadd	(chrome.dll -message_pump_win.cc:52 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x000007fedfc3d6c0	(chrome.dll -run_loop.cc:102 )	base::RunLoop::Run()
0x000007fee0021167	(chrome.dll -chrome_browser_main.cc:2023 )	ChromeBrowserMainParts::MainMessageLoopRun(int *)
0x000007fee0020f79	(chrome.dll -browser_main_loop.cc:998 )	content::BrowserMainLoop::RunMainMessageLoopParts()
0x000007fee0020f24	(chrome.dll -browser_main_runner_impl.cc:165 )	content::BrowserMainRunnerImpl::Run()
0x000007fedfc3e6bf	(chrome.dll -browser_main.cc:47 )	content::BrowserMain(content::MainFunctionParams const &)
0x000007fedfc3e583	(chrome.dll -content_main_runner_impl.cc:541 )	content::RunBrowserProcessMain(content::MainFunctionParams const &,content::ContentMainDelegate *)
0x000007fedfc38596	(chrome.dll -content_main_runner_impl.cc:914 )	content::ContentMainRunnerImpl::Run(bool)
0x000007fedfc24feb	(chrome.dll -main.cc:472 )	service_manager::Main(service_manager::MainParams const &)
0x000007fedfc24be4	(chrome.dll -content_main.cc:19 )	content::ContentMain(content::ContentMainParams const &)
0x000007fedfc219c9	(chrome.dll -chrome_main.cc:102 )	ChromeMain
0x000000013f29374b	(chrome.exe -main_dll_loader_win.cc:201 )	MainDllLoader::Launch(HINSTANCE__ *,base::TimeTicks)
0x000000013f2915ef	(chrome.exe -chrome_exe_main_win.cc:229 )	wWinMain
0x000000013f365a61	(chrome.exe -exe_common.inl:283 )	__scrt_common_main_seh
0x76a759cc	(KERNEL32.dll + 0x000159cc )	BaseThreadInitThunk
0x76ecb980	(ntdll.dll + 0x0002b980 )	RtlUserThreadStart

Adding release blocker label for this issue.Please reduce priority or remove if not the case.

Note: This stack trace is similar to issue 894323.

Thank You!
Status: Started (was: Assigned)
Noting that the crash only occurs a few times after the flag is changed (both enabling and disabling) before stopping completely. Currently investigating why this is the case.
Issue 894323 has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 12

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

commit 8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723
Author: Kristi Park <kristipark@chromium.org>
Date: Fri Oct 12 22:16:52 2018

Initialize test UrlValidityChecker in InstantService and simplify UrlValidityChecker testing

Fix crashes caused by |url_checker_for_testing_| not being initialized
properly.

Additionally, construct UrlValidityChecker with NoDestructor instead of
a global LazyInstance. We prefer to leak singletons to avoid
unnecessary work at shutdown. Also simplify UrlValidityChecker by
passing TickClock, which provides cleaner timeout and duration testing.

Bug:  894742 
Change-Id: I468f76e07dc6f551a8515d8416d0c67f5f2035a1
Reviewed-on: https://chromium-review.googlesource.com/c/1277959
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599377}
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/chrome/browser/search/instant_service.cc
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/chrome/browser/search/instant_service.h
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/chrome/browser/search/url_validity_checker_factory.cc
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/chrome/browser/search/url_validity_checker_factory.h
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/components/search/url_validity_checker.h
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/components/search/url_validity_checker_impl.cc
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/components/search/url_validity_checker_impl.h
[modify] https://crrev.com/8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723/components/search/url_validity_checker_impl_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-71 label, otherwise remove Merge-TBD label. Thanks.
M71 #3578, branched at chromium revision 599034. So this will need a merge to M71. Pls request a merge to M71 ASAP. Thank you.
Labels: Merge-Request-71
Verified that this is working correctly in Canary 72.0.3580.0. Requesting merge to M71.
Labels: -Merge-TBD
Project Member

Comment 12 by sheriffbot@chromium.org, Oct 16

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact 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
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 16

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

commit f39c9f73903fb51c1a130c561e8f60b31ba744ee
Author: Kristi Park <kristipark@chromium.org>
Date: Tue Oct 16 18:59:33 2018

[Merge M71] Initialize test UrlValidityChecker in InstantService and simplify UrlValidityChecker testing

Fix crashes caused by |url_checker_for_testing_| not being initialized
properly.

Additionally, construct UrlValidityChecker with NoDestructor instead of
a global LazyInstance. We prefer to leak singletons to avoid
unnecessary work at shutdown. Also simplify UrlValidityChecker by
passing TickClock, which provides cleaner timeout and duration testing.

Bug:  894742 
Change-Id: I468f76e07dc6f551a8515d8416d0c67f5f2035a1
Reviewed-on: https://chromium-review.googlesource.com/c/1277959
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599377}(cherry picked from commit 8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723)
Reviewed-on: https://chromium-review.googlesource.com/c/1284063
Reviewed-by: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#54}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/chrome/browser/search/instant_service.cc
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/chrome/browser/search/instant_service.h
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/chrome/browser/search/url_validity_checker_factory.cc
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/chrome/browser/search/url_validity_checker_factory.h
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/components/search/url_validity_checker.h
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/components/search/url_validity_checker_impl.cc
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/components/search/url_validity_checker_impl.h
[modify] https://crrev.com/f39c9f73903fb51c1a130c561e8f60b31ba744ee/components/search/url_validity_checker_impl_unittest.cc

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

Commit: f39c9f73903fb51c1a130c561e8f60b31ba744ee
Author: kristipark@chromium.org
Commiter: kristipark@chromium.org
Date: 2018-10-16 18:59:33 +0000 UTC

[Merge M71] Initialize test UrlValidityChecker in InstantService and simplify UrlValidityChecker testing

Fix crashes caused by |url_checker_for_testing_| not being initialized
properly.

Additionally, construct UrlValidityChecker with NoDestructor instead of
a global LazyInstance. We prefer to leak singletons to avoid
unnecessary work at shutdown. Also simplify UrlValidityChecker by
passing TickClock, which provides cleaner timeout and duration testing.

Bug:  894742 
Change-Id: I468f76e07dc6f551a8515d8416d0c67f5f2035a1
Reviewed-on: https://chromium-review.googlesource.com/c/1277959
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599377}(cherry picked from commit 8c7eeb6df92a1d9d33cd68f60bfe51ecaaeeb723)
Reviewed-on: https://chromium-review.googlesource.com/c/1284063
Reviewed-by: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#54}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment