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

Issue 908790 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression
Team-Accessibility



Sign in to add a comment

Regression: Tab crash is seen in chrome://settings/datetime/timeZone when Select-to-Speak is enabled

Project Member Reported by rkalavakuntla@chromium.org, Nov 27

Issue description

Chrome Version:71.0.3578.71/11151.45.0  beta-channel Kip, Daisy & Reks
OS:Chrome OS

What steps will reproduce the problem?
(1)Sign into user >> From Ubertray/Accessibility, enable Select-to-Speak
(2)Click on 'System date and time' in ubertray to open 'Date and Time' Section in settings page
(3)Go to Time zone page, switch the radio buttons continuously and observe Tab crash(Please refer video)

Actual: Tab crash is seen in chrome://settings/datetime/timeZone when Select-to-Speak is enabled
Expected: Tab shouldn't crash in chrome://settings/datetime/timeZone when Select-to-Speak is enabled

This is a Regression issue as same is works fine in M-68

Note: Issue is seen in M-69, M-70 stable also

Crash Ids:
----------
8da3387ab6726221
8edf3972ad34bc9b

Stack Trace:
------------
Thread 0 (id: 0x1d00) CRASHED [SIGSEGV /SEGV_MAPERR @ 0x00000000 ] MAGIC SIGNATURE THREAD
Stack Quality100%Show frame trust levels
0x000057f4b5fb942e	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/third_party/blink/renderer/modules/accessibility/ax_menu_list_option.cc:84 )	blink::AXMenuListOption::ComputeParent() const
0x000057f4adbc75b8	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/third_party/blink/renderer/modules/accessibility/ax_object.cc:2229 )	blink::AXObject::AriaHiddenRoot() const
0x000057f4b5f79b90	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/third_party/blink/renderer/modules/accessibility/ax_object.cc:989 )	blink::AXObject::ComputeIsInertOrAriaHidden(blink::HeapVector<blink::IgnoredReason, 0u>*) const
0x000057f4b5f790ff	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/third_party/blink/renderer/modules/accessibility/ax_object.cc:896 )	blink::AXObject::UpdateCachedAttributeValuesIfNeeded() const
0x000057f4b5f78d61	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/third_party/blink/renderer/modules/accessibility/ax_object.cc:881 )	blink::AXObject::AccessibilityIsIgnored() const
0x000057f4b4948d51	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/third_party/blink/renderer/modules/exported/web_ax_object.cc:1068 )	content::RenderAccessibilityImpl::SendPendingAccessibilityEvents()
0x000057f4b4946ba0	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/renderer/accessibility/render_accessibility_impl.cc:692 )	content::RenderAccessibilityImpl::OnMessageReceived(IPC::Message const&)
0x000057f4b4854172	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/renderer/render_frame_impl.cc:2033 )	content::RenderFrameImpl::OnMessageReceived(IPC::Message const&)
0x000057f4b11a4716	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/ipc/ipc_channel_proxy.cc:320 )	IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&)
0x000057f4adaa6cfe	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/callback.h:99 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0x000057f4adaa05e0	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/task/sequence_manager/thread_controller_impl.cc:196 )	base::sequence_manager::internal::ThreadControllerImpl::DoWork(base::sequence_manager::internal::ThreadControllerImpl::WorkType)
0x000057f4ada98a16	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/callback.h:99 )	base::MessageLoop::DoWork()
0x000057f4ada9a32a	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/message_loop/message_pump_default.cc:37 )	base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
0x000057f4b0b5ed73	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/run_loop.cc:102 )	<name omitted>
0x000057f4b48d3972	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/renderer/renderer_main.cc:202 )	content::RendererMain(content::MainFunctionParams const&)
0x000057f4b06b46a2	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main_runner_impl.cc:499 )	content::ContentMainRunnerImpl::Run(bool)
0x000057f4b06bba3d	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/services/service_manager/embedder/main.cc:472 )	service_manager::Main(service_manager::MainParams const&)
0x000057f4adbef104	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main.cc:19 )	ChromeMain
0x00007be17b7a8735	(libc-2.23.so -libc-start.c:289 )	__libc_start_main
0x000057f4adbd1138	(chrome + 0x003bb138 )	_start
0x00007ffde69ab507		


Link to the list of builds:
----------------------------

https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3AAXMenuListOption%3A%3AComputeParent%27+AND+product_name%3D%27Chrome_ChromeOS%27




 
Tab crash.mp4
12.7 MB View Download
Issue 907363 has been merged into this issue.
Cc: dmazz...@chromium.org
Also crash/e42e979a43c04498
Labels: Hotlist-ConOps-CrOS Hotlist-ConOps-Source-Forum
Components: UI>Accessibility
Owner: katie@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 27

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

commit cd9c59d7dadf0e095a4669b9ea8fd93bd6d475c9
Author: Katie D <katie@chromium.org>
Date: Tue Nov 27 23:30:52 2018

Fix possible null pointer in AXMenuListOption.

Bug:  908790 
Change-Id: I7b8fe648f2190a83954d0eba08d636fa5055e6c6
Reviewed-on: https://chromium-review.googlesource.com/c/1352409
Commit-Queue: Katie Dektar <katie@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611411}
[modify] https://crrev.com/cd9c59d7dadf0e095a4669b9ea8fd93bd6d475c9/third_party/blink/renderer/modules/accessibility/ax_menu_list_option.cc

Labels: Merge-Request-71
It'd be good to merge this into M71. It's a pretty simple (2 line) change.
Project Member

Comment 7 by sheriffbot@chromium.org, Nov 27

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: We are only 6 days from stable.
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
Can you comment on testing?  Even with two lines we need assurances of low risk, esp this late into M71.
Note: this crash will happen with ChromeVox or Select to Speak on Chrome OS, meaning none of those users can use the date/time settings page.

Testing:
There's another crash masking this crash in M72. When I reverted the commits causing the other crash, then applied this 2 line patch, this crash was also fixed. I only tested this in Chrome OS on Linux manually, but can test more broadly if desired - please let me know what's needed!

Here's the masking settings crash in M72: crbug.com/908916. (This does not occur in M71.)


If you can test more broadly please do.  Per comments in #8.  Thanks
Cc: dtseng@chromium.org
I tried to test in M71 but am getting build errors:

git checkout -b m71 -t branch-heads/3578
git fetch https://chromium.googlesource.com/chromium/src refs/changes/09/1352409/2 && git cherry-pick FETCH_HEAD
autoninja -C out/cros chrome

../../chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc:671:12: error: no member named 'IsArcProvisioned' in namespace 'arc'
      arc::IsArcProvisioned(Profile::FromBrowserContext(browser_context())))));

Once I figure out how to build M71 beta I can put it on a chromebook to test further. If anyone knows how to fix this I'd appreciate your input - hence this WIP update!

Impact of not merging:
51192 crashes in M70 so far (of 11725436 reports in M70) = .44% of crashes are from this
Users with accessibility features enabled cannot get to settings://display


I can't check this patched into M71, but:

I did test further in M72, but deploying it to an Eve chromebook.
I confirmed the crash is there if I revert the 5 patches to remove the masking crash: crbug.com/908916
And I confirmed that adding this two line change fixes the crash, and the date/time pages are usable.

I am pretty sure the solution is very safe because the function already returns a nullptr in several other similar cases, in the lines above. But if that's not enough information, maybe dtseng@ can chime in with more.
For help building older versions of Chrome, see:
https://chromium.googlesource.com/chromium/src/+/lkgr/docs/building_old_revisions.md

In particular, it looks like `gclient sync` was missing from your workflow. You might also need an older version of gclient and a new working directory per the instructions above.
Thanks Michael! I did run
gclient sync --with_branch_heads

I also did a few other tries at checking out and building M72, but couldn't get it to build no matter which instructions I followed. It doesn't build with our without my patch. I can build Linux chrome in M72 just fine.

I didn't try cleaning with -ffd and also gclient sync -D --force --reset, so I tried those too. I'm still getting the arc error, though.

I am not going to try getting compatible depot_tools because M71 has commits as recent as today, so I don't think that makes sense.
I tried fixing the build breakage by manually commenting out the part that caused the failure in autotest_private_api.cc. Patching in my change on that and uploading to an Eve chromebook on the Beta channel - confirmed fix in M71!
Cc: khmel@chromium.org
Yeah, that's a real build failure. Filed  issue 910408 .
Looks like the build issue was resolved in #16.  Possible to try again?
Alright:
The crash occurs in M71 at M71 HEAD
The crash does not occur in M71 HEAD with the patch. No other bad behavior was observed in the Settings / time zone page either.

Good to merge?
Labels: -Merge-Review-71 Merge-Approved-71
Yep, merge approved for M71 Chrome OS
Project Member

Comment 20 by bugdroid1@chromium.org, Nov 30

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

commit 1ffd7469b97535404fcd80e9b3e0c712cd96ea0d
Author: Katie D <katie@chromium.org>
Date: Fri Nov 30 21:33:23 2018

Cherry pick to M71: Fix possible null pointer in AXMenuListOption.

TBR=dmazzoni@chromium.org

Bug:  908790 
Change-Id: I7b8fe648f2190a83954d0eba08d636fa5055e6c6
Reviewed-on: https://chromium-review.googlesource.com/c/1352409
Commit-Queue: Katie Dektar <katie@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#611411}(cherry picked from commit cd9c59d7dadf0e095a4669b9ea8fd93bd6d475c9)
Reviewed-on: https://chromium-review.googlesource.com/c/1355777
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#862}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/1ffd7469b97535404fcd80e9b3e0c712cd96ea0d/third_party/blink/renderer/modules/accessibility/ax_menu_list_option.cc

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

Commit: 1ffd7469b97535404fcd80e9b3e0c712cd96ea0d
Author: katie@chromium.org
Commiter: katie@chromium.org
Date: 2018-11-30 21:33:23 +0000 UTC

Cherry pick to M71: Fix possible null pointer in AXMenuListOption.

TBR=dmazzoni@chromium.org

Bug:  908790 
Change-Id: I7b8fe648f2190a83954d0eba08d636fa5055e6c6
Reviewed-on: https://chromium-review.googlesource.com/c/1352409
Commit-Queue: Katie Dektar <katie@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#611411}(cherry picked from commit cd9c59d7dadf0e095a4669b9ea8fd93bd6d475c9)
Reviewed-on: https://chromium-review.googlesource.com/c/1355777
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#862}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)
Cc: vkhabarov@chromium.org
Labels: Hotlist-Enterprise

Sign in to add a comment