1246854 - Debug test logs overflow on GTK+2 because of "WARNING: dom.w3c_touch_events.enabled=2 not implemented!"

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Phil Ringnalda (:philor)]

Apparently nobody has run a debug build on gtk2 for several months, because bug 1217515 set the pref to autodetect touch event support, and touch event autodetection isn't supported on gtk2, only gtk3, so when we switched beta back to gtk2, we get debug test logs like https://treeherder.mozilla.org/logviewer.html#?job_id=783451&repo=mozilla-beta with a trillion NS_WARNINGS until we overflow the maximum log size.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Tracking as it is a critical issue

Comment 2

[Phil Ringnalda (:philor)]

Ah, thanks to http://mxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#49 we only get it in test suites which use a profile, but don't use prefs based on that, which seems to be jsreftests and e10s-reftests.

Comment 3

[Phil Ringnalda (:philor)]

Make that "everything that's run by the reftest harness", including crashtests and non-e10s reftests, but those two have enough log size headroom to be able to absorb the trillions of warnings without log overflow.

Comment 4

[Phil Ringnalda (:philor)]

Attachment: Set the pref to not spew

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8717302 [details] [diff] [review]
Set the pref to not spew

thanks

Comment 6

[Kartikaya Gupta (email:[e-mail])]

I think we should actually just drop the warning from dom/events/TouchEvent.cpp and set it to 2 (or just not override it) in the testing harnesses.

Comment 7

[Phil Ringnalda (:philor)]

Comment on attachment 8717302 [details] [diff] [review]
Set the pref to not spew

https://hg.mozilla.org/releases/mozilla-beta/rev/74179c50c9d1

Comment 8

[Phil Ringnalda (:philor)]

Not spewing certainly seems like the right thing to do, my very first thought about this was "but, doesn't that make gtk2 debug builds completely unusable?" I was just aiming to get a patch landed that would let us run jsreftests and e10s reftests and thus reopen the tree, but we've still got trunk and aurora to fix, and perhaps beta to refix.

Comment 9

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> I think we should actually just drop the warning from
> dom/events/TouchEvent.cpp

Do you mean just for GTK2, or in general?

Comment 10

[Kartikaya Gupta (email:[e-mail])]

In general. For platforms that don't support touch events we can just "autodetect" false. Just like how for b2g/android we "autodetect" true at https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/dom/events/TouchEvent.cpp#185

Comment 11

[Botond Ballo [:botond]]

I thought the point of the warning was that we had some platforms where we *could* auto-detect, it just isn't implemented, but perhaps there are no such platforms.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Just to be clear - with the change to logging, this stops being a blocker?

Comment 13

[Phil Ringnalda (:philor)]

Right, that wasn't an accidental change of severity. It is no longer a blocker for 45, it will next become a blocker when we don't do anything on aurora, merge aurora to beta, and then decide yet again to punt on shipping gtk3 at 7pm minutes before going to build with a beta.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Stealing

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Attachment: MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond

Review commit: https://reviewboard.mozilla.org/r/34789/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34789/

Comment 16

[Kartikaya Gupta (email:[e-mail])]

Since this also called so often, I tried making the code use gfxPrefs instead of Preferences, but that crashed on startup because it gets invoked before gfxPlatform is initialized and so before gfxPrefs is set up.

Comment 17

[Botond Ballo [:botond]]

Comment on attachment 8718898 [details]
MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond

https://reviewboard.mozilla.org/r/34789/#review31415

Please add a comment at https://dxr.mozilla.org/mozilla-central/rev/576a6dcde5b68c2ea45324ed5ce1dabb7d833d09/modules/libpref/init/all.js#4644 saying "auto-detect is currently only supported on Windows and GTK3".

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b60e13839c82

Comment 19

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b60e13839c82

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Does this need uplifting to 46? I'm assuming not but I don't know what the state of gtk3 is. Please flip status-firefox46 to wontfix if we don't need it uplifted.

Comment 21

[Phil Ringnalda (:philor)]

I would assume it does need uplifting, unless you want to have local (or third-party automation) gtk2 builds be unable to sanely run reftests and then have sudden permaorange if relman yet again decides at the last second to not ship gtk3 like they have twice already.

Comment 22

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8718898 [details]
MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond

Approval Request Comment
[Feature/regressing bug #]: bug 1217515
[User impact if declined]: tests possibly failing on debug builds due to log overflowing with NS_WARNING. No user-visible impact since NS_WARNING does nothing in opt builds. See philor's above comment.
[Describe test coverage new/current, TreeHerder]: covered by automated tests
[Risks and why]: low risk, just removing a NS_WARNING
[String/UUID change made/needed]: none

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8718898 [details]
MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond

Match the 45 release, taking it.
Should be in 46 beta 5

Comment 24

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-beta/rev/c93a1ee478a3