962911 - Bring back more-than-one-native-event-per-gecko-event processing

Status: RESOLVED INCOMPLETE

(Core :: Widget, defect)

Description

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

Attachment: more_native_events.diff

In bug 930793 I took the rather extreme approach to process only one native event
per gecko event. But as bug 961869 for example shows, that was a bit too much.
Instead of bringing back max 20ms processing, the patch gives max 10ms.

(pushed from a bit old tree)
https://tbpl.mozilla.org/?tree=Try&rev=25da36e626c7
https://tbpl.mozilla.org/?tree=Try&rev=a1fdcbb35dd2

(pushed from up-to-data)
https://tbpl.mozilla.org/?tree=Try&rev=72e57f6b02f2

Based on try the patch seems to work rather well.
tp doesn't seem to regress.

The old code was
http://mxr.mozilla.org/mozilla-aurora/source/widget/xpwidgets/nsBaseAppShell.cpp?mark=254-254,275-278#253

(apparently roc is on vacation, so perhaps you avih can review this partial backout.)

Comment 1

[Avi Halachmi (:avih)]

Comment on attachment 8364098 [details] [diff] [review]
more_native_events.diff

Review of attachment 8364098 [details] [diff] [review]:
-----------------------------------------------------------------

Good move. This is closer to how native events were processed before bug 930793 landed, but still without the starvation. /me likes.

Comment 2

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/5297eafc36e2

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Backed out for making B2G reftests much more failure prone. R1 and R5 were near perma-fail after this landed. It's worth noting that the R1 failures are almost all on file, so it's probably exacerbating a preexisting timing issue.
https://hg.mozilla.org/integration/mozilla-inbound/rev/93440685551d

https://tbpl.mozilla.org/php/getParsedLog.php?id=33461914&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=33461884&tree=Mozilla-Inbound

Comment 4

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

Bah. 

Tryserver push with limit 20, which is the old value.

https://tbpl.mozilla.org/?tree=Try&rev=ddf67ad3fae6

Comment 5

[Ryan VanderMeulen [:RyanVM]]

In general, it appears to just have made B2G tests more failure-prone. More examples:
https://tbpl.mozilla.org/php/getParsedLog.php?id=33467269&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=33466936&tree=Mozilla-Inbound

Comment 6

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

But I need to now test whether 20ms causes other regressions.
I thought it did...

Comment 7

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

looks like the fractal demo is still ok with 20ms.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Confirmed that B2G is a lot more green post-backout.

Comment 9

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

Nice, 20ms fails too. And I think wrapper.html is just broken.
...fixing another test.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 11

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.

Comment 12

[Stephen A Pohl [:spohl]]

I'm going to close this bug since there hasn't been any activity in 8 years. Let's open a new bug if there is anything left to address here.