Issue 641403 link

Starred by 2 users

Issue metadata

Status:	Fixed
Owner:	yosin@chromium.org
Closed:	Aug 2016
Cc:	editing-dev@chromium.org
Components:	Blink>Editing
EstimatedDays:	----
NextAction:	----
OS:	Windows
Pri:	1
Type:	Bug
M-54 Hotlist-Merge-Approved merge-merged-2840

EventHandler::sendContextMenuEvent() calls SelectionController::sendContextMenuEvent() with dirty layout tree

Project Member Reported by lfg@chromium.org, Aug 26 2016

Issue description

Version: ToT (c062afd5b208ce6767978148ee84e99d01487309)
OS: Windows (debug build)

What steps will reproduce the problem?
(1) open new tab page
(2) right click on the page
(3) DCHECK(!needsLayoutTreeUpdate(position)) triggers

Comment 1 by yosin@chromium.org, Aug 29 2016

Status: Available (was: Untriaged)

Stack trace:
base.dll!base::debug::BreakDebugger() Line 21
base.dll!logging::LogMessage::~LogMessage() Line 751
blink_core.dll!blink::isEditablePosition(const blink::PositionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> > & position) Line 359
blink_core.dll!blink::VisibleSelectionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> >::isContentEditable() Line 712
blink_core.dll!blink::FrameSelection::isContentEditable() Line 107
blink_core.dll!blink::SelectionController::sendContextMenuEvent(const blink::EventWithHitTestResults<blink::PlatformMouseEvent> & mev, const blink::LayoutPoint & position) Line 605
blink_core.dll!blink::EventHandler::sendContextMenuEvent(const blink::PlatformMouseEvent & event, blink::Node * overrideTargetNode) Line 2047
blink_web.dll!blink::WebViewImpl::mouseContextMenu(const blink::WebMouseEvent & event) Line 620
blink_web.dll!blink::WebViewImpl::handleMouseUp(blink::LocalFrame & mainFrame, const blink::WebMouseEvent & event) Line 635
blink_web.dll!blink::PageWidgetDelegate::handleInputEvent(blink::PageWidgetEventHandler & handler, const blink::WebInputEvent & event, blink::LocalFrame * root) Line 153
blink_web.dll!blink::WebViewImpl::handleInputEvent(const blink::WebInputEvent & inputEvent) Line 2221
content.dll!content::RenderWidgetInputHandler::HandleInputEvent(const blink::WebInputEvent & input_event, const ui::LatencyInfo & latency_info, content::InputEventDispatchType dispatch_type) Line 325
content.dll!content::RenderWidget::OnHandleInputEvent(const blink::WebInputEvent * input_event, const ui::LatencyInfo & latency_info, content::InputEventDispatchType dispatch_type) Line 676
content.dll!base::DispatchToMethodImpl<content::RenderWidget * __ptr64,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64,std::tuple<blink::WebInputEvent const * __ptr64,ui::LatencyInfo,enum content::InputEventDispatchType> const & __ptr64,0,1,2>(content::RenderWidget * const & obj, void(content::RenderWidget::*)(const blink::WebInputEvent *, const ui::LatencyInfo &, content::InputEventDispatchType) method, const std::tuple<blink::WebInputEvent const *,ui::LatencyInfo,enum content::InputEventDispatchType> & args, base::IndexSequence<0,1,2> __formal) Line 145
content.dll!base::DispatchToMethod<content::RenderWidget * __ptr64,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64,std::tuple<blink::WebInputEvent const * __ptr64,ui::LatencyInfo,enum content::InputEventDispatchType> const & __ptr64>(content::RenderWidget * const & obj, void(content::RenderWidget::*)(const blink::WebInputEvent *, const ui::LatencyInfo &, content::InputEventDispatchType) method, const std::tuple<blink::WebInputEvent const *,ui::LatencyInfo,enum content::InputEventDispatchType> & args) Line 153
content.dll!IPC::DispatchToMethod<content::RenderWidget,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent const * __ptr64,ui::LatencyInfo const & __ptr64,enum content::InputEventDispatchType) __ptr64,void,std::tuple<blink::WebInputEvent const * __ptr64,ui::LatencyInfo,enum content::InputEventDispatchType> >(content::RenderWidget * obj, void(content::RenderWidget::*)(const blink::WebInputEvent *, const ui::LatencyInfo &, content::InputEventDispatchType) method, void * __formal, const std::tuple<blink::WebInputEvent const *,ui::LatencyInfo,enum content::InputEventDispatchType> & tuple) Line 27
content.dll!IPC::MessageT<InputMsg_HandleInputEvent_Meta,std::tuple<blink::WebInputEvent const * __ptr64,ui::LatencyInfo,enum content::InputEventDispatchType>,void>::Dispatch<content::RenderWidget,content::RenderWidget,void,void (__cdecl content::RenderWidget::*)(blink::WebInputEvent 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 ui::LatencyInfo &, content::InputEventDispatchType) func) Line 122
content.dll!content::RenderWidget::OnMessageReceived(const IPC::Message & message) Line 473
content.dll!content::RenderViewImpl::OnMessageReceived(const IPC::Message & message) Line 1377
ipc.dll!IPC::MessageRouter::RouteMessage(const IPC::Message & msg) Line 53
content.dll!content::ChildThreadImpl::ChildThreadMessageRouter::RouteMessage(const IPC::Message & msg) Line 369
ipc.dll!IPC::MessageRouter::OnMessageReceived(const IPC::Message & msg) Line 45
content.dll!content::ChildThreadImpl::OnMessageReceived(const IPC::Message & msg) Line 768
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
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 249
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 285
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 350
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 328
content.dll!base::Callback<void __cdecl(IPC::Message const & __ptr64),1>::Run(const IPC::Message & <args_0>) Line 389
content.dll!base::CancelableCallback<void __cdecl(IPC::Message const & __ptr64)>::Forward(const IPC::Message & <args_0>) Line 109
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
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 306
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 350
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 328
content.dll!base::Callback<void __cdecl(IPC::Message const & __ptr64),1>::Run(const IPC::Message & <args_0>) Line 389
content.dll!content::InputEventFilter::HandleEventOnMainThread(int routing_id, const blink::WebInputEvent * event, const ui::LatencyInfo & latency_info, content::InputEventDispatchType dispatch_type) Line 261
content.dll!content::MainThreadEventQueue::PopEventOnMainThread() Line 127
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
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 285
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 350
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 328
base.dll!base::Callback<void __cdecl(void),1>::Run() Line 389
base.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, const base::PendingTask & pending_task) Line 56
blink_platform.dll!blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue(blink::scheduler::internal::WorkQueue * work_queue, blink::scheduler::internal::TaskQueueImpl::Task * out_previous_task) Line 319
blink_platform.dll!blink::scheduler::TaskQueueManager::DoWork(base::TimeTicks run_time, bool from_main_thread) Line 218
blink_platform.dll!base::internal::FunctorTraits<void (__cdecl blink::scheduler::TaskQueueManager::*)(base::TimeTicks,bool) __ptr64,void>::Invoke<base::WeakPtr<blink::scheduler::TaskQueueManager> const & __ptr64,base::TimeTicks const & __ptr64,bool const & __ptr64>(void(blink::scheduler::TaskQueueManager::*)(base::TimeTicks, bool) method, const base::WeakPtr<blink::scheduler::TaskQueueManager> & receiver_ptr, const base::TimeTicks & <args_0>, const bool & <args_1>) Line 215
blink_platform.dll!base::internal::InvokeHelper<1,void>::MakeItSo<void (__cdecl blink::scheduler::TaskQueueManager::*const & __ptr64)(base::TimeTicks,bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager> const & __ptr64,base::TimeTicks const & __ptr64,bool const & __ptr64>(void(blink::scheduler::TaskQueueManager::*)(base::TimeTicks, bool) & functor, const base::WeakPtr<blink::scheduler::TaskQueueManager> & weak_ptr, const base::TimeTicks & <args_0>, const bool & <args_1>) Line 306
blink_platform.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl blink::scheduler::TaskQueueManager::*)(base::TimeTicks,bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool>,void __cdecl(void)>::RunImpl<void (__cdecl blink::scheduler::TaskQueueManager::*const & __ptr64)(base::TimeTicks,bool) __ptr64,std::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool> const & __ptr64,0,1,2>(void(blink::scheduler::TaskQueueManager::*)(base::TimeTicks, bool) & functor, const std::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool> & bound, base::IndexSequence<0,1,2> __formal) Line 350
blink_platform.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl blink::scheduler::TaskQueueManager::*)(base::TimeTicks,bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool>,void __cdecl(void)>::Run(base::internal::BindStateBase * base) Line 328
base.dll!base::Callback<void __cdecl(void),1>::Run() Line 389
base.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, const base::PendingTask & pending_task) Line 56
base.dll!base::MessageLoop::RunTask(const base::PendingTask & pending_task) Line 489
base.dll!base::MessageLoop::DeferOrRunPendingTask(base::PendingTask pending_task) Line 500
base.dll!base::MessageLoop::DoWork() Line 621
base.dll!base::MessagePumpForUI::DoRunLoop() Line 262
base.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 142
base.dll!base::MessageLoop::RunHandler() Line 452
base.dll!base::RunLoop::Run() Line 36
base.dll!base::Thread::Run(base::RunLoop * run_loop) Line 229

