If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

make test plugin usable for reftests

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: ted, Assigned: ted)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Josh wrote a test plugin in bug 386676 for use in plugin mochitests etc. This is great, since we severely lack coverage there. We also lack coverage of plugins in our reftests. We should extend the test plugin to be able to render simple color patterns so we could use it in reftest. As a start, something like "fill completely with green" would make it usable for a lot of reftests.
(Assignee)

Comment 1

9 years ago
Created attachment 356969 [details] [diff] [review]
very basic mac implementation

Here's a not-very-useful Mac implementation. It adds a drawMode field to the plugin data, and adds DM_SOLID_COLOR as an option, with the color specified in another field of the plugin data. I don't really know enough about plugins to add any useful way to set this (via scripting or URL params or embed params) but I assume it would be straightforward to do so. Having the ability to do:
<embed type="application/x-test" bgcolor="FF00FF00"></embed>

or something like that would make this quite usable for reftests.
(Assignee)

Comment 2

9 years ago
Created attachment 357162 [details] [diff] [review]
let test plugin draw solid colors, add reftests

Ok, this lets you specify some parameters in the <embed> (or presumably <param>s under the <object>, if the docs are to be believed), so you can do like:
<embed type="application/x-test" width="400" height="400" drawmode="solid" color="FF00FF00"></embed>
to get a solid green plugin that's 400x400 px. The color format is AARRGGBB. You can leave off the AA and it will default to 0xFF.

I wrote some basic sanity checking reftests, and they pass, which is cool. dbaron: is there a way to include these in the main reftest.list only if plugins are enabled? Alternately, is there a way to make them all fails-if(plugins are disabled)?
Assignee: nobody → ted.mielczarek
Attachment #356969 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #357162 - Flags: review?(joshmoz)

Comment 3

9 years ago
Comment on attachment 357162 [details] [diff] [review]
let test plugin draw solid colors, add reftests

>+PRUint32
>+parseHexColor(char* color)

Please move this into nptest_utils

>+  for (int i=0; i<argc; i++) {

Style is "i = 0", "i < argc".

What is the point of testing so many basic colors in exactly the same way?

We can figure out how to make a nice system for parsing arguments into the plugin later.
Attachment #357162 - Flags: review?(joshmoz)
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> What is the point of testing so many basic colors in exactly the same way?

Mostly it's to test that my hex color parsing code works. :) Never hurts to have sanity checks.
(In reply to comment #2)
> dbaron: is there a way to include these in the main reftest.list only if
> plugins are enabled? Alternately, is there a way to make them all
> fails-if(plugins are disabled)?

fails-if(MOZ_PLUGINS!="1"), I'd think.  That does add another dependency on the build, though...
(Assignee)

Comment 6

9 years ago
We could probably fake that in JS by looking for something that's only present when plugins are compiled, right? Actually, we could be even more specific, and see if we have the actual test plugin available, and expose that to the reftest manifest parser.
(Assignee)

Updated

9 years ago
Depends on: 473935
(Assignee)

Comment 7

9 years ago
Created attachment 357344 [details] [diff] [review]
fix nits, simplify reftests

Ok, fixed review comments, and combined the multi-color reftests into one single reftest testing 4 different colors. Ideally I'd have a few more tests in there to thoroughly test the hex color parsing code, but this is a good start.
Attachment #357162 - Attachment is obsolete: true
Attachment #357344 - Flags: review?(joshmoz)

Comment 8

9 years ago
Comment on attachment 357344 [details] [diff] [review]
fix nits, simplify reftests

>+      byte[0] = color[len-2];
>+      byte[1] = color[len-1];
...
>+      byte[1] = color[len-1];

Few more cases of not having a space around operators.

Looks good to me, thanks!
Attachment #357344 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 9

9 years ago
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/86c69da673eb
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 10

9 years ago
Ted, will you check in bug 469831 too?
(Assignee)

Comment 11

9 years ago
Sure. You really don't have commit access yet? For shame!

Comment 12

9 years ago
Yeah, everyone's saying I should get it. I'd like to become more experienced in hg first, though: http://quotes.burntelectrons.org/3853 :)
I just checked in a bustage fix for TB trunk: http://hg.mozilla.org/mozilla-central/rev/184eadf4e185 (add missing prtypes.h header).
(Assignee)

Comment 14

9 years ago
(In reply to comment #13)

Thanks, sorry I missed that!
You need to log in before you can comment on or make changes to this bug.