mash: Support the system display extension api for chrome://settings/display |
|||||||||||||||||
Issue descriptionToT r444462 Run chrome --mash --login-manager Login to a real account Browser crashes with stack below. My test account does not have an unusual extensions installed, just the ones you get by default with a new account. [12246:12246:0118/134142.746757:FATAL:shell.cc(196)] Check failed: instance_. #0 0x7fe73818458e base::debug::StackTrace::StackTrace() #1 0x7fe7381a623a logging::LogMessage::~LogMessage() #2 0x7fe7320c3391 ash::Shell::GetInstance() #3 0x7fe739fc408c extensions::DisplayInfoProviderChromeOS::GetAllDisplaysInfo() #4 0x7fe738f92f17 extensions::SystemDisplayGetInfoFunction::Run() #5 0x7fe738e7db4d ExtensionFunction::RunWithValidation() #6 0x7fe738e8054b extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal() #7 0x7fe738e7ffba extensions::ExtensionFunctionDispatcher::Dispatch() #8 0x7fe738e9f3a3 _ZN3IPC8MessageTI29ExtensionHostMsg_Request_MetaSt5tupleIJ31ExtensionHostMsg_Request_ParamsEEvE8DispatchIN10extensions28ExtensionWebContentsObserverES8_N7content15RenderFrameHostEMS8_FvPSA_RKS3_EEEbPKNS_7MessageEPT_PT0_PT1_T2_ #9 0x7fe738e9f2ec extensions::ExtensionWebContentsObserver::OnMessageReceived() #10 0x7fe739fb5ead extensions::ChromeExtensionWebContentsObserver::OnMessageReceived() #11 0x7fe735debdda content::WebContentsImpl::OnMessageReceived() #12 0x7fe735b0a4ec content::RenderFrameHostImpl::OnMessageReceived() #13 0x7fe735cde35b content::RenderProcessHostImpl::OnMessageReceived() #14 0x7fe736ca7ca5 IPC::ChannelProxy::Context::OnDispatchMessage() #15 0x7fe736caad2b _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEJ13scoped_refptrIS5_ES6_EEEFvvEE3RunEPNS0_13BindStateBaseE #16 0x7fe73818513e base::debug::TaskAnnotator::RunTask() #17 0x7fe7381b328d base::MessageLoop::RunTask() #18 0x7fe7381b3ba5 base::MessageLoop::DoWork() #19 0x7fe7381b6189 base::MessagePumpLibevent::Run() #20 0x7fe7381b3007 base::MessageLoop::RunHandler() #21 0x7fe7381e575f base::RunLoop::Run() #22 0x7fe7396f0609 ChromeBrowserMainParts::MainMessageLoopRun() #23 0x7fe7359f8909 content::BrowserMainLoop::RunMainMessageLoopParts() #24 0x7fe7359fb87a content::BrowserMainRunnerImpl::Run() #25 0x7fe7359f3c2e content::BrowserMain() #26 0x7fe7361dacec content::ContentMainRunnerImpl::Run() #27 0x7fe7361d97e0 content::ContentMain() #28 0x7fe738d96e90 ChromeMain #29 0x7fe72ebeff45 __libc_start_main #30 0x7fe738d96ced <unknown> Something probably just needs to be skipped on --mash. rjkroege, can you find an owner?
,
Feb 14 2017
This causes crashes when running the desktopui_MashLogin test on-device. It's caused by the "Smart Lock" extension. I can also repro by logging in with my jamescooktest2@gmail.com account. I'm going to try adding some IsRunningInMash() checks to get my test working, but this will need to get fixed for real in Q1.
,
Feb 14 2017
"Smart Lock" is a component app used for proximity auth. The app is loaded in the signin profile on the login screen and also in the user profile after login [1]. Not sure why it needs to call chrome.system.display.getInfo but we should fix the API for mash. [1]: https://cs.chromium.org/chromium/src/chrome/browser/signin/easy_unlock_service.cc?rcl=ea69b8739b1bf1a220ea541cca7886597c04bf0b&l=677
,
Feb 14 2017
,
Feb 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/850f6feb8d0dc1a39dcc8ac63a91918a5c72a71d commit 850f6feb8d0dc1a39dcc8ac63a91918a5c72a71d Author: jamescook <jamescook@chromium.org> Date: Wed Feb 15 00:09:48 2017 mash: Work around startup crash in extensions::DisplayInfoProviderChromeOS The "Smart Lock" extension queries display information on login. This causes a browser crash attempting to access ash::Shell to get the DisplayManager. The crash is blocking enabling our desktopui_MashLogin autotest. For now, just early-exit all the calls on mash. BUG= 682402 TEST=desktopui_MashLogin does not result in browser crash Review-Url: https://codereview.chromium.org/2692043006 Cr-Commit-Position: refs/heads/master@{#450522} [modify] https://crrev.com/850f6feb8d0dc1a39dcc8ac63a91918a5c72a71d/chrome/browser/extensions/display_info_provider_chromeos.cc
,
Feb 15 2017
Mitigation landed, back to rjkroege@ to find an owner. I suspect this isn't important for Q1.
,
Feb 21 2017
,
Dec 20 2017
,
Dec 20 2017
We may want to consider migrating this extension API (maybe with some cleanup/modification) to a mojo service that can live in Ash and provide an interface for extensions (including Settings) and Chrome.
,
Feb 26 2018
,
Mar 14 2018
,
Mar 20 2018
Hey Steven, you mentioned that you might take on some of the planning/work here.
,
Mar 20 2018
,
Mar 20 2018
Here is a doc describing the current relationship between various classes and directories supporting chrome.system.display: https://docs.google.com/drawings/d/1cIal7xCl9w0uJzEzPVTPjDyJpj9BTJHKInM4_rgtsgM/edit
,
Mar 20 2018
My context comes from Issue 806318 ; I'm trying to support display mirroring/unified in mash, and that's toggled by chrome://settings/display I suggested extending the AshDisplayController mojo api to support existing Chrome<->Ash patterns in DisplayInfoProviderChromeOS. Ash might need to defer to Mus (via an extended DisplayController mojo api), and we'd need to make DisplayInfoProvider support this async behavior. If I understand correctly, you're suggesting having the chrome.system.display extension Javascript / system_display.idl (or the settings WebUI JS itself) actually use some mojo interface to Ash or Mus directly? That seems like a better long-term goal, skipping layers of JS/Renderer<->Chrome<->Ash<->Mus/Viz indirection, but I'm not sure I'm ready to tackle that at the moment. Maybe using/extending the device service would help? That already has services/device/public/mojom/screen_orientation.mojom
,
Mar 20 2018
Possibly relevant bugs: issue 735078 and issue 823634 - it sounds like there's some screen orientation work going on with ARC++/exo/ash.
,
Mar 20 2018
So, the current relationship, outlined in the doc in comment #14 is: Settings JS -> chrome.system.display API -> system_display_api.cc -> DisplayInfoProviderChromeOS -> [ ui::DisplayManager, ui::display::Screen, ash::DisplayConfigurationController, etc ] What I think I would like to propose initially is: Settings JS -> chrome.system.display API -> system_display_api_chromeos.cc -> [display_controller.mojom] -> [ mus / ash servers] And eventually: Settings JS -> [display_controller.mojom] -> [ mus / ash servers] What I'm not clear on: a) Can/should we implement the entire interface in display_controller.mojom, and have it make further requests to ash where necessary? b) Should we implement a 'display_configuration.mojom' (or somesuch) API that is closer to the existing extension API and have that call out to display_controller.mojom as necessary? c) Should system_display_api_chromeos.cc (and eventually the settings JS) talk to both display_controller.mojom and 'display_configuration.mojom' (or whatever).
,
Mar 20 2018
,
Mar 29 2018
,
Mar 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a commit b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a Author: Steven Bennetts <stevenjb@chromium.org> Date: Fri Mar 30 18:17:56 2018 Make DisplayInfoProvider async In order to support mustash, DisplayInfoProvider needs to make asynchronous calls to Ash. To facilitate this, the DiplayInfoProvider API needs to be made asynchronous. (Note: the extension API that consumes DisplayInfoProvider is already async). NOTE: This CL removes the DisplayInfoProviderChromeOS error messages from the public namespace. They were only exposed for testing and the tests do not need to test the error message. When the implementation is migrated to use the mojo API, it will use enums instead which will be tested in the Ash (model) implementation. For minor changes to cast_display_info_provider.cc: TBR=halliwell@chromium.org Bug: 682402 Change-Id: I14020599258d07c48dcd31eb8d126c5f5e431bae Reviewed-on: https://chromium-review.googlesource.com/981202 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Toni Barzic <tbarzic@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Malay Keshav <malaykeshav@chromium.org> Commit-Queue: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#547211} [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_aura.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_aura.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_chromeos.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_chromeos.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_chromeos_unittest.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_mac.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_mac.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_win.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chrome/browser/extensions/display_info_provider_win.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chromecast/browser/extensions/cast_display_info_provider.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/chromecast/browser/extensions/cast_display_info_provider.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/browser/api/system_display/display_info_provider.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/browser/api/system_display/display_info_provider.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/browser/api/system_display/system_display_api.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/browser/api/system_display/system_display_api.h [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/browser/api/system_display/system_display_apitest.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/shell/browser/shell_display_info_provider.cc [modify] https://crrev.com/b1c3bdd27856b7aca2b6617c991e9bbf9d50e99a/extensions/shell/browser/shell_display_info_provider.h
,
Apr 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc commit 9c10bc49bf5df05271237b8d47f9ce95be8b6dcc Author: Steven Bennetts <stevenjb@chromium.org> Date: Mon Apr 02 17:02:04 2018 DisplayInfoProviderChromeOS: Cleanup Additional cleanup in preparation for moving the ash implementation to src/ash. Note: This CL simplifies and clarifies when and how DisplayProperties are applied. Bug: 682402 Change-Id: Ic89eefecbbb2c35b018d05861963044b46dad4c3 Reviewed-on: https://chromium-review.googlesource.com/983006 Commit-Queue: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Toni Barzic <tbarzic@chromium.org> Cr-Commit-Position: refs/heads/master@{#547454} [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/chrome/browser/extensions/display_info_provider_chromeos.cc [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/chrome/browser/extensions/display_info_provider_chromeos_unittest.cc [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/extensions/browser/api/system_display/display_info_provider.cc [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/extensions/browser/api/system_display/display_info_provider.h [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/extensions/common/api/system_display.idl [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/ui/display/display_layout.cc [modify] https://crrev.com/9c10bc49bf5df05271237b8d47f9ce95be8b6dcc/ui/display/display_layout.h
,
Apr 11 2018
,
Apr 11 2018
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a9b4384d2d478253ef22a803277ca0c19dee0f2 commit 2a9b4384d2d478253ef22a803277ca0c19dee0f2 Author: Steven Bennetts <stevenjb@chromium.org> Date: Thu Apr 26 23:41:19 2018 Introduce cros_display_config.mojom This includes the implementation in ash, but no clients (dependent CL will follow). Bug: 682402 Change-Id: I9fe82f915f2b2ddacc29c27f039a47bc8b8a043b Reviewed-on: https://chromium-review.googlesource.com/1011563 Commit-Queue: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Cr-Commit-Position: refs/heads/master@{#554187} [modify] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/BUILD.gn [add] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/display/cros_display_config.cc [add] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/display/cros_display_config.h [add] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/display/cros_display_config_unittest.cc [modify] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/manifest.json [modify] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/mojo_interface_factory.cc [modify] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/public/interfaces/BUILD.gn [add] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/public/interfaces/cros_display_config.mojom [modify] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/shell.cc [modify] https://crrev.com/2a9b4384d2d478253ef22a803277ca0c19dee0f2/ash/shell.h
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d114aae1625e5c8da36eaa3bc714053804ac163b commit d114aae1625e5c8da36eaa3bc714053804ac163b Author: Steven Bennetts <stevenjb@chromium.org> Date: Fri Apr 27 00:51:24 2018 DisplayInfoProviderChromeOS: Add is_unified test and cleanup Bug: 682402 Change-Id: I10a333e541a0de8552aac7b044dd1777fbc5b44b Reviewed-on: https://chromium-review.googlesource.com/1011416 Commit-Queue: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Toni Barzic <tbarzic@chromium.org> Cr-Commit-Position: refs/heads/master@{#554237} [modify] https://crrev.com/d114aae1625e5c8da36eaa3bc714053804ac163b/chrome/browser/extensions/system_display/display_info_provider_chromeos.cc [modify] https://crrev.com/d114aae1625e5c8da36eaa3bc714053804ac163b/chrome/browser/extensions/system_display/display_info_provider_chromeos_unittest.cc [modify] https://crrev.com/d114aae1625e5c8da36eaa3bc714053804ac163b/extensions/common/api/system_display.idl
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3 commit 108b8b2284dc25089504f7f09eaa3df5b7e7f6d3 Author: Steven Bennetts <stevenjb@chromium.org> Date: Wed May 02 01:43:00 2018 Implement DisplayInfoProviderChromeos using cros_display_config.mojom Bug: 682402 Change-Id: I6502a46e7dfa5293e8e5b29fefd44b3c3034db2c Reviewed-on: https://chromium-review.googlesource.com/1011547 Commit-Queue: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Toni Barzic <tbarzic@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#555280} [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/chrome/browser/extensions/system_display/display_info_provider_chromeos.cc [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/chrome/browser/extensions/system_display/display_info_provider_chromeos.h [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/chrome/browser/extensions/system_display/display_info_provider_chromeos_unittest.cc [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/extensions/browser/api/system_display/display_info_provider.cc [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/extensions/browser/api/system_display/display_info_provider.h [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/extensions/browser/api/system_display/system_display_api.cc [modify] https://crrev.com/108b8b2284dc25089504f7f09eaa3df5b7e7f6d3/extensions/common/api/system_display.idl
,
May 4 2018
|
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by xiy...@chromium.org
, Jan 18 2017