Comment 2 by yosin@chromium.org, Aug 29 2016

Document::childNeedsStyleRecalc() returns true even if calling after Document::updateStyleAndLayoutTree().

Comment 3 by yosin@chromium.org, Aug 29 2016

Summary: EventHandler::sendContextMenuEvent() calls SelectionController::sendContextMenuEvent() with dirty layout tree (was: DCHECK on isEditablePosition)

Since, Document::prepareMouseEvent() makes layout tree dirty by setting hover element, EventHandler::sendContextMenuEvent() calls SelectionController::sendContextMenuEvent() with dirty layout tree.

Below is minimum reproduce case:
  <style>*:hover { color: red; }</style>
  <div>foo</div>

Right-Click other than "foo" causes DCHECK in isEditablePosition.

Comment 4 by yosin@chromium.org, Aug 29 2016

Status: Started (was: Available)

in-review: http://crrev.com/2288083002

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7e52616f252b5dca01400374e1076ab683c9620

commit e7e52616f252b5dca01400374e1076ab683c9620
Author: yosin <yosin@chromium.org>
Date: Tue Aug 30 03:42:33 2016

Call SelectionController().sendContextMenuEvent() with up-to-date layout tree

BUG= 641403 
TEST=run_webkit_unit_tests --gtest_filter=ventHandlerTest.sendContextMenuEventWithHover

