DCHECK in viz::HostFrameSinkManager::RegisterFrameSinkId when keyboard shortcut viewer app opened twice |
|||
Issue descriptionToT chromeos-linux r568038 1. Run --enable-features=KeyboardShortcutViewerApp 2. Hit Ctrl-Alt-/ 3. Dialog opens 4. Hit Ctrl-Alt-/ 5. Dialog closes 6. Hit Ctrl-Alt-/ [21766:21766:0619/144413.483309:FATAL:host_frame_sink_manager.cc(69)] Check failed: !data.IsFrameSinkRegistered(). #0 0x7f175c9b027c base::debug::StackTrace::StackTrace() #1 0x7f175c90030b logging::LogMessage::~LogMessage() #2 0x7f1756ff75bb viz::HostFrameSinkManager::RegisterFrameSinkId() #3 0x7f174d36454f ui::ws2::ClientRoot::RegisterVizEmbeddingSupport() #4 0x7f174d377a6d ui::ws2::WindowTree::NewTopLevelWindow() #5 0x7f174d38df2d ui::mojom::WindowTreeStubDispatch::Accept() #6 0x7f175cc35996 mojo::InterfaceEndpointClient::HandleValidatedMessage() #7 0x7f175cc352c6 mojo::FilterChain::Accept() #8 0x7f175cc36d25 mojo::InterfaceEndpointClient::HandleIncomingMessage() #9 0x7f175cc3d49c mojo::internal::MultiplexRouter::ProcessIncomingMessage() #10 0x7f175cc3c8a0 mojo::internal::MultiplexRouter::Accept() #11 0x7f175cc352c6 mojo::FilterChain::Accept() #12 0x7f175cc30513 mojo::Connector::ReadSingleMessage() #13 0x7f175cc30f51 mojo::Connector::ReadAllAvailableMessages() #14 0x7f175cc30df9 mojo::Connector::OnHandleReadyInternal() #15 0x7f175cc31647 mojo::SimpleWatcher::DiscardReadyState() #16 0x7f175cbf8594 mojo::SimpleWatcher::OnHandleReady() #17 0x7f175cbf8aa1 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE #18 0x7f175c8e3305 base::debug::TaskAnnotator::RunTask() #19 0x7f175c90cb59 base::internal::IncomingTaskQueue::RunTask() #20 0x7f175c91032b base::MessageLoop::RunTask() #21 0x7f175c9106ba base::MessageLoop::DeferOrRunPendingTask() #22 0x7f175c91091c base::MessageLoop::DoWork() #23 0x7f175c9d02f9 base::MessagePumpLibevent::Run() #24 0x7f175c90fd14 base::MessageLoop::Run() #25 0x7f175c942009 base::RunLoop::Run() #26 0x555d36cf37cb ChromeBrowserMainParts::MainMessageLoopRun() #27 0x7f175a413e97 content::BrowserMainLoop::RunMainMessageLoopParts() #28 0x7f175a416b26 content::BrowserMainRunnerImpl::Run() #29 0x7f175a410134 content::BrowserMain() #30 0x7f175aea89d3 content::RunBrowserProcessMain() #31 0x7f175aea9998 content::ContentMainRunnerImpl::Run() #32 0x7f175cc638cc service_manager::Main() #33 0x7f175aea79f4 content::ContentMain() #34 0x555d36108b03 ChromeMain #35 0x7f174e31b2b1 __libc_start_main #36 0x555d3610897a _start
,
Jun 20 2018
Issue 854742 has been merged into this issue.
,
Jun 20 2018
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/390aec8e345d15f309f5eb119eecdf874f3125a8 commit 390aec8e345d15f309f5eb119eecdf874f3125a8 Author: Mike Wasserman <msw@chromium.org> Date: Fri Jun 22 00:40:41 2018 ws: Decrement client ids assigned by WindowService in Classic Ash This should avoid WindowService and RenderProcessHost ids colliding. (RenderProcessHost uses incrementing process ids as Viz FrameSinkIds) (we can remove this once the WS assigns renderer's ids in Mash/OopAsh) Bug: 854368 Test: No DCHECKs or crashes launching KSV app on CrOS (Ctrl-Alt-/) Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: Ibac8817c1cfc9205e4bf40fa73735a6ec2aa5976 Reviewed-on: https://chromium-review.googlesource.com/1109262 Commit-Queue: Michael Wasserman <msw@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#569472} [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/ash/ws/window_service_owner.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws/window_tree_client_unittest.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/ids.h [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/test_change_tracker.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/window_service.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/window_service.h [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/window_service_unittest.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/window_tree_client_unittest.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/window_tree_test_helper.cc [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/services/ui/ws2/window_tree_test_helper.h [modify] https://crrev.com/390aec8e345d15f309f5eb119eecdf874f3125a8/ui/compositor/test/in_process_context_factory.cc
,
Jun 22 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by sky@chromium.org
, Jun 20 2018Owner: msw@chromium.org
This is happening because of conflicting FrameSinkIds. Specifically the WindowService assigns a unique id for each client. This id lives in WindowTree::client_id_ and is an ever increasing integer. The id is also used with Viz. Specifically the id is used in FrameSinkId::client_id. When ash is in process content uses viz and generates its own FrameSinkIds. This means the ids assigned by content may conflict with those from the WindowService. Content assigns ids from the process id [1]. Process ids are generated by way of ChildProcessHostImpl::GenerateChildProcessUniqueId (an ever increasing integer). The ids feed into FrameSinkIds in code such as RenderWidgetHostImpl [2] In talking with Sadrul there are two possibilities for addressing this: 1. Add a ClientId (uint32_t) to RenderProcessHost that is used for FrameSinkId::client_id instead of GetId(). This ClientId would be assigned by way of ContentBrowserClient. The default implementation would be: virtual uint32_t GenerateVizFrameSinkIdForProcess(uint32_t proposed_id) { return proposed_id; } Chrome's implementation would redirect to the windowservice (assuming in ash and not mash/oopash). The WindowService implementation would be ++next_window_id_; Getting this to work would require having ash create the WindowService early on (it's lazily created now). That should be a relatively easy to change WindowServiceOwner. This seems relatively straightforward. I'm mentioning the following for completeness, but I don't believe we can get option 2 to work. 2. Content is the one actually handling launching of the utility process. Content *may* be assigning ids for these processes too. If it is, it may be possible to feed this id into the WindowService so that when a client connects to the WindowService the WindowService knows the id to use. I spoke with Ken about this and he mentioned this isn't currently possible. There is an API to observe process launching from the ServiceManager, but it may be racy and it wouldn't get the same id content. So, I don't think this approach would work. [1] https://cs.chromium.org/chromium/src/content/public/browser/render_process_host.h?q=RenderProcessHost&sq=package:chromium&dr=CSs&l=222 [2] https://chromium.googlesource.com/chromium/src/+/master/content/browser/renderer_host/render_widget_host_impl.cc#382