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

Issue 831258 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

Chrome OS crash reading unregistered Docked Magnifier preference.

Project Member Reported by msw@chromium.org, Apr 10 2018

Issue description

Chrome OS crash reading unregistered Docked Magnifier preference.

On ToT @ #549307 build chrome os on linux desktop.
(1) Run chrome, open settings from system tray or about:settings
(2) Enable the "Always show accessibility options in the system menu" option.
(3) Click the system tray to show the bubble.
Expected: Bubble shows, no crash.
Actual: Crash; stack below.

Ahmed, please take a look, it appears that registration occurs here:
https://cs.chromium.org/chromium/src/ash/magnifier/docked_magnifier_controller.cc?rcl=c2bbed2e92b8ec200bb6d83056088c810366cc49&l=148

[75936:75936:0410/111422.492304:FATAL:pref_service.cc(121)] Check failed: false. Trying to read an unregistered pref: ash.docked_magnifier.enabled
#0 0x7ffb7f9bdf4c base::debug::StackTrace::StackTrace()
#1 0x7ffb7f9e4e2b logging::LogMessage::~LogMessage()
#2 0x7ffb7ae93062 PrefService::GetBoolean()
#3 0x5561e305f10d chromeos::AccessibilityManager::ShouldShowAccessibilityMenu()
#4 0x5561e497a3a3 (anonymous namespace)::AccessibilityDelegateImpl::ShouldShowAccessibilityMenu()
#5 0x7ffb79c432f6 ash::TrayAccessibility::CreateDefaultView()
#6 0x7ffb79c38584 ash::SystemTrayView::CreateItemViews()
#7 0x7ffb79c35059 ash::SystemTrayBubble::InitView()
#8 0x7ffb79c333ab ash::SystemBubbleWrapper::InitView()
#9 0x7ffb79c329df ash::SystemTray::ShowItems()
#10 0x7ffb79c327f9 ash::SystemTray::ShowDefaultView()
#11 0x7ffb79c33871 ash::SystemTray::PerformAction()
#12 0x7ffb79c2e209 ash::ActionableView::ButtonPressed()
#13 0x7ffb7a1ea21d views::Button::OnMouseReleased()
#14 0x7ffb7a1d8669 views::InkDropHostView::OnMouseEvent()
#15 0x7ffb7b5b9482 ui::ScopedTargetHandler::OnEvent()
#16 0x7ffb7b5b6174 ui::EventDispatcher::ProcessEvent()
#17 0x7ffb7b5b5f13 ui::EventDispatcherDelegate::DispatchEvent()
#18 0x7ffb7a25d08d views::internal::RootView::OnMouseReleased()
#19 0x7ffb7a262c67 views::Widget::OnMouseEvent()
#20 0x7ffb7a281526 views::NativeWidgetAura::OnMouseEvent()
#21 0x7ffb7b5b6174 ui::EventDispatcher::ProcessEvent()
#22 0x7ffb7b5b5f13 ui::EventDispatcherDelegate::DispatchEvent()
#23 0x7ffb7b5b7712 ui::EventProcessor::OnEventFromSource()
#24 0x7ffb7b5b7d51 ui::EventSource::SendEventToSink()
#25 0x7ffb7b31a6ca aura::WindowTreeClient::OnWindowInputEvent()
#26 0x7ffb7b35fe86 ui::mojom::WindowTreeClientStubDispatch::Accept()
#27 0x7ffb7ec6caa6 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#28 0x7ffb7ec6c3e6 mojo::FilterChain::Accept()
#29 0x7ffb7ec6ddd5 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#30 0x7ffb7ec7453c mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#31 0x7ffb7ec73940 mojo::internal::MultiplexRouter::Accept()
#32 0x7ffb7ec6c3e6 mojo::FilterChain::Accept()
#33 0x7ffb7ec675c3 mojo::Connector::ReadSingleMessage()
#34 0x7ffb7ec67ff1 mojo::Connector::ReadAllAvailableMessages()
#35 0x7ffb7ec67e99 mojo::Connector::OnHandleReadyInternal()
#36 0x7ffb7ec68727 mojo::SimpleWatcher::DiscardReadyState()
#37 0x7ffb7ec30f26 mojo::SimpleWatcher::OnHandleReady()
#38 0x7ffb7ec31431 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE
#39 0x7ffb7f9be8e5 base::debug::TaskAnnotator::RunTask()
#40 0x7ffb7f9f1099 base::internal::IncomingTaskQueue::RunTask()
#41 0x7ffb7f9f4bdb base::MessageLoop::RunTask()
#42 0x7ffb7f9f4f6a base::MessageLoop::DeferOrRunPendingTask()
#43 0x7ffb7f9f51cc base::MessageLoop::DoWork()
#44 0x7ffb7f9f75f9 base::MessagePumpLibevent::Run()
#45 0x7ffb7f9f4509 base::MessageLoop::Run()
#46 0x7ffb7fa2b209 base::RunLoop::Run()
#47 0x5561e355e75a ChromeBrowserMainParts::MainMessageLoopRun()
#48 0x7ffb7cbd8b67 content::BrowserMainLoop::RunMainMessageLoopParts()
#49 0x7ffb7cbdbc66 content::BrowserMainRunnerImpl::Run()
#50 0x7ffb7cbd4df9 content::BrowserMain()
#51 0x7ffb7d5adf7d content::ContentMainRunnerImpl::Run()
#52 0x7ffb7ff03322 service_manager::Main()
#53 0x7ffb7d5ac5f4 content::ContentMain()
#54 0x5561e29d9133 ChromeMain
#55 0x7ffb7243f2b1 __libc_start_main
#56 0x5561e29d8faa _start

 

