Closed Bug 517078 Opened 15 years ago Closed 15 years ago

Create mochitests for stream API's in NPAPI


(Core Graveyard :: Plug-ins, defect)

Not set


(Not tracked)



(Reporter: jgriffin, Assigned: jgriffin)




(2 files, 1 obsolete file)

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.
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?
Depends on: 484729
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.
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?
Attachment #402176 - Flags: review?(jgriffin)
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.
Attachment #402176 - Attachment is obsolete: true
Attachment #402196 - Flags: review?
Attachment #402176 - Flags: review?(jgriffin)
Attachment #402196 - Flags: review? → review?(benjamin)
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?
Attachment #402196 - Flags: review?(benjamin) → review+
Depends on: 518475
Can we please have documentation in testplugin/README describing the new parameters and other functionality?
Blocks: OOPP
Blocks: 519574
New test for NPN_NewStream and NPN_Write pushed as, now that bug 484729 is fixed.
Depends on: 519870
Tests for NPP_ stream functions returning error values pushed as
"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.
Moved "using namespace std" from header to source files;
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.
Fixed. Please file followups if necessary.
Closed: 15 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.