Review-Url: https://codereview.chromium.org/2288083002
Cr-Commit-Position: refs/heads/master@{#415052}

[modify] https://crrev.com/e7e52616f252b5dca01400374e1076ab683c9620/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/e7e52616f252b5dca01400374e1076ab683c9620/third_party/WebKit/Source/core/input/EventHandlerTest.cpp

Comment 6 by yosin@chromium.org, Aug 30 2016

Status: Fixed (was: Started)

Comment 7 by marshall@chromium.org, Sep 2 2016

Labels: Merge-Request-54

Please merge to M54.

Comment 8 by dimu@chromium.org, Sep 2 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved

Your change meets the bar and is auto-approved for M54 (branch: 2840)

Comment 9 by ligim...@chromium.org, Sep 2 2016

Please merge your change to M54 (branch: 2840) before 5:00 PM PST Monday [09/05] if you would like to make it to M54 Beta promotion on Thursday [09/08].

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 5 2016

Labels: -merge-approved-54 merge-merged-2840

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5a5b60cdc9501097867e8819a16edd6fae674e9

commit b5a5b60cdc9501097867e8819a16edd6fae674e9
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Mon Sep 05 03:43:34 2016

Call SelectionController().sendContextMenuEvent() with up-to-date layout tree

BUG= 641403 
TEST=run_webkit_unit_tests --gtest_filter=ventHandlerTest.sendContextMenuEventWithHover

Review-Url: https://codereview.chromium.org/2288083002
Cr-Commit-Position: refs/heads/master@{#415052}
(cherry picked from commit e7e52616f252b5dca01400374e1076ab683c9620)

Review URL: https://codereview.chromium.org/2308333002 .

Cr-Commit-Position: refs/branch-heads/2840@{#150}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/b5a5b60cdc9501097867e8819a16edd6fae674e9/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/b5a5b60cdc9501097867e8819a16edd6fae674e9/third_party/WebKit/Source/core/input/EventHandlerTest.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5a5b60cdc9501097867e8819a16edd6fae674e9

commit b5a5b60cdc9501097867e8819a16edd6fae674e9
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Mon Sep 05 03:43:34 2016

Call SelectionController().sendContextMenuEvent() with up-to-date layout tree

BUG= 641403 
TEST=run_webkit_unit_tests --gtest_filter=ventHandlerTest.sendContextMenuEventWithHover

Review-Url: https://codereview.chromium.org/2288083002
Cr-Commit-Position: refs/heads/master@{#415052}
(cherry picked from commit e7e52616f252b5dca01400374e1076ab683c9620)

Review URL: https://codereview.chromium.org/2308333002 .

Cr-Commit-Position: refs/branch-heads/2840@{#150}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/b5a5b60cdc9501097867e8819a16edd6fae674e9/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/b5a5b60cdc9501097867e8819a16edd6fae674e9/third_party/WebKit/Source/core/input/EventHandlerTest.cpp

►

Issue 641403 link

Issue metadata

EventHandler::sendContextMenuEvent() calls SelectionController::sendContextMenuEvent() with dirty layout tree

Issue description

Comment 1 by yosin@chromium.org, Aug 29 2016

Comment 1 Deleted

Comment 2 by yosin@chromium.org, Aug 29 2016

Comment 2 Deleted

Comment 3 by yosin@chromium.org, Aug 29 2016

Comment 3 Deleted

Comment 4 by yosin@chromium.org, Aug 29 2016

Comment 4 Deleted

Comment 5 by bugdroid1@chromium.org, Aug 30 2016

Comment 5 Deleted

Comment 6 by yosin@chromium.org, Aug 30 2016

Comment 6 Deleted

Comment 7 by marshall@chromium.org, Sep 2 2016

Comment 7 Deleted

Comment 8 by dimu@chromium.org, Sep 2 2016

Comment 8 Deleted

Comment 9 by ligim...@chromium.org, Sep 2 2016

Comment 9 Deleted

Comment 10 by bugdroid1@chromium.org, Sep 5 2016

Comment 10 Deleted

Comment 11 by bugdroid1@chromium.org, Oct 27 2016

Comment 11 Deleted

Sign in to add a comment