Comment 1 by msw@chromium.org, Apr 10 2018

Actually, I can't repro this on a fresh profile... I'll let you know if I hit it again.
perhaps something was wrong with my old profile or this is broken for pre-existing profiles?
Cc: steve...@chromium.org
I can repro this if I enable DCHECK and open the status area.

Looking in the code we access this from both src/chrome/browser/chromeos/accessibility/accessibility_manager.cc using Profile::GetPrefs(), however the pref is registered from Shell::RegisterUserProfilePrefs().

Cc: afakhry@chromium.org jamescook@chromium.org
Owner: warx@chromium.org
warx is working on a fix as part of relanding this CL https://chromium-review.googlesource.com/c/chromium/src/+/1018823

I don't know the right way to fix this, but it may be by registering the docked magnifier pref in chrome as a foreign pref.

Comment 4 by warx@chromium.org, Apr 25 2018

Status: Started (was: Assigned)
I would register docked magnifier pref on chrome and make it foreign in ash until ShouldShowAccessibilityMenu() doesn't live in chrome.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 25 2018

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

commit dcf11e3e2f37233268e354ad7cbc3ca368b5786d
Author: Qiang Xu <warx@google.com>
Date: Wed Apr 25 17:10:17 2018

reland: cros: remove DockedMagnifierClient

this cl changes:
- Move ash::prefs::kDockedMagnifierEnabled registration to chrome to
  avoid ShouldShowAccessibilityMenu() crash.

original changes:
The plan is to make MagnificationManager a class to make mojo calls to
ash for FullscreenMagnifier or DockedMagnifier when
- Focus changed in page.
- Corresponding feature is enabled.
This CL removes DockedMagnifierClient and merges it into
MagnificationManager.

TBR=dcheng@chromium.org

Bug: 817157,  831258 
Test: existing tests and played on device working fine.
Change-Id: Ie8346291f03cc50b1b2580489413c3e45f6e38c8
Reviewed-on: https://chromium-review.googlesource.com/1018823
Commit-Queue: Qiang Xu <warx@google.com>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552583}
Reviewed-on: https://chromium-review.googlesource.com/1027080
Reviewed-by: Qiang Xu <warx@google.com>
Cr-Commit-Position: refs/heads/master@{#553606}
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/magnifier/docked_magnifier_controller.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/magnifier/docked_magnifier_controller.h
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/magnifier/docked_magnifier_controller_unittest.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/magnifier/magnification_controller.h
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/magnifier/magnification_controller_unittest.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/public/interfaces/docked_magnifier_controller.mojom
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/ash/shell.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/chrome/browser/chromeos/accessibility/magnification_manager.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/chrome/browser/chromeos/accessibility/magnification_manager.h
[delete] https://crrev.com/540a25f935cbb605cdce15892cd2c6f7dd91f148/chrome/browser/chromeos/accessibility/magnification_manager_unittest.cc
[delete] https://crrev.com/540a25f935cbb605cdce15892cd2c6f7dd91f148/chrome/browser/chromeos/docked_magnifier/docked_magnifier_client.cc
[delete] https://crrev.com/540a25f935cbb605cdce15892cd2c6f7dd91f148/chrome/browser/chromeos/docked_magnifier/docked_magnifier_client.h
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/dcf11e3e2f37233268e354ad7cbc3ca368b5786d/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.h

Comment 6 by warx@chromium.org, Apr 25 2018

Status: Fixed (was: Started)

Sign in to add a comment