Changing window focus in PDF Viewer triggers a DCHECK |
|||||
Issue descriptionChromium 65.0.3303.0 (Developer Build) (64-bit) Revision 525e4537ba0e2f4e3331a38fb35b646a4bae3b51-refs/heads/master@{#526151} OS: Linux What steps will reproduce the problem? (1) Open a .pdf with Chromium (2) Change focus to another window (3) Change focus back to Chromium What is the expected result? The window gets focus back normally. What happens instead? DCHECK with stack: [1:1:0103/144211.523105:FATAL:WebViewImpl.cpp(2062)] Check failed: false. #0 0x7faaf98127dd base::debug::StackTrace::StackTrace() #1 0x7faaf9810c1c base::debug::StackTrace::StackTrace() #2 0x7faaf9897c0a logging::LogMessage::~LogMessage() #3 0x7faae7e25a12 blink::WebViewImpl::HandleInputEvent() #4 0x7faae7f9372c blink::WebViewFrameWidget::HandleInputEvent() #5 0x7faaf4f9192f content::RenderWidgetInputHandler::HandleInputEvent() #6 0x7faaf5133b0b content::RenderWidget::HandleInputEvent() #7 0x7faaf5123eae content::RenderViewImpl::HandleInputEvent() #8 0x7faaf4f8ceef content::MainThreadEventQueue::HandleEventOnMainThread() #9 0x7faaf4f8dd6d content::QueuedWebInputEvent::Dispatch() #10 0x7faaf4f8c51b content::MainThreadEventQueue::DispatchEvents() #11 0x7faaf2be231f _ZN4base8internal13FunctorTraitsIMN7content28ServiceManagerConnectionImpl15IOThreadContextEFvvEvE6InvokeI13scoped_refptrIS4_EJEEEvS6_OT_DpOT0_ #12 0x7faaf2be2294 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMN7content28ServiceManagerConnectionImpl15IOThreadContextEFvvEJ13scoped_refptrIS6_EEEEvOT_DpOT0_ #13 0x7faaf2be2240 _ZN4base8internal7InvokerINS0_9BindStateIMN7content28ServiceManagerConnectionImpl15IOThreadContextEFvvEJ13scoped_refptrIS5_EEEEFvvEE7RunImplIS7_NSt3__15tupleIJS9_EEEJLm0EEEEvOT_OT0_NSE_16integer_sequenceImJXspT1_EEEE #14 0x7faaf2be2199 _ZN4base8internal7InvokerINS0_9BindStateIMN7content28ServiceManagerConnectionImpl15IOThreadContextEFvvEJ13scoped_refptrIS5_EEEEFvvEE7RunOnceEPNS0_13BindStateBaseE #15 0x7faaf97bcf41 _ZNO4base12OnceCallbackIFvvEE3RunEv Bisect points at https://chromium-review.googlesource.com/c/chromium/src/+/838626 as the culprit. This bug makes testing the PDF viewer a bit hard because it crashes so much.
,
Jan 3 2018
It looks like the browser is now sending a mouse-enter event to the renderer, where it previously did not. My understanding is that we typically do not dispatch mouse-enter events from browser, and instead generate mouse-move events and send that to renderer. So this seems a bit odd. dtapuska@/wjmaclean@: any ideas? I am handing off to wjmaclean@ for now since I can't take a look right now. But I can start looking at it later tonight if there's no fix by then.
,
Jan 3 2018
,
Jan 3 2018
I believe kenrb@ added code to add enter/leave events here: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=572&gs=kythe%253A%252F%252Fchromium%253Flang%253Dc%25252B%25252B%253Fpath%253Dsrc%252Fcontent%252Fbrowser%252Frenderer_host%252Frender_widget_host_input_event_router.cc%2523H2vauXXX%25252FsiejmEZez%25252F%25252FXkRy0zpireb9PllTgPSCo1Q%25253D&gsn=SendMouseEnterOrLeaveEvents&ct=xref_usages
,
Jan 3 2018
Right. But in there, I see we send a Leave event [1], and Move events [2, 3], but no explicit Enter event. [1] https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=652 [2] https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=668 [3] https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=682
,
Jan 3 2018
I have to leave shortly to catch a train, but will try to take a look before I go ...
,
Jan 3 2018
To supplement c#2: On linux, we create mouse move events from EnterNotify X events to match Windows. https://cs.chromium.org/chromium/src/ui/events/x/events_x_utils.cc?rcl=95245d4c4061fdee5fcec5d70b05e9aa3045e5e6&l=380
,
Jan 3 2018
Re c#5 ... ahh, ok ... I didn't look carefully enough, and relied on the function name ;-) But the change in focus is a bit of a clue ... I wonder if some other source is sending them as part of the focus switch ... shouldn't be too hard to find where they're coming from ...
,
Jan 3 2018
The kMouseEnter is being sent via: https://cs.chromium.org/chromium/src/content/browser/frame_host/render_widget_host_view_guest.cc?type=cs&l=758 Here is an explicit crash I added when a kMouseEnter was sent to the renderer... 1749:1749:0103/165808.320978:FATAL:input_router_impl.cc(295)] Check failed: mouse_event.event.GetType() != blink::WebInputEvent::kMouseEnter. #0 0x5623c119fc6c base::debug::StackTrace::StackTrace() #1 0x5623c11bf91c logging::LogMessage::~LogMessage() #2 0x5623bfa1d117 content::InputRouterImpl::SendMouseEventImmediately() #3 0x5623bfab538c content::RenderWidgetHostImpl::ForwardMouseEventWithLatencyInfo() #4 0x5623bfab50ea content::RenderWidgetHostImpl::ForwardMouseEvent() #5 0x5623bf8c022c content::RenderWidgetHostViewGuest::OnHandleInputEvent() #6 0x5623bf8bfdc7 _ZN3IPC8MessageTI42BrowserPluginHostMsg_HandleInputEvent_MetaNSt3__15tupleIJiPKN5blink13WebInputEventEEEEvE8DispatchIN7content25RenderWidgetHostViewGuestESC_NSB_20RenderWidgetHostImplEMSC_FvPSD_iS7_EEEbPKNS_7MessageEPT_PT0_PT1_T2_ #7 0x5623bf8bfd09 content::RenderWidgetHostViewGuest::OnMessageReceivedFromEmbedder() #8 0x5623bf727203 content::BrowserPluginGuest::OnMessageReceivedFromEmbedder() #9 0x5623bf72c7a7 content::BrowserPluginMessageFilter::ForwardMessageToGuest() #10 0x5623bf72c604 content::BrowserPluginMessageFilter::OnMessageReceived() #11 0x5623bf621cd9 content::BrowserMessageFilter::Internal::DispatchMessage() #12 0x5623bf621e08 _ZN4base8internal7InvokerINS0_9BindStateIMN7content8protocol12_GLOBAL__N_115CookieRetrieverEFvRKNSt3__16vectorIN3net15CanonicalCookieENS7_9allocatorISA_EEEEEJ13scoped_refptrIS6_ESD_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE #13 0x5623c11a126a base::debug::TaskAnnotator::RunTask() #14 0x5623c11c71c6 base::internal::IncomingTaskQueue::RunTask() #15 0x5623c11c5637 base::MessageLoop::RunTask() #16 0x5623c11c5a54 base::MessageLoop::DeferOrRunPendingTask() #17 0x5623c11c5d18 base::MessageLoop::DoWork() #18 0x5623c11c9e0f base::(anonymous namespace)::WorkSourceDispatch() #19 0x7fb3002ed7f7 g_main_context_dispatch #20 0x7fb3002eda60 <unknown> #21 0x7fb3002edb0c g_main_context_iteration #22 0x5623c11c9bc2 base::MessagePumpGlib::Run() #23 0x5623c11c4e8c base::MessageLoop::Run() #24 0x5623c11f5296 base::RunLoop::Run() #25 0x5623c0e1ebc7 ChromeBrowserMainParts::MainMessageLoopRun() #26 0x5623bf721bc7 content::BrowserMainLoop::RunMainMessageLoopParts() #27 0x5623bf7253b3 content::BrowserMainRunnerImpl::Run() #28 0x5623bf71da6a content::BrowserMain() #29 0x5623c0deed3b content::RunNamedProcessTypeMain() #30 0x5623c0def816 content::ContentMainRunnerImpl::Run() #31 0x5623c0dfc734 service_manager::Main() #32 0x5623c0dee081 content::ContentMain() #33 0x5623becc61cc ChromeMain #34 0x7fb2fc4812b1 __libc_start_main #35 0x5623becc602a _start Are sadrul@'s changes ok for resending embedded input back?
,
Jan 3 2018
I published a patch here: https://chromium-review.googlesource.com/c/chromium/src/+/847939 to avoid the NOTREACHED statements. Seems to work ok. Sending it to the trybots.
,
Jan 3 2018
I notice here https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/events/WebInputEventConversion.cpp?rcl=e0fe27a0124cb198feabbd52605bc769cfa7d056&l=202 we're creating WebInputEvent::kMouseEnter out of EventTypeNames::mouseovers. Changing this to kMouseMove also seems to fix the issue.
,
Jan 4 2018
Since this seems specific to kMouseEnter being forwarded from the embedder via BrowserPlugin, would it make sense to instead change the type to kMouseOver in RenderWidgetHostViewGuest before sending it to the guest's embedder? That way we don't 'normalize' the idea of the renderer receiving these (just in case that could be problematic down the road). We're definitely getting closer to BrowserPlugin going away, so the fix would disappear with the problem in that case.
,
Jan 4 2018
From an IPC point of view I believe my change is a little more correct in that since kMouseEnter is allowed to be sent from the browser->renderer (and other code paths deal with it) we should handle it.
,
Jan 4 2018
Ok, as long this is something the renderer should be handling anyways then the fix should be in the renderer.
,
Jan 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e9db95192c09f00e9f8e0c6878fae978748ffcca commit e9db95192c09f00e9f8e0c6878fae978748ffcca Author: Dave Tapuska <dtapuska@chromium.org> Date: Thu Jan 04 17:44:29 2018 Fix crash with renderer receiving kMouseEnter and a mouse locked target. Convert kMouseEnter to mouseover event. BUG= 798810 Change-Id: Ie62a45df72536d611f9a38c5bef2d6f9a84a449a Reviewed-on: https://chromium-review.googlesource.com/847939 Commit-Queue: James MacLean <wjmaclean@chromium.org> Reviewed-by: James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#527027} [modify] https://crrev.com/e9db95192c09f00e9f8e0c6878fae978748ffcca/third_party/WebKit/Source/core/exported/WebViewImpl.cpp [modify] https://crrev.com/e9db95192c09f00e9f8e0c6878fae978748ffcca/third_party/WebKit/Source/core/frame/WebFrameWidgetImpl.cpp
,
Jan 4 2018
,
Jan 4 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by hnakashima@chromium.org
, Jan 3 2018