SelectionController::passMousePressEventToSubframe() should call FrameSelection::contains() with clean layout tree |
||||
Issue descriptionChrome Version: 59.0.3037.0 (Developer Build) (64-bit) 611dfce737a7df5d5a1e88e8528d405ebed37b8d-refs/heads/master@{#455700} ** note this is a Debug build of Chromium OS: Win 7 What steps will reproduce the problem? (1) Open Gmail, try to start a Hangouts chat (2) Assertion failure What is the expected result? No assert. What happens instead? I get an assertion failure of assertSelectionValid() (SelectionEditor::updateCachedVisibleSelectionIfNeeded() Line 369) I'm also attaching a minidump from a Debug build, so it should hopefully be more tractable to figure out what's going on. This has been 100% reproducible for me so far. base.dll!base::debug::BreakDebugger() Line 21 C++ base.dll!logging::LogMessage::~LogMessage() Line 762 C++ > blink_core.dll!blink::SelectionEditor::updateCachedVisibleSelectionIfNeeded() Line 369 C++ blink_core.dll!blink::SelectionEditor::computeVisibleSelectionInFlatTree() Line 89 C++ blink_core.dll!blink::FrameSelection::computeVisibleSelectionInFlatTree() Line 129 C++ blink_core.dll!blink::FrameSelection::contains(const blink::LayoutPoint & point) Line 483 C++ blink_core.dll!blink::SelectionController::passMousePressEventToSubframe(const blink::EventWithHitTestResults<blink::WebMouseEvent> & mev) Line 1024 C++ blink_core.dll!blink::EventHandler::passMousePressEventToSubframe(blink::EventWithHitTestResults<blink::WebMouseEvent> & mev, blink::LocalFrame * subframe) Line 2065 C++ blink_core.dll!blink::EventHandler::handleMousePressEvent(const blink::WebMouseEvent & mouseEvent) Line 609 C++ blink_web.dll!blink::PageWidgetEventHandler::handleMouseDown(blink::LocalFrame & mainFrame, const blink::WebMouseEvent & event) Line 249 C++ blink_web.dll!blink::WebViewImpl::handleMouseDown(blink::LocalFrame & mainFrame, const blink::WebMouseEvent & event) Line 521 C++ blink_web.dll!blink::PageWidgetDelegate::handleInputEvent(blink::PageWidgetEventHandler & handler, const blink::WebCoalescedInputEvent & coalescedEvent, blink::LocalFrame * root) Line 163 C++ blink_web.dll!blink::WebViewImpl::handleInputEvent(const blink::WebCoalescedInputEvent & coalescedEvent) Line 2225 C++ blink_web.dll!blink::WebViewFrameWidget::handleInputEvent(const blink::WebCoalescedInputEvent & event) Line 89 C++ content.dll!content::RenderWidgetInputHandler::HandleInputEvent(const blink::WebCoalescedInputEvent & coalesced_event, const ui::LatencyInfo & latency_info, content::InputEventDispatchType dispatch_type) Line 312 C++ content.dll!content::RenderWidget::OnHandleInputEvent(const blink::WebInputEvent * input_event, const std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> > & coalesced_events, const ui::LatencyInfo & latency_info, content::InputEventDispatchType dispatch_type) Line 822 C++ content.dll!base::DispatchToMethodImpl<content::RenderWidget * __ptr64,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> > const & __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64,std::tuple<blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> >,ui::LatencyInfo,enum content::InputEventDispatchType> const & __ptr64,0,1,2,3>(content::RenderWidget * const & obj, void(content::RenderWidget::*)(const blink::WebInputEvent *, const std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> > &, const ui::LatencyInfo &, content::InputEventDispatchType) method, const std::tuple<blink::WebInputEvent const *,std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> >,ui::LatencyInfo,enum content::InputEventDispatchType> & args, base::IndexSequence<0,1,2,3> __formal) Line 92 C++ content.dll!base::DispatchToMethod<content::RenderWidget * __ptr64,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> > const & __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64,std::tuple<blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> >,ui::LatencyInfo,enum content::InputEventDispatchType> const & __ptr64>(content::RenderWidget * const & obj, void(content::RenderWidget::*)(const blink::WebInputEvent *, const std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> > &, const ui::LatencyInfo &, content::InputEventDispatchType) method, const std::tuple<blink::WebInputEvent const *,std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> >,ui::LatencyInfo,enum content::InputEventDispatchType> & args) Line 100 C++ content.dll!IPC::DispatchToMethod<content::RenderWidget,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> > const & __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64,void,std::tuple<blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> >,ui::LatencyInfo,enum content::InputEventDispatchType> >(content::RenderWidget * obj, void(content::RenderWidget::*)(const blink::WebInputEvent *, const std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> > &, const ui::LatencyInfo &, content::InputEventDispatchType) method, void * __formal, const std::tuple<blink::WebInputEvent const *,std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> >,ui::LatencyInfo,enum content::InputEventDispatchType> & tuple) Line 27 C++ content.dll!IPC::MessageT<InputMsg_HandleInputEvent_Meta,std::tuple<blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> >,ui::LatencyInfo,enum content::InputEventDispatchType>,void>::Dispatch<content::RenderWidget,content::RenderWidget,void,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,std::vector<blink::WebInputEvent const * __ptr64,std::allocator<blink::WebInputEvent const * __ptr64> > const & __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64>(const IPC::Message * msg, content::RenderWidget * obj, content::RenderWidget * sender, void * parameter, void(content::RenderWidget::*)(const blink::WebInputEvent *, const std::vector<blink::WebInputEvent const *,std::allocator<blink::WebInputEvent const *> > &, const ui::LatencyInfo &, content::InputEventDispatchType) func) Line 122 C++ content.dll!content::RenderWidget::OnMessageReceived(const IPC::Message & message) Line 605 C++ content.dll!content::RenderViewImpl::OnMessageReceived(const IPC::Message & message) Line 1290 C++ ipc.dll!IPC::MessageRouter::RouteMessage(const IPC::Message & msg) Line 57 C++ content.dll!content::ChildThreadImpl::ChildThreadMessageRouter::RouteMessage(const IPC::Message & msg) Line 347 C++ ipc.dll!IPC::MessageRouter::OnMessageReceived(const IPC::Message & msg) Line 49 C++ content.dll!content::ChildThreadImpl::OnMessageReceived(const IPC::Message & msg) Line 745 C++ content.dll!base::internal::FunctorTraits<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64,void>::Invoke<content::RenderThreadImpl * __ptr64,IPC::Message const & __ptr64>(bool(content::ChildThreadImpl::*)(const IPC::Message &) method, content::RenderThreadImpl * && receiver_ptr, const IPC::Message & <args_0>) Line 215 C++ content.dll!base::internal::FunctorTraits<base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64>,void>::Invoke<base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64> const & __ptr64,content::RenderThreadImpl * __ptr64,IPC::Message const & __ptr64>(const base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const &)> & ignore_result_helper, content::RenderThreadImpl * && <args_0>, const IPC::Message & <args_1>) Line 250 C++ content.dll!base::internal::InvokeHelper<0,void>::MakeItSo<base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64> const & __ptr64,content::RenderThreadImpl * __ptr64,IPC::Message const & __ptr64>(const base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const &)> & functor, content::RenderThreadImpl * && <args_0>, const IPC::Message & <args_1>) Line 287 C++ content.dll!base::internal::Invoker<base::internal::BindState<base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64>,base::internal::UnretainedWrapper<content::RenderThreadImpl> >,void __cdecl(IPC::Message const & __ptr64)>::RunImpl<base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64> const & __ptr64,std::tuple<base::internal::UnretainedWrapper<content::RenderThreadImpl> > const & __ptr64,0>(const base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const &)> & functor, const std::tuple<base::internal::UnretainedWrapper<content::RenderThreadImpl> > & bound, base::IndexSequence<0> __formal, const IPC::Message & <unbound_args_0>) Line 365 C++ content.dll!base::internal::Invoker<base::internal::BindState<base::internal::IgnoreResultHelper<bool (__cdecl content::ChildThreadImpl::*)(IPC::Message const & __ptr64) __ptr64>,base::internal::UnretainedWrapper<content::RenderThreadImpl> >,void __cdecl(IPC::Message const & __ptr64)>::Run(base::internal::BindStateBase * base, const IPC::Message & <unbound_args_0>) Line 343 C++ content.dll!base::internal::RunMixin<base::Callback<void __cdecl(IPC::Message const & __ptr64),1,1> >::Run(const IPC::Message & <args_0>) Line 86 C++ content.dll!base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::Forward(const IPC::Message & <args_0>) Line 111 C++ content.dll!base::internal::FunctorTraits<void (__cdecl base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::*)(IPC::Message const & __ptr64)const __ptr64,void>::Invoke<base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)> > const & __ptr64,IPC::Message const & __ptr64>(void(const base::CancelableCallback<void __cdecl(IPC::Message const &)>::*)(const IPC::Message &) method, const base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const &)> > & receiver_ptr, const IPC::Message & <args_0>) Line 235 C++ content.dll!base::internal::InvokeHelper<1,void>::MakeItSo<void (__cdecl base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::*const & __ptr64)(IPC::Message const & __ptr64)const __ptr64,base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)> > const & __ptr64,IPC::Message const & __ptr64>(void(const base::CancelableCallback<void __cdecl(IPC::Message const &)>::*)(const IPC::Message &) & functor, const base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const &)> > & weak_ptr, const IPC::Message & <args_0>) Line 308 C++ content.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::*)(IPC::Message const & __ptr64)const __ptr64,base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)> > >,void __cdecl(IPC::Message const & __ptr64)>::RunImpl<void (__cdecl base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::*const & __ptr64)(IPC::Message const & __ptr64)const __ptr64,std::tuple<base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)> > > const & __ptr64,0>(void(const base::CancelableCallback<void __cdecl(IPC::Message const &)>::*)(const IPC::Message &) & functor, const std::tuple<base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const &)> > > & bound, base::IndexSequence<0> __formal, const IPC::Message & <unbound_args_0>) Line 365 C++ content.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::*)(IPC::Message const & __ptr64)const __ptr64,base::WeakPtr<base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)> > >,void __cdecl(IPC::Message const & __ptr64)>::Run(base::internal::BindStateBase * base, const IPC::Message & <unbound_args_0>) Line 343 C++ content.dll!base::internal::RunMixin<base::Callback<void __cdecl(IPC::Message const & __ptr64),1,1> >::Run(const IPC::Message & <args_0>) Line 86 C++ content.dll!content::InputEventFilter::HandleEventOnMainThread(int routing_id, const blink::WebCoalescedInputEvent * event, const ui::LatencyInfo & latency_info, content::InputEventDispatchType dispatch_type) Line 302 C++ content.dll!content::MainThreadEventQueue::DispatchInFlightEvent() Line 247 C++ content.dll!content::MainThreadEventQueue::DispatchSingleEvent() Line 281 C++ content.dll!base::internal::FunctorTraits<void (__cdecl content::MainThreadEventQueue::*)(void) __ptr64,void>::Invoke<scoped_refptr<content::MainThreadEventQueue> const & __ptr64>(void(content::MainThreadEventQueue::*)() method, const scoped_refptr<content::MainThreadEventQueue> & receiver_ptr) Line 215 C++ content.dll!base::internal::InvokeHelper<0,void>::MakeItSo<void (__cdecl content::MainThreadEventQueue::*const & __ptr64)(void) __ptr64,scoped_refptr<content::MainThreadEventQueue> const & __ptr64>(void(content::MainThreadEventQueue::*)() & functor, const scoped_refptr<content::MainThreadEventQueue> & <args_0>) Line 287 C++ content.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl content::MainThreadEventQueue::*)(void) __ptr64,scoped_refptr<content::MainThreadEventQueue> >,void __cdecl(void)>::RunImpl<void (__cdecl content::MainThreadEventQueue::*const & __ptr64)(void) __ptr64,std::tuple<scoped_refptr<content::MainThreadEventQueue> > const & __ptr64,0>(void(content::MainThreadEventQueue::*)() & functor, const std::tuple<scoped_refptr<content::MainThreadEventQueue> > & bound, base::IndexSequence<0> __formal) Line 365 C++ content.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl content::MainThreadEventQueue::*)(void) __ptr64,scoped_refptr<content::MainThreadEventQueue> >,void __cdecl(void)>::Run(base::internal::BindStateBase * base) Line 343 C++ base.dll!base::internal::RunMixin<base::Callback<void __cdecl(void),0,0> >::Run() Line 68 C++ base.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, base::PendingTask * pending_task) Line 61 C++ blink_platform.dll!blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue(blink::scheduler::internal::WorkQueue * work_queue, bool is_nested, blink::scheduler::LazyNow time_before_task, base::TimeTicks * time_after_task) Line 536 C++ blink_platform.dll!blink::scheduler::TaskQueueManager::DoWork(bool delayed) Line 331 C++ blink_platform.dll!base::internal::FunctorTraits<void (__cdecl blink::scheduler::TaskQueueManager::*)(bool) __ptr64,void>::Invoke<base::WeakPtr<blink::scheduler::TaskQueueManager> const & __ptr64,bool const & __ptr64>(void(blink::scheduler::TaskQueueManager::*)(bool) method, const base::WeakPtr<blink::scheduler::TaskQueueManager> & receiver_ptr, const bool & <args_0>) Line 215 C++ blink_platform.dll!base::internal::InvokeHelper<1,void>::MakeItSo<void (__cdecl blink::scheduler::TaskQueueManager::*const & __ptr64)(bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager> const & __ptr64,bool const & __ptr64>(void(blink::scheduler::TaskQueueManager::*)(bool) & functor, const base::WeakPtr<blink::scheduler::TaskQueueManager> & weak_ptr, const bool & <args_0>) Line 308 C++ blink_platform.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl blink::scheduler::TaskQueueManager::*)(bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager>,bool>,void __cdecl(void)>::RunImpl<void (__cdecl blink::scheduler::TaskQueueManager::*const & __ptr64)(bool) __ptr64,std::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>,bool> const & __ptr64,0,1>(void(blink::scheduler::TaskQueueManager::*)(bool) & functor, const std::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>,bool> & bound, base::IndexSequence<0,1> __formal) Line 365 C++ blink_platform.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl blink::scheduler::TaskQueueManager::*)(bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager>,bool>,void __cdecl(void)>::Run(base::internal::BindStateBase * base) Line 343 C++ base.dll!base::internal::RunMixin<base::Callback<void __cdecl(void),0,0> >::Run() Line 68 C++ base.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, base::PendingTask * pending_task) Line 61 C++ base.dll!base::MessageLoop::RunTask(base::PendingTask * pending_task) Line 424 C++ base.dll!base::MessageLoop::DeferOrRunPendingTask(base::PendingTask pending_task) Line 437 C++ base.dll!base::MessageLoop::DoWork() Line 527 C++ base.dll!base::MessagePumpDefault::Run(base::MessagePump::Delegate * delegate) Line 33 C++ base.dll!base::MessageLoop::RunHandler() Line 388 C++ base.dll!base::RunLoop::Run() Line 38 C++ content.dll!content::RendererMain(const content::MainFunctionParams & parameters) Line 200 C++ content.dll!content::RunNamedProcessTypeMain(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & process_type, const content::MainFunctionParams & main_function_params, content::ContentMainDelegate * delegate) Line 491 C++ content.dll!content::ContentMainRunnerImpl::Run() Line 836 C++ content.dll!content::ContentMain(const content::ContentMainParams & params) Line 20 C++ chrome.dll!ChromeMain(HINSTANCE__ * instance, sandbox::SandboxInterfaceInfo * sandbox_info, __int64 exe_entry_point_ticks) Line 121 C++ chrome.exe!MainDllLoader::Launch(HINSTANCE__ * instance, base::TimeTicks exe_entry_point_ticks) Line 201 C++ chrome.exe!wWinMain(HINSTANCE__ * instance, HINSTANCE__ * prev, wchar_t * __formal, int __formal) Line 283 C++ [External Code]
,
Mar 10 2017
,
Mar 10 2017
We should move updateStyleAndLayout*() call in L1029 before L1024 before calling of FS::contain().
1016 void SelectionController::passMousePressEventToSubframe(
1017 const MouseEventWithHitTestResults& mev) {
1018 // If we're clicking into a frame that is selected, the frame will appear
1019 // greyed out even though we're clicking on the selection. This looks
1020 // really strange (having the whole frame be greyed out), so we deselect the
1021 // selection.
1022 IntPoint p = m_frame->view()->rootFrameToContents(
1023 flooredIntPoint(mev.event().positionInRootFrame()));
1024 if (!selection().contains(p))
1025 return;
1026
1027 // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
1028 // needs to be audited. See http://crbug.com/590369 for more details.
1029 m_frame->document()->updateStyleAndLayoutIgnorePendingStylesheets();
,
Mar 10 2017
I'm going to fix it
,
Mar 11 2017
Minimized repro:
<!doctype html>
<style>
#frame:active {
border-width: 5px;
}
</style>
<iframe id=frame></iframe>
<script>
const frameHTML = [
'<style>',
'#text:active {',
'color:red;',
'}',
'</style>',
'<div id=text>foo</div>'
].join('');
document.getElementById('frame').setAttribute('srcdoc', frameHTML);
</script>
Clicking at the the text in the subframe crashes
,
Mar 11 2017
I thought this call site of FrameSelection::contain() already has clean layout because it's called after a hit test: EventHandler::handleMousePressEvent() calls Document::performMouseEventHitTest() before entering SelectionController::passMousePressEventToSubframe() I didn't notice that Document::performMouseEventHitTest() invalidates style by calling Document::updateHoverActiveState...
,
Mar 11 2017
The test case in #6 can be even more minimized: the subframe can be empty.
,
Mar 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ebefd65d9e7285db1b4f85ef732df331f3f23fd1 commit ebefd65d9e7285db1b4f85ef732df331f3f23fd1 Author: xiaochengh <xiaochengh@chromium.org> Date: Mon Mar 13 01:06:48 2017 Ensure clean layout for a call site of FrameSelection::contains() When calling FrameSelection::contains() from SelectionController::passMousePressEventToSubframe(), the layout can be dirty due to style changes performed in Document::updateHoverActiveState(). Hence, this patch updates layout for this call site. BUG= 700248 TEST=hittesting/subframe_active_crash.html Review-Url: https://codereview.chromium.org/2747463003 Cr-Commit-Position: refs/heads/master@{#456313} [add] https://crrev.com/ebefd65d9e7285db1b4f85ef732df331f3f23fd1/third_party/WebKit/LayoutTests/hittesting/subframe_active_crash.html [modify] https://crrev.com/ebefd65d9e7285db1b4f85ef732df331f3f23fd1/third_party/WebKit/Source/core/editing/SelectionController.cpp
,
Mar 14 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by mikelawther@chromium.org
, Mar 10 2017