Crash in [@ mozilla::PresShell::EventHandler::HandleEventWithTarget]
Categories
(Core :: DOM: Events, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr68 | --- | unaffected |
| firefox69 | --- | unaffected |
| firefox70 | --- | wontfix |
| firefox71 | --- | wontfix |
| firefox72 | --- | fixed |
People
(Reporter: marcia, Assigned: masayuki)
References
(Depends on 1 open bug)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
This bug is for crash report bp-aa9b71d3-3b8b-4098-8fdb-43d4a0190913.
small volume Windows and macOS regression which started in 20190909214621: https://bit.ly/2lPJQhr.
All crashes have MOZ_DIAGNOSTIC_ASSERT(aEvent->IsTrusted())
Possible regression range based on build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=171d5f099ad0d22feda354c74182703ed02842a7&tochange=bdb64cf16b68c4a7212ba16aef425bce66d8f4ca
Top 10 frames of crashing thread:
0 xul.dll mozilla::PresShell::EventHandler::HandleEventWithTarget layout/base/PresShell.cpp:7677
1 xul.dll mozilla::PresShell::HandleEventWithTarget layout/base/PresShell.h:643
2 xul.dll static nsresult mozilla::EventStateManager::InitAndDispatchClickEvent dom/events/EventStateManager.cpp:4973
3 xul.dll nsresult mozilla::EventStateManager::DispatchClickEvents dom/events/EventStateManager.cpp:5074
4 xul.dll nsresult mozilla::EventStateManager::PostHandleMouseUp dom/events/EventStateManager.cpp:5017
5 xul.dll mozilla::EventStateManager::PostHandleEvent dom/events/EventStateManager.cpp:3299
6 xul.dll nsresult mozilla::PresShell::EventHandler::DispatchEvent layout/base/PresShell.cpp:7901
7 xul.dll mozilla::PresShell::EventHandler::HandleEventWithCurrentEventInfo layout/base/PresShell.cpp:7809
8 xul.dll nsresult mozilla::PresShell::EventHandler::HandleEventUsingCoordinates layout/base/PresShell.cpp:6768
9 xul.dll mozilla::PresShell::EventHandler::HandleEvent layout/base/PresShell.cpp:6573
|
||
Comment 1•5 years ago
|
||
Masayuki, you added the assert in the PresShell refactoring work. I guess you may want to take a look.
|
||
Updated•5 years ago
|
|
Assignee | |
Comment 2•5 years ago
|
||
Yes, it is... PresShell::EventHandler is designed only for working with trusted events. EventStateManager::InitAndDispatchClickEvent() uses trsted state of handled eMouseUp event. The eMouseUp event comes from the chrome process. However, it should be trusted events. And it's also tested in PresShell::EventHandler::HandleEvent() before the crash point. So, the crash report indicates we have a bug.
|
|
Assignee | |
Comment 3•5 years ago
|
||
WidgetEvent::mFlags::mIsTrusted is modified only by dom::Event::SetTrusted(). So, somebody must overwrite the eMouseUp event's trusted flag with it. Most of them set it to true. I suspect dom::Event::InitEvent(), but I have no idea how to call Event.init() before dispatching mouseup event since Event::mEvent will be swaped to different instance immediately after finishing dispatching it.
|
|
Assignee | |
Comment 4•5 years ago
|
||
Hmm, I cannot check the comments of crash reports due to no permission... Could somebody check the comments for looking for some hits to reproduce the crash?
|
|
||
Comment 5•5 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: 9/21~9/29) from comment #4)
Hmm, I cannot check the comments of crash reports due to no permission... Could somebody check the comments for looking for some hits to reproduce the crash?
I'll try if I could get something.
|
Reporter | |
Comment 6•5 years ago
|
||
Doesn't appear to be any comments. Only 2 URLs:
*https://www.pinterest.com/pin/693765517579940310/?nic=1
*https://jamesfriend.com.au/pce-js/mecc/oregon-trail.html
|
||
Comment 7•5 years ago
|
||
Wontfix for 70 which is in RC and fix-optional for 71 given the volume of crashes.
|
|
Assignee | |
Comment 8•5 years ago
|
||
(In reply to Marcia Knous [:marcia - needinfo? me] from comment #6)
Doesn't appear to be any comments. Only 2 URLs:
*https://www.pinterest.com/pin/693765517579940310/?nic=1
*https://jamesfriend.com.au/pce-js/mecc/oregon-trail.html
Thank you, I reproduce this bug with the latter URL at clicking the image. I'll try to investigate with debugger.
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 9•5 years ago
|
||
The crash occurs when dispatching a user input event which is a default action
of a raw user input event like click event caused by mouseup event if
the raw event's isTrusted is set to false accidentally during dispatch.
User input events are fired in the main process first. Then,
EventStateManager sends it to remote process from PostHandleEvent() if
necessary. However, at this time, WidgetEvent::mFlags::mDispatchedAtLeastOnce
is never rest, but its only referrer, EventDispatcher::DispatchDOMEvent()
assumes that when it's true, WidgetEvent::mFlags:mIsBeingDispatched is
false. Therefore, only in content process, mouseup event's isTrusted is
set to false by EventTarget.dispatchEvent() even while it's being dispatch.
And also the trusted state will be used for creating next event which is part
of the default action.
https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/dom/events/EventDispatcher.cpp#1121,1126,1130-1131,1135,1138,1143
Therefore, this patch makes WidgetEvent::mFlags reset mDispatchedAtLeastOnce
when it's copied across process boundary and make
EventDispatcher::DispatchDOMEvent() won't modify being dispatched events for
avoiding any odd issues.
Unfortunately, this patch adds "expected: FAIL" to the new WPT test only on
Windows. The failure reason is still unclear. I cannot reproduce the failure
on my Windows environment, but on Try Server, it fails permanently since
the driver succeeds to send the mouse click, but the button never receives
mouseup nor click event.
|
|
Assignee | |
Comment 10•5 years ago
|
||
Surprisingly, the new WPT added by the patch fails only on Windows on the server. I'm not sure why it does not fail on the other platforms and on my Windows machine... That's the reason why I took a couple of days after created first patch and took totally one week!
|
||
Comment 11•5 years ago
|
||
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/585a6c8f92e3 `WidgetEvent::mFlags::mDispatchedAtLeastOnce` needs to be reset before dispatching in content process again r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20448 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
|
||
Comment 14•5 years ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
|
||
Updated•4 years ago
|
Description
•