517078 - Create mochitests for stream API's in NPAPI

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jonathan Griffin (:jgriffin)]

These mochitests should exercise the NPAPI stream API's.  They should be designed to run in-process, and then can be used to verify out-of-process plugin streams.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: NPAPI stream mochitests and application/x-test plugin update

Attached is version 0.1 of the NPAPI stream tests.  It's not quite ready for formal review and checkin because:

- I haven't verified it on the Mac.  I can compile it, but can't run the mochitests due to Snow Leopard problems (bug 516789).
- I need to add tests for NPN_PostURLNotify and NPN_NewStream.
- I need to add documentation to README.txt for the plugin's new capabilities.
- I want to add more tests for error and boundary conditions.
- I want to add the ability to use binary streams (currently only text streams are supported).

That said, there's probably enough here to be useful in electrolysis plugin work.

You can run a test without mochitest by compiling the plugin, then loading an HTML page with two elements:

<iframe name="testframe"/>
<embed style="width: 200px; height: 100px;" type="application/x-test" 
src="loremipsum.txt" streammode="normal" streamchunksize="100" frame="testframe"/>

You can vary the NPAPI functions tested by changing the parmeters used in the <embed> element; see test_pluginstream.html for examples of valid combinations.

If everything works correctly, the frame should be filled with the contents of 
loremipsum.txt, or other text file you specify.

While working on this, I discovered some plugin problems, which I'll file as separate bugs as soon as this new plugin code gets checked in:

1 - If a plugin receives a file via an NP_NORMAL stream, and then later another instance of the plugin tries to get the same file via an NP_ASFILEONLY stream, the browser will never call NPP_StreamAsFile.  It works OK as long as the two tests use different files, which is why this patch contains loremipsum.txt and loremipsum_file.txt.
2 - If the plugin calls NPN_RequestRead with a single NPByteRange, it works OK.  But if the plugin calls NPN_RequestRead with a linked list, the browser returns NPERR_NO_ERROR but never calls NPP_Write.
3 - (may or may not be bugs) I don't entirely understand the rules for seekable streams.  If you call NPN_RequestRead from NPP_NewStream, it always returns NPERR_STREAM_NOT_SEEKABLE, even for seekable streams.  If you save the stream pointer and use it later in a call to NPN_RequestRead, the browser will crash -- apparently the stream pointer is no longer valid, even though NPP_DestroyStream is not called.  Thus, the only time you can call NPN_RequestRead is from within NPP_Write.  Is this by design?

Comment 2

[Jonathan Griffin (:jgriffin)]

I've run all the unit tests which use the application/x-test plugin against this version of the plugin, and found no problems.  bsmedberg, you can go ahead and check this in if you like.

Comment 3

[Benjamin Smedberg]

Part 1 checked in: http://hg.mozilla.org/mozilla-central/rev/c12368faa2fe

Comment 4

[Benjamin Smedberg]

Attachment: separate the NPStream tests for easier debugging, rev. 1

I separated out the tests so that they could each run standalone for easier debugging in Electrolysis. jgriffin could you make sure this looks right?

Comment 5

[Jonathan Griffin (:jgriffin)]

Attachment: patch to separate plugin stream tests v2

I'd rather put the script in a separate file, instead of including identical copy in each test, to make future maintenance and enhancements easier, per the attached.  I also fixed a problem in that the comparison was always hardcoded to be run against "loremipsum.txt", instead of the file that was actually passed to the plugin.

Comment 6

[Benjamin Smedberg]

Comment on attachment 402196 [details] [diff] [review]
patch to separate plugin stream tests v2

You really had to go adding back all those useless </head> and </body> tags, hrm?

Comment 7

[Benjamin Smedberg]

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

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can we please have documentation in testplugin/README describing the new parameters and other functionality?

Comment 9

[Jonathan Griffin (:jgriffin)]

README updated; http://hg.mozilla.org/mozilla-central/rev/a0fe465ba17d

Comment 10

[Jonathan Griffin (:jgriffin)]

pushed to branch per Clint's request: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0bbcf751c0a6

Comment 11

[Jonathan Griffin (:jgriffin)]

New test for NPN_NewStream and NPN_Write pushed as http://hg.mozilla.org/mozilla-central/rev/d1328e8b7e4d, now that bug 484729 is fixed.

Comment 12

[Jonathan Griffin (:jgriffin)]

Tests for NPP_ stream functions returning error values pushed as http://hg.mozilla.org/mozilla-central/rev/479a86c78731.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

"using namespace ...;" in header files is actually against our style rules. Maybe it doesn't matter too much in the test plugin but it would be nice to avoid in case people copy it.

Comment 14

[Jonathan Griffin (:jgriffin)]

Moved "using namespace std" from header to source files; http://hg.mozilla.org/mozilla-central/rev/faf30cf5f6dc.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks!

Comment 16

[Michael Wu [:mwu]]

How hard would it be to add another test for NPN_PostURL with file=true? This would help ensure the patch for bug 273025 doesn't break anything.

Comment 17

[Benjamin Smedberg]

Fixed. Please file followups if necessary.