Intermittent browser_bug1025195_switchToTabHavingURI_aOpenParams.js | Test timed out | application crashed [@ mozilla::dom::DocGroup::ValidateAccess] (Assertion failure: IsSafeToRun(), at SchedulerGroup.h:74) when Gecko 57 merges to Beta on 2017-09-20

VERIFIED FIXED in Firefox 57

Status

()

Core
Panning and Zooming
P5
normal
VERIFIED FIXED
2 months ago
a month ago

People

(Reporter: Treeherder Bug Filer, Assigned: rhunt)

Tracking

({assertion, crash, intermittent-failure})

unspecified
mozilla57
assertion, crash, intermittent-failure
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 verified)

Details

(Whiteboard: [gfx-noted])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 months ago
treeherder
Filed by: rvandermeulen [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=128041886&repo=try

https://queue.taskcluster.net/v1/task/KWUcw8rxQlKeM8-WLz59hg/runs/0/artifacts/public/logs/live_backing.log

Bisection on Try has confirmed that this was caused by bug 1376525. Appears to only affect Linux, but always in the same test. Failure rate is ~50%.

The patch below should be enough to reproduce.
https://hg.mozilla.org/try/rev/70cb6436d870bc25344b98e8ec3ed1be9cf9afc6
Flags: needinfo?(rhunt)
Keywords: assertion, crash

Comment 1

2 months ago
8 failures in 939 pushes (0.009 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* try: 8

Platform breakdown:
* linux32: 3
* linux64-stylo: 2
* linux64: 2
* linux32-stylo: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1396323&startday=2017-08-28&endday=2017-09-03&tree=all
(Assignee)

Comment 2

a month ago
My guess is:

1. A script is long running with DocGroup for tab A active
2. A tab switch happens
3. JS interrupt runs, triggering new tab B to paint
4. Keyboard APZ creates a focus target for this paint
5. Focus target collects event targets that would be hit for a key event (using an eVoid event)
6. One of them is an <input> which lazily initializes [1]
7. The lazy initialization causes a DOM modification for tab B [2] http://searchfox.org/mozilla-central/rev/f2a1911ad310bf8651f342d719e4f4ca0a7b9bfb/layout/forms/nsTextControlFrame.cpp#1202

The assertion is because the DocGroup for tab A is active, while we are modifying B.

I don't think we need to lazy init <input> when we are dispatching eVoid events. Fixing that resolved this issue for me. I can't see any other GetEventTargetParent implementations that do similar, so I think that's all that needed.

[1] http://searchfox.org/mozilla-central/rev/f2a1911ad310bf8651f342d719e4f4ca0a7b9bfb/dom/html/HTMLInputElement.cpp#3611
Assignee: nobody → rhunt
Flags: needinfo?(rhunt)
Whiteboard: [gfx-noted]
Comment hidden (mozreview-request)
Comment on attachment 8904716 [details]
Bug 1396323 - Don't initialize HTMLInputElement editor for eVoidEvent.

Looks good on Try!
Attachment #8904716 - Flags: feedback+

Comment 5

a month ago
mozreview-review
Comment on attachment 8904716 [details]
Bug 1396323 - Don't initialize HTMLInputElement editor for eVoidEvent.

https://reviewboard.mozilla.org/r/176500/#review181760

::: dom/html/HTMLInputElement.cpp:3582
(Diff revision 1)
>        aVisitor.mEvent->mClass == eMutationEventClass) {
>      return false;
>    }
>  
>    switch (aVisitor.mEvent->mMessage) {
> +  case eVoidEvent:

I think it'd be nice to include the description in the commit message as a code comment around here instead.
Attachment #8904716 - Flags: review?(ehsan) → review+

Comment 6

a month ago
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3067ce95b439
Don't initialize HTMLInputElement editor for eVoidEvent. r=ehsan
https://hg.mozilla.org/mozilla-central/rev/3067ce95b439
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.