Closed Bug 1777052 Opened 3 years ago Closed 3 years ago

High freq toolkit/content/tests/mochitest/test_mousecapture.xhtml | single tracking bug

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
113 Branch
Tracking Flags:
Tracking Status
firefox113 --- fixed

People

(Reporter: jmaher, Assigned: hiro)

References

Details

(Keywords: intermittent-failure, intermittent-testcase, Whiteboard: [stockwell needswork:owner])

Attachments

(1 file)

Bug 1777052 - Skip counting scroll events where the scroll position is unchanged in test_mousecapture.xhtml. r?botond
48 bytes, text/x-phabricator-request
Details | Review
No description provided.

Additional information about this bug failures and frequency patterns can be found by running: ./mach test-info failure-report --bug 1777052

Severity: normal → S3

Update

There have been 31 failures, all of them on OS X 10.15 WebRender debug.

Recent failure log: https://treeherder.mozilla.org/logviewer?job_id=409411590&repo=mozilla-central&lineNumber=43555

[task 2023-03-19T23:09:10.463Z] 23:09:10     INFO - TEST-PASS | toolkit/content/tests/mochitest/test_mousecapture.xhtml | text is selected 
[task 2023-03-19T23:09:10.463Z] 23:09:10     INFO - Scrolled 120 pixels
[task 2023-03-19T23:09:10.463Z] 23:09:10     INFO - Buffered messages finished
[task 2023-03-19T23:09:10.464Z] 23:09:10     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/mochitest/test_mousecapture.xhtml | selection scroll position after timer is at least 140 
[task 2023-03-19T23:09:10.464Z] 23:09:10     INFO -     SimpleTest.ok@SimpleTest/SimpleTest.js:421:16
[task 2023-03-19T23:09:10.464Z] 23:09:10     INFO -     selectionScrollDone@toolkit/content/tests/mochitest/test_mousecapture.xhtml:87:7
[task 2023-03-19T23:09:10.464Z] 23:09:10     INFO -     EventListener.handleEvent*selectionScrollCheck/<@toolkit/content/tests/mochitest/test_mousecapture.xhtml:111:17
[task 2023-03-19T23:09:10.465Z] 23:09:10     INFO - TEST-PASS | toolkit/content/tests/mochitest/test_mousecapture.xhtml | selection scroll position after timer is multiple of 20

Hsin-Yi, can you help us assign this to someone?
Thank you.

Flags: needinfo?(htsai)
Whiteboard: [stockwell needswork:owner]

Hey Gijs,
Code blame told me you were probably the person who may have investigated the intermittent failure in test_mousecapture.xhtml the most recently, even though it's still several years ago ... ... can we still have your help and take a look at this ?

Flags: needinfo?(htsai) → needinfo?(gijskruitbosch+bugs)

I'm confused because this feels like deja vu...

Oh right, bug 1821127. Well,

(In reply to Hsin-Yi Tsai (she/her) [:hsinyi] from comment #37)

Code blame told me you were probably the person who may have investigated the intermittent failure in test_mousecapture.xhtml the most recently, even though it's still several years ago ... ... can we still have your help and take a look at this ?

(In reply to :Gijs (he/him) from bug 1821127 comment #5)

Just to fix some other intermittent failure by loosening a test condition to allow for some whitespace...

I think this is fundamentally an APZ type of failure where we're trying to drag-to-scroll and we're scrolling less than expected (looks like 120 instead of 140px). It's not clear to me how this happens given the comment in the test: https://searchfox.org/mozilla-central/rev/f078cd02746b29652c134b144f0629d47e378166/toolkit/content/tests/mochitest/test_mousecapture.xhtml#74-93 . It appears to only fail on mac - not entirely clear to me why.

I don't actually know anything about scrolling and the like - which is also clear when I'm re-reading the comments in bug 1285176 😅. :kats and :mconley helped a lot at the time, and it looks like Neil Deakin wrote the original test, with :roc reviewing. :dholbert, are you able to take a look here, or if not, can you recommend someone else who understands our scroll/selection code?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dholbert)

Given this...

(In reply to :Gijs (he/him) from comment #38)

I think this is fundamentally an APZ type of failure where we're trying to drag-to-scroll and we're scrolling less than expected (looks like 120 instead of 140px).

...I'll punt the needinfo over to :botond, to either take a look or hand off to someone on the APZ team.

Flags: needinfo?(dholbert) → needinfo?(botond)
Summary: Intermittent toolkit/content/tests/mochitest/test_mousecapture.xhtml | single tracking bug → High freq toolkit/content/tests/mochitest/test_mousecapture.xhtml | single tracking bug

The test failures are all on Mac. Hiro, could you check please if you're able to reproduce the failure locally? If not, I can try to investigate using try pushes with logging.

Flags: needinfo?(botond) → needinfo?(hikezoe.birchill)

One potential theory is that we hit something like bug 1682203 that causes us to receive a duplicate scroll event after a single scroll operation.

Though I haven't been able to reproduce the failure locally on my mac book. I am assigning to myself for now.

(In reply to Botond Ballo [:botond] from comment #42)

One potential theory is that we hit something like bug 1682203 that causes us to receive a duplicate scroll event after a single scroll operation.

Yeah, bug 1682203 could be a source of the problem. One thing I noticed is that most failures happened on debug builds, and it seems to have ASSERTION: Unexpected document: 'aCapturingContent->OwnerDoc() == GetDocument()', maybe it disturbs the test? And I wonder why the assertion happens, if the assertion is valid, there's definitely an issue in our implementation that this failure caught.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED
Flags: needinfo?(hikezoe.birchill)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #43)

One thing I noticed is that most failures happened on debug builds, and it seems to have ASSERTION: Unexpected document: 'aCapturingContent->OwnerDoc() == GetDocument()', maybe it disturbs the test? And I wonder why the assertion happens, if the assertion is valid, there's definitely an issue in our implementation that this failure caught.

Though it was a while ago and things may have changed since then, in bug 1285176 comment 21 Kats said he thought this assertion failure was unrelated.

I am getting to close the source of the problem. So there's an unexpected scroll event caused by session restore, specifically restoring the scroll position in question.

During running the test case, scrolling by selection keeps going, and in the middle of scrolling, we save the scroll position at a point, and later in a reflow we restore the the saved scroll position, thus there's the unexpected scroll position change. The reason why we save the scroll position changed by selection is. I presume, this isScrollAnimation doesn't care it.

That said, there's one more question, why do we reflow during running the test case?

So the isScrollAnimation is unrelated. A big problem is the reflow comes from reconstructing the documentElement frame, thus when we reconstruct the scroll frame we try to restore the original scroll position, the previous scroll position for the newly reconstructed scroll frame is (0, 0) unfortunately, thus we fire a scroll event!

Ideally we should stop firing scroll events if the restored scroll position is unchanged, but it will have some amount of work since we can't store the previous scroll position in the scroll frame, we need to store somewhere outside the scroll frame since the frame is destroyed, nsIContent would be one of candidates.

Another option to fix this test failure is finding out the source of reconstructing the documentElement, I don't see it on my macbook at all, it's probably our CI specific, it will be tough.

Since we understand the cause of the duplicate scroll event, it may also be fine to make the test slightly more permissive to handle it, for example to record the previous scroll offset and if we get a scroll event with the same scroll offset, not increment the count.

See Also: → 1825470

It worked perfectly!. Thanks for the great idea!

Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/07b5a9d0df25 Skip counting scroll events where the scroll position is unchanged in test_mousecapture.xhtml. r=botond

https://hg.mozilla.org/mozilla-central/rev/07b5a9d0df25

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox113: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: