1407700 - onmouseover alert() crashes browser tab

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P1)

Description

[George Pîrlea]

Attachment: crash.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171010220102

Steps to reproduce:

1. Open the attached HTML file.
2. Hover your mouse over the text.
3. Click 'OK' to close the alert dialog that appears.


Actual results:

The browser tab crashes.

Crash reports:
  * Ubuntu 17.04: https://crash-stats.mozilla.com/report/index/5a666d3f-ffad-4046-aa8d-d78240171011
  * Windows 10: https://crash-stats.mozilla.com/report/index/f0456dc1-9102-4a00-a177-16fdd0171011


Expected results:

The tab shouldn't have crashed.

Comment 1

[George Pîrlea]

Bug 1407445 and Bug 1407350 seem related, as per bp-5a666d3f-ffad-4046-aa8d-d78240171011 and bp-f0456dc1-9102-4a00-a177-16fdd0171011, respectively.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Something to do with MaybeDispatchCoalescedMouseMoveEvents -> stone

Comment 3

[Ming-Chou Shih [:stone]]

Attachment: Part1: Revise CoalescedInputData interfaces.

Comment 4

[Ming-Chou Shih [:stone]]

Attachment: Part2: Handle the case to re-entry event loop for mousemove coalescing

Comment 5

[Ming-Chou Shih [:stone]]

Comment on attachment 8917688 [details] [diff] [review]
Part2: Handle the case to re-entry event loop for mousemove coalescing

Dropping events may be a bad idea. Trying to revise it.

Comment 6

[Ming-Chou Shih [:stone]]

(In reply to George Pîrlea from comment #0)
> Created attachment 8917458 [details]
> crash.html
Thanks for identifying the steps and creating the test case.

Comment 7

[Ming-Chou Shih [:stone]]

Attachment: Part2: Handle the case to re-entry event loop for mousemove coalescing

Comment 8

[Ming-Chou Shih [:stone]]

We might have a similar problem of wheel event coalescing that dispatches disorder events while reentrying the event loop. Created bug 1408232 to fix it.

Comment 9

[Ming-Chou Shih [:stone]]

(In reply to Ming-Chou Shih [:stone] from comment #7)
> Created attachment 8918067 [details] [diff] [review]
> Part2: Handle the case to re-entry event loop for mousemove coalescing

This is caused by reentrying the event loop and touch the same hash table while traversing it and dispatching an event. When dispatching the coalesced events, move them to another queue first and then start dispatching events.

When receiving a new mousemove which we can't coalesce, we should dispatch the coalesced event with the same pointer id. When receiving a new mouse event other than mouse move, we should dispatch all coalesced events. We could put the new mouse event (other than mousemove) in the same queue to make sure we won't dispatch it with incorrect order while reentrying the event loop.

Comment 10

[Carmen Fat [:cfat] - Ecosystem QA]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0

I have tested this issue on Windows 10 x64, using the STR provided in comment 0, performed a regression, the result as follows:

Last good revision: 212ceaf885b0d8750201f816030ad0db8391c7d7
First bad revision: 5e0a1d82ef670c537b7b5c60ba0f2606a9f04ef7
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=212ceaf885b0d8750201f816030ad0db8391c7d7&tochange=5e0a1d82ef670c537b7b5c60ba0f2606a9f04ef7

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1404273

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8917687 [details] [diff] [review]
Part1: Revise CoalescedInputData interfaces.

>+  UniquePtr<InputEventType> GetCoalescedEvent()
>   {
>-    return mCoalescedInputEvent.get();
>+    return Move(mCoalescedInputEvent);
>   }
The method should be called something like
TakeCoalescedEvent to hint that it actually clears the member variable on the object.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Sorry about delay with review.

Comment 13

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] (work week Oct 16-20) from comment #12)
> Sorry about delay with review.

It's okay. I know you are in the work week and thanks for the review.

Comment 14

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa5f7bc53c71fa9998555f3511ecd36bab2e9e26

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/78041b59b1a1
Part1: Revise CoalescedInputData interfaces. r=smaug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b9629c68575
Part2: Handle the case to re-entry event loop for mousemove coalescing. r=smaug.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/78041b59b1a1
https://hg.mozilla.org/mozilla-central/rev/9b9629c68575