Closed Bug 1396323 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Panning and Zooming, defect, P5)

defect

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- verified

People

(Reporter: intermittent-bug-filer, Assigned: rhunt)

References

Details

(Keywords: assertion, crash, intermittent-failure, Whiteboard: [gfx-noted])

Attachments

(1 file)

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)
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 on attachment 8904716 [details]
Bug 1396323 - Don't initialize HTMLInputElement editor for eVoidEvent.

Looks good on Try!
Attachment #8904716 - Flags: feedback+
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+
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
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.