OOPIF: Debug crash in EventHandler::scrollBox when scrolling |
||||
Issue descriptionVersion: 52.0.2717.0 OS: All What steps will reproduce the problem? (0) Start a debug build of Chrome with --site-per-process (1) Visit http://csreis.github.io/tests/cross-site-iframe.html (2) Click "Go cross-site (complex page)" to load the build waterfall in an OOPIF (3) Scroll in the iframe What is the expected output? The iframe should scroll. What do you see instead? We crash in EventHandler::scrollBox on ASSERT(m_frame->isMainFrame()); Note that Nick is not seeing this in a trunk build on Windows, but we may be sync'd to different revisions. Ken, can you take a look to see if it repros for you? ASSERTION FAILED: m_frame->isMainFrame() ../../third_party/WebKit/Source/core/input/EventHandler.cpp(644) : blink::ScrollResult blink::EventHandler::scrollBox(blink::LayoutBox *, blink::ScrollGranularity, const blink::FloatSize &, const blink::FloatPoint &, const blink::FloatSize &, bool *) Received signal 11 SEGV_MAPERR 0000fbadbeef #0 0x7f92ed135bbe base::debug::StackTrace::StackTrace() #1 0x7f92ed1356ff base::debug::(anonymous namespace)::StackDumpSignalHandler() #2 0x7f92daf64340 <unknown> #3 0x7f92d5a8eafe blink::EventHandler::scrollBox() #4 0x7f92d5a8f278 blink::EventHandler::physicalScroll() #5 0x7f92d5a990bc blink::EventHandler::handleGestureScrollUpdate() #6 0x7f92d5a961b0 blink::EventHandler::handleGestureScrollEvent() #7 0x7f92d5a95c84 blink::EventHandler::handleGestureEvent() #8 0x7f92e0e73477 blink::WebFrameWidgetImpl::handleGestureEvent() #9 0x7f92e0e05dbc blink::PageWidgetDelegate::handleInputEvent() #10 0x7f92e0e716ef blink::WebFrameWidgetImpl::handleInputEvent() #11 0x7f92e7f1391c content::RenderWidgetInputHandler::HandleInputEvent() #12 0x7f92e807b030 content::RenderWidget::OnHandleInputEvent() #13 0x7f92e7ef9f3d _ZN4base20DispatchToMethodImplIPN7content16IdleUserDetectorEMS2_FvPKN5blink13WebInputEventERKN2ui11LatencyInfoENS1_22InputEventDispatchTypeEEJS7_S9_SC_EJLm0ELm1ELm2EEEEvRKT_T0_RKSt5tupleIJDpT1_EENS_13IndexSequenceIJXspT2_EEEE #14 0x7f92e808a285 _ZN4base16DispatchToMethodIPN7content12RenderWidgetEMS2_FvPKN5blink13WebInputEventERKN2ui11LatencyInfoENS1_22InputEventDispatchTypeEEJS7_S9_SC_EEEvRKT_T0_RKSt5tupleIJDpT1_EE #15 0x7f92e808a22f _ZN3IPC16DispatchToMethodIN7content12RenderWidgetEMS2_FvPKN5blink13WebInputEventERKN2ui11LatencyInfoENS1_22InputEventDispatchTypeEEvSt5tupleIJS6_S8_SB_EEEEvPT_T0_PT1_RKT2_ #16 0x7f92e8083cd9 _ZN3IPC8MessageTI30InputMsg_HandleInputEvent_MetaSt5tupleIJPKN5blink13WebInputEventEN2ui11LatencyInfoEN7content22InputEventDispatchTypeEEEvE8DispatchINS9_12RenderWidgetESE_vMSE_FvS6_RKS8_SA_EEEbPKNS_7MessageEPT_PT0_PT1_T2_ #17 0x7f92e807a026 content::RenderWidget::OnMessageReceived() #18 0x7f92e4fac08b IPC::MessageRouter::RouteMessage() #19 0x7f92e601ea48 content::ChildThreadImpl::ChildThreadMessageRouter::RouteMessage() #20 0x7f92e4fac00e IPC::MessageRouter::OnMessageReceived() #21 0x7f92e6022074 content::ChildThreadImpl::OnMessageReceived() #22 0x7f92e4f720a8 IPC::ChannelProxy::Context::OnDispatchMessage() #23 0x7f92e4f76736 _ZN4base8internal15RunnableAdapterIMN3IPC12ChannelProxy7ContextEFvRKNS2_7MessageEEE3RunIS4_JS7_EEEvRK13scoped_refptrIT_EDpOT0_ #24 0x7f92e4f7664e _ZN4base8internal12InvokeHelperILb0EvNS0_15RunnableAdapterIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEEEE8MakeItSoIJRK13scoped_refptrIS5_ES8_EEEvSB_DpOT_ #25 0x7f92e4f765fd _ZN4base8internal7InvokerINS_13IndexSequenceIJLm0ELm1EEEENS0_9BindStateINS0_15RunnableAdapterIMN3IPC12ChannelProxy7ContextEFvRKNS6_7MessageEEEEFvPS8_SB_EJSF_SB_EEENS0_12InvokeHelperILb0EvSE_EEFvvEE3RunEPNS0_13BindStateBaseE #26 0x7f92ed115f6e base::Callback<>::Run() #27 0x7f92ed13b5ae base::debug::TaskAnnotator::RunTask() #28 0x7f92d8f51ca4 scheduler::TaskQueueManager::ProcessTaskFromWorkQueue() #29 0x7f92d8f4fc02 scheduler::TaskQueueManager::DoWork() #30 0x7f92d8f574fe _ZN4base8internal15RunnableAdapterIMN9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEE3RunIJRKS4_RKbEEEvPS3_DpOT_ #31 0x7f92d8f573fa _ZN4base8internal12InvokeHelperILb1EvNS0_15RunnableAdapterIMN9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEEEE8MakeItSoINS_7WeakPtrIS4_EEJRKS5_RKbEEEvS8_T_DpOT0_ #32 0x7f92d8f57378 _ZN4base8internal7InvokerINS_13IndexSequenceIJLm0ELm1ELm2EEEENS0_9BindStateINS0_15RunnableAdapterIMN9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEEEFvPS7_S8_bEJNS_7WeakPtrIS7_EERS8_bEEENS0_12InvokeHelperILb1EvSB_EEFvvEE3RunEPNS0_13BindStateBaseE #33 0x7f92ed115f6e base::Callback<>::Run() #34 0x7f92ed13b5ae base::debug::TaskAnnotator::RunTask() #35 0x7f92ed1b330c base::MessageLoop::RunTask() #36 0x7f92ed1b35a8 base::MessageLoop::DeferOrRunPendingTask() #37 0x7f92ed1b3772 base::MessageLoop::DoWork() #38 0x7f92ed1c5083 base::MessagePumpDefault::Run() #39 0x7f92ed1b2d1f base::MessageLoop::RunHandler() #40 0x7f92ed2593c4 base::RunLoop::Run() #41 0x7f92ed1b1d94 base::MessageLoop::Run() #42 0x7f92e80a3cb8 content::RendererMain() #43 0x7f92e8416036 content::RunZygote() #44 0x7f92e84162e0 content::RunNamedProcessTypeMain() #45 0x7f92e8417ee9 content::ContentMainRunnerImpl::Run() #46 0x7f92e8415702 content::ContentMain() #47 0x7f92edd135a0 ChromeMain #48 0x7f92edd13532 main #49 0x7f92d922aec5 __libc_start_main #50 0x7f92edd13435 <unknown>
,
Apr 27 2016
Yep, neither OOPIF nor non-OOPIF should reach there but the problem must lie where we register the viewport apply scroll callback in Document.cpp. I've got a CL in the commit queue right now that modifies that code significantly. If it still repros after that lands I'll take a closer look and put up a fix tomorrow.
,
Apr 29 2016
I just tried the repro in the original report and I don't get any crashes. Can someone confirm?
,
Apr 29 2016
Is the idea that this might have fixed it? https://codereview.chromium.org/1913843004/ I am building now with your change, but I have concerns with what is happening there. It looks like you are using Document::ownerElement() as a signal for whether this is the main frame's Document or not (ownerElement() => not main frame), but this is not an accurate generalization because I don't OOPIF Document's have ownerElements, which I interpreted to be the likely cause of this bug. The ownerElement situation isn't fully resolved, as is evident by the call to Frame::deprecatedLocalOwner() that returns nullptr for OOPIFs.
,
Apr 29 2016
Ok, I assumed ownerElement() would return non-null for an iframe document even in the OOPIF case. What's the correct way to check for that? FYI: there's plenty of code in event handling that uses that as an assumption.
,
Apr 29 2016
Hmm. There are several options: 1) Add a FrameOwner getter to Document, like Frame::owner() and fix the comment on Document::ownerElement(). 2) Alternatively, just use document().frame()->owner() for this particular case (but you might also need to check frame() is not null, which is unfortunate...)
,
Apr 29 2016
#5: "FYI: there's plenty of code in event handling that uses that as an assumption." I noticed that this morning. tbh I am surprised this hasn't resulted in a lot more bugs. dcheng's first suggestion matches what I had in mind. The second is a bit awkward given all the places this is used.
,
Apr 29 2016
Ok, I'll fix this with dcheng's suggestion and update the other usages shortly.
,
May 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f66e6b578fce9b4aed705f973acc547235e73e2c commit f66e6b578fce9b4aed705f973acc547235e73e2c Author: bokan <bokan@chromium.org> Date: Tue May 10 17:29:11 2016 Rename Document::ownerElement to localOwner and fix main frame checks. Document::ownerElement returns nullptr if the owner is in a remote frame so the new name is more clear of that. I've updated call sites that were using !ownerElement as a signal that the document was in the main frame to check the frame's isMainFrame method instead since that should work in OOPIF scenarios. BUG= 607001 Review-Url: https://codereview.chromium.org/1942623002 Cr-Commit-Position: refs/heads/master@{#392637} [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/dom/Element.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/dom/Fullscreen.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/dom/Node.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/frame/EventHandlerRegistry.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/html/ImageDocument.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/html/MediaDocument.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/html/PluginDocument.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/html/TextDocument.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/input/EventHandler.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/input/EventHandler.h [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/input/TouchActionUtil.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/input/TouchEventManager.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/inspector/InspectorResourceAgent.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/layout/LayoutView.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/loader/SinkDocument.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/page/TouchAdjustment.cpp [modify] https://crrev.com/f66e6b578fce9b4aed705f973acc547235e73e2c/third_party/WebKit/Source/core/paint/ViewPainter.cpp
,
May 16 2016
bokan@: Can this be marked fixed now? I don't see any of these crashes since 52.0.2733.0, which I think was the first Canary after your CL landed. (Thanks!)
,
May 16 2016
Sounds like bokan@ is out, so I'll mark this fixed. |
||||
►
Sign in to add a comment |
||||
Comment 1 by kenrb@chromium.org
, Apr 27 2016