628589 - [SeaMonkey] mochitest-plain-4: permanent "TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on x86-64 OS X". (Windows 2003 too)

Status: VERIFIED FIXED

(Core :: Graphics, defect, P2)

Description

[Serge Gautherie (:sgautherie)]

Maybe acceleration is not (yet) enabled/supported on SeaMonkey builders?

Comment 1

[Serge Gautherie (:sgautherie)]

Windows is affected too:
"9576 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on Windows"

Comment 2

[Legacy TBPL/Treeherder Robot]

ehsan%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1295989472.1295991548.30030.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-3/5 on 2011/01/25 13:04:32

s: talos-r3-snow-014
14387 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on x86-64 OS X

Comment 3

[Justin Wood (:Callek)]

dropping [SeaMonkey] per ehsan's c#2

Comment 4

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1295989472.1295991548.30030.gz
> Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-3/5 on
> 2011/01/25 13:04:32

A Firefox "random" is ("interesting" but) not this bug.
(Unless the underlying issue could be explained.)


(In reply to comment #3)
> dropping [SeaMonkey] per ehsan's c#2

Well, no.

Comment 5

[Justin Wood (:Callek)]

(In reply to comment #4)
> (In reply to comment #3)
> > dropping [SeaMonkey] per ehsan's c#2
> 
> Well, no.

Are we sure its not the same bug; if so, why? if not I think we should defer to this being a Firefox intermittent atm.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> Are we sure its not the same bug; if so, why? if not I think we should defer to
> this being a Firefox intermittent atm.

100% sure? No. It might be the same bug, but there is no evidence of that atm.
Thus I don't think 1 Firefox random should "hide" days of SeaMonkey perma-failures on two platforms just yet.

What I'd like to know is whether acceleration is actually supported/enabled on these SeaMonkey builders (when running the browser manually for example)...

Comment 7

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows NT 5.0; rv:2.0b11pre) Gecko/20110126 Firefox/4.0b11pre] (.../1296058257) (W2Ksp4)
{
GPU Accelerated Windows  0/1
}
This test fails on my Windows 2000 too.

***

Does Core support acceleration on Windows 2000/XP/2003 (and Linux)?

Fwiw, see "Hardware Acceleration by operating system" table at
http://hacks.mozilla.org/2010/09/hardware-acceleration/

Comment 8

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-WIPv1) Add missing 'var', Add/Improve checks

Looking for early comments:
I'm not sure which exact platform acceleration is supported on :-/
I'll Try this patch, fwiw...

Comment 9

[Joe Drew (not getting mail)]

Comment on attachment 507162 [details] [diff] [review]
(Av1-WIPv1) Add missing 'var', Add/Improve checks

I'd prefer you didn't move the Windows_NT block down, because it makes hg annotate more complicated, but I don't feel super-strongly about it.

Comment 10

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Add missing 'var', Add/Improve checks

Av1-WIPv1, enhanced.

I'm still not 100% sure about which platforms are supposed to support which features,
but this rewrite succeeded on (Firefox) Try and it's just a mochitest.
At least, it's more explicit about the current expectations.


(In reply to comment #9)
> I'd prefer you didn't move the Windows_NT block down, because it makes hg
> annotate more complicated, but I don't feel super-strongly about it.

As I am +/- rewriting it all, I prefer to list the platform alphabetically.

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to comment #10)
> this rewrite succeeded on (Firefox) Try

Except on 'Maemo 5 opt' where mochi-3 suite doesn't start atm...

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> What I'd like to know is whether acceleration is actually supported/enabled on
> these SeaMonkey builders (when running the browser manually for example)...

Justin, could you download and manually run a Firefox build on these builders and check <about:support>?
So you should find out whether the issue is in SM builds or boxes.

Comment 13

[Joe Drew (not getting mail)]

Can you please split up the functionality changes from the cosmetic ones, for ease of review?

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Add missing 'var', Sort os names alphabetically [Checked in: Comment 26]

Av1, split up per comment 13.

Comment 15

[Legacy TBPL/Treeherder Robot]

me%shawnwilsher.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1296162708.1296163245.26366.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central opt test mochitests-3/5 on 2011/01/27 13:11:48

s: talos-r3-snow-014
14387 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on x86-64 OS X

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 507609 [details] [diff] [review]
(Bv1) Add missing 'var', Sort os names alphabetically
[Checked in: Comment 26]

Not sure what the advantage of sorting the list alphabetically is. Seems like needless churn to me.

Comment 17

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) Enhance existing checks, Add Windows 2000 check, Add Windows checks for DWrite, Add default case/check (for non-MacOSX non-Windows) [Checked in: Comment 27]

Av1, split up per comment 13,
with added DWrite checks.

Ftr,
http://mxr.mozilla.org/mozilla-central/source/widget/public/nsIGfxInfo.idl
It might be interesting to use getFeatureStatus() (and other methods) to enhance the test and/or add further details (when a check fails).

Comment 18

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-3/5 on
> 2011/01/25 13:04:32
> 
> s: talos-r3-snow-014

(In reply to comment #4)
> A Firefox "random" is ("interesting" but) not this bug.
> (Unless the underlying issue could be explained.)

(In reply to comment #15)
> me%shawnwilsher.com
> Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central opt test mochitests-3/5 on
> 2011/01/27 13:11:48
> 
> s: talos-r3-snow-014

Something wrong with 'talos-r3-snow-014' box?

***

Still "not this bug" as a Firefox "random" test failure.

(In reply to comment #17)
> http://mxr.mozilla.org/mozilla-central/source/widget/public/nsIGfxInfo.idl
> It might be interesting to use getFeatureStatus() (and other methods) to
> enhance the test and/or add further details (when a check fails).

But this could be a hint/confirmation that current platform checks are not appropriate as a Core test, could it not?
(As in: if some runtime requirements are not met, the test should report a todo instead of a failure.)

Comment 19

[Legacy TBPL/Treeherder Robot]

jmathies%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1296180548.1296180956.28638.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central opt test mochitests-3/5 on 2011/01/27 18:09:08

s: talos-r3-snow-014
14387 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on x86-64 OS X

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #18)
> Something wrong with 'talos-r3-snow-014' box?

(In reply to comment #19)
> s: talos-r3-snow-014

I filed bug 629598.

Comment 21

[Serge Gautherie (:sgautherie)]

(In reply to comment #20)
> I filed bug 629598.

I filed bug 629616 to change screen resolution of SeaMonkey boxes...

Comment 22

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) test_selection_underline.html: Move waitForExplicitFinish() out of onLoad(), Make existing todo() more explicit [Checked in: Comment 30]

(Found when I searched for tests using nsIGfxInfo...)

Comment 23

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1) test_canvas.html: s/tab/4-spaces/g, Call IsD2DEnabled() once only, Add a todo() when Direct2D is enabled

(Found when I searched for tests using nsIGfxInfo...)

Comment 24

[Serge Gautherie (:sgautherie)]

(In reply to comment #17)
> http://mxr.mozilla.org/mozilla-central/source/widget/public/nsIGfxInfo.idl
> It might be interesting to use getFeatureStatus() (and other methods) to
> enhance the test and/or add further details (when a check fails).

Related to this, I filed bug 629957.

Comment 25

[Joe Drew (not getting mail)]

Comment on attachment 507609 [details] [diff] [review]
(Bv1) Add missing 'var', Sort os names alphabetically
[Checked in: Comment 26]

I see no reason to sort the switch alphabetically, but I don't care much either.

Comment 26

[Serge Gautherie (:sgautherie)]

Comment on attachment 507609 [details] [diff] [review]
(Bv1) Add missing 'var', Sort os names alphabetically
[Checked in: Comment 26]

http://hg.mozilla.org/mozilla-central/rev/b7ca6147425e

Comment 27

[Serge Gautherie (:sgautherie)]

Comment on attachment 507620 [details] [diff] [review]
(Cv1) Enhance existing checks, Add Windows 2000 check, Add Windows checks for DWrite, Add default case/check (for non-MacOSX non-Windows)
[Checked in: Comment 27]

http://hg.mozilla.org/mozilla-central/rev/3bb4b3a3189a

Comment 28

[Serge Gautherie (:sgautherie)]

(In reply to comment #27)
> http://hg.mozilla.org/mozilla-central/rev/3bb4b3a3189a

+
http://hg.mozilla.org/mozilla-central/rev/178298b4d9db
(Fv1) s/DWrite/DirectWrite/g

Comment 29

[Serge Gautherie (:sgautherie)]

Ftr, udated logs:

OSX 10.6:
{
9820 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on x86-64 OS X - didn't expect 0, but got it
}

Windows 2003:
{
9699 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on Windows XP or newer - didn't expect 0, but got it
9700 INFO TEST-PASS | /tests/gfx/test_acceleration.html | Direct2D not supported on Windows 2003 or older
9701 INFO TEST-PASS | /tests/gfx/test_acceleration.html | DirectWrite not supported on Windows 2003 or older
}

Comment 30

[Serge Gautherie (:sgautherie)]

Comment on attachment 508169 [details] [diff] [review]
(Dv1) test_selection_underline.html: Move waitForExplicitFinish() out of onLoad(), Make existing todo() more explicit
[Checked in: Comment 30]

http://hg.mozilla.org/mozilla-central/rev/ee7c16ea0e60

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/38b8e1b2c1b8
(Fv1-192) test_selection_underline.html: Move waitForExplicitFinish() out of onLoad(), Make existing todo() more explicit

Comment 31

[Legacy TBPL/Treeherder Robot]

david.bolter%gmail.com
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1299269077.1299269462.21175.gz
Rev3 WINNT 5.1 tryserver opt test mochitests-3/5 on 2011/03/04 12:04:37

s: talos-r3-xp-009
14449 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on Windows XP or newer - didn't expect 0, but got it

Comment 32

[Serge Gautherie (:sgautherie)]

(In reply to comment #31)
> david.bolter%gmail.com
> http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1299269077.1299269462.21175.gz

Still "not this bug" as a Firefox "random" test failure!

Comment 33

[Legacy TBPL/Treeherder Robot]

St8
https://tbpl.mozilla.org/php/getParsedLog.php?id=6924244&tree=Try
Rev4 MacOSX Snow Leopard 10.6 try opt test mochitests-4/5 on 2011-10-18 21:31:27

109 ERROR TEST-UNEXPECTED-FAIL | /tests/gfx/test_acceleration.html | Acceleration enabled on x86-64 OS X - didn't expect 0, but got it

Comment 34

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1a) test_canvas.html: s/tab/4-spaces/g, Call IsD2DEnabled() once only, Add a todo() when Direct2D is enabled

Ev1, unbitrotted.

Comment 35

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 582549 [details] [diff] [review]
(Ev1a) test_canvas.html: s/tab/4-spaces/g, Call IsD2DEnabled() once only, Add a todo() when Direct2D is enabled

Do these indentation changes move us closer or further from upstream?

Comment 36

[Serge Gautherie (:sgautherie)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #35)
> Do these indentation changes move us closer or further from upstream?

I didn't know there was an upstream for this test(s). What/Where is it?

Comment 37

[Jeff Muizelaar [:jrmuizel]]

(In reply to Serge Gautherie (:sgautherie) from comment #36)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #35)
> > Do these indentation changes move us closer or further from upstream?
> 
> I didn't know there was an upstream for this test(s). What/Where is it?

http://philip.html5.org/tests/canvas/suite/tests/

Also see bug 703488

Comment 38

[Serge Gautherie (:sgautherie)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #37)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #35)
> > > Do these indentation changes move us closer or further from upstream?
> 
> http://philip.html5.org/tests/canvas/suite/tests/

Hum, all tests were merged into one in m-c...

I tried to compare test_2d.clearRect.basic.html,
but http://philip.html5.org/tests/canvas/suite/tests/2d.clearRect.basic.html source code
looks "completely" different from the one in m-c :-/

> Also see bug 703488

Maybe that bug should be done first...

Yet, if you would accept my patch without the indentation changes, that would be good enough for this bug. Would you?

While there, I think a reference to upstream should be added to m-c file, should it not?

Comment 39

[Serge Gautherie (:sgautherie)]

Attachment: (Fv1 (bis)) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration

At last ;-<

Comment 40

[Joe Drew (not getting mail)]

Comment on attachment 588328 [details] [diff] [review]
(Fv1 (bis)) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration

Definitely don't want the (acceleratedWindows == 0) condition. I think whatever we do should be only on SeaMonkey.

Also, we probably want an actual test here, so when switching machines we verify that something has changed. So it could be

if (navigator.userAgent.match(/ SeaMonkey\//)) {
  is(acceleratedWindows, 0, "Acceleration not supported on x86-64 OS X" +
                            " (This is expected on SeaMonkey (tinderboxes).)");
} else {
  isnot(acceleratedWindows, 0, "Acceleration enabled on x86-64 OS X");
}

Comment 41

[Serge Gautherie (:sgautherie)]

Attachment: (Fv2) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration [Checked in: Comments 42 and 46]

Fv1, with comment 40 suggestion(s).


(In reply to Joe Drew (:JOEDREW!) from comment #40)

> Definitely don't want the (acceleratedWindows == 0) condition. I think
> whatever we do should be only on SeaMonkey.

Agreed.

> Also, we probably want an actual test here, so when switching machines we
> verify that something has changed.

That was exactly what I had done in the first place ;->
But, on one hand, new machines would only happen once (if ever): new behavior can be manually checked at that very time and workaround wouldn't hurt anyway;
whereas, on the other hand, no actual test lets local testers (if any) check the full test (if they have a supported environment) :-| (Pros and cons...)

Comment 42

[Serge Gautherie (:sgautherie)]

Comment on attachment 588587 [details] [diff] [review]
(Fv2) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration
[Checked in: Comments 42 and 46]

https://hg.mozilla.org/mozilla-central/rev/42a133039277


[Approval Request Comment]
Regression caused by (bug #): bug 627498.
User impact if declined: none, but perma-orange mochi-4 on SeaMonkey (due to this very test only).
Testing completed (on m-c, etc.): this comment.
Risk to taking this patch (and alternatives if risky): No risk, test-only SeaMonkey workaround.

Comment 43

[Serge Gautherie (:sgautherie)]

Comment on attachment 588328 [details] [diff] [review]
(Fv1 (bis)) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration

Oh, there was already a patch named 'Fv1' in comment 28+30 :-/

Comment 44

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326800207.1326801067.649.gz
OS X 10.6 comm-central-trunk debug test mochitests-4/5 on 2012/01/17 03:36:47
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326791848.1326794814.20843.gz
WINNT 5.2 comm-central-trunk debug test mochitests-4/5 on 2012/01/17 01:17:28

V.Fixed

Comment 45

[Alex Keybl [:akeybl]]

Comment on attachment 588587 [details] [diff] [review]
(Fv2) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration
[Checked in: Comments 42 and 46]

[Triage Comment]
Test only - approved for aurora/beta.

Comment 46

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 588587 [details] [diff] [review]
(Fv2) test_acceleration.html: Add a workaround for SeaMonkey (tinderboxes) which don't support acceleration
[Checked in: Comments 42 and 46]

http://hg.mozilla.org/releases/mozilla-beta/rev/afcb5dfe30eb

Comment 47

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1329966089.1329968490.29674.gz
OS X 10.6 comm-beta debug test mochitests-4/5 on 2012/02/22 19:01:29

firefox11: verified.

Comment 48

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 582549 [details] [diff] [review]
(Ev1a) test_canvas.html: s/tab/4-spaces/g, Call IsD2DEnabled() once only, Add a todo() when Direct2D is enabled

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

I expect this has rotted a great deal. It also mixed cosmetic changes with functional ones.