163888 - NPAPI tester plugin broken: NPN_Write fails to write

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Peter Lubczynski]

The NPAPI tester plugin can be found in modules/plugin/tools/tester

Notice no output goes to the bottom frame. In debugging I found that
|mOutputStream->Write| is returning NS_BASE_STREAM_CLOSED in
|nsPluginStreamToFile::Write| in nsPluginInstancePeer.cpp. The |open| call was
removed from this method in bug 149102.

Does anyone happen to know what's the correct way to open the file associated
here with the output stream?

Comment 1

[Alec Flett]

it looks like the old code use to open it up (now done with
NS_NewLocalFileOutputStream), close it, and then rely on the Write()
implementation to reopen it. Maybe we should just leave it open? Or look at the
impl to NS_NewLocalFileOutputStream

Comment 2

[Darin Fisher]

if a stream is closed, then writes should fail.  sounds like the code that uses
the output stream needs to be revised.

Comment 3

[av (gone)]

Peter, doing this

- mOutputStream->Close();

in the |nsPluginStreamToFile| constructor fixes the bug. Do you remember why it
is there?

Comment 4

[Peter Lubczynski]

Attachment: patch v.1

okay, this patch opens the output stream on each Write like before.

Comment 5

[serge (gone)]

Attachment: alt patch proposal

alt patch
but I do not think it's complete,
w/o plugins NPN_DestroyStream() we leak nsPluginStreamToFile obj
ns4xStreamWrapper obj, and never delete tmp file:(

Comment 6

[serge (gone)]

actually it is not that alt it is slightly modified peterl's one

Comment 7

[Peter Lubczynski]

>+  // we have to remove tmp file first,
>+  // other way we'll write whole file, which has all previous chunks of stream
>+  mTempFile->Remove(PR_TRUE);

Correct me if I'm wrong, but the old code didn't remove the temp file on
subsequent write's but simply PR_APPEND'ed the new data to the end?

Should should the temp file also be deleted in ~nsPluginStreamToFile?

Should the #define kPluginTmpDirName in nsPluginHostImpl.cpp be reused here?

Comment 8

[serge (gone)]

>but simply PR_APPEND'ed the new data to the end?
is this the correct behavior?
I cannot figure this pot from the spec:(
http://developer.netscape.com/docs/manuals/communicator/plugin/pgfn2.htm#1007379 

>Should should the temp file also be deleted in ~nsPluginStreamToFile?
already in the patch

+  if (mTempFile)
+    mTempFile->Remove(PR_TRUE);

>Should the #define kPluginTmpDirName in nsPluginHostImpl.cpp be reused here?
in the patch

+// this is the name of the directory which will be created
+// to cache temporary files.
+#define kPluginTmpDirName NS_LITERAL_CSTRING("plugtmp")
 
 nsPluginStreamToFile::nsPluginStreamToFile(const char* target,
nsIPluginInstanceOwner* owner) :
   mTarget(PL_strdup(target)),
@@ -200,7 +206,12 @@
   nsresult rv;
   nsCOMPtr<nsIFile> pluginTmp;
   rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(pluginTmp));
-  if (NS_FAILED(rv)) return;
+  if (NS_FAILED(rv)) return;
+  
+  rv = pluginTmp->AppendNative(kPluginTmpDirName);
+    if (NS_FAILED(rv)) return;

Comment 9

[Peter Lubczynski]

> is this the correct behavior?
What does 4.x do?

Comment 10

[Peter Lubczynski]

Serge's already got a patch going, ---->reassigning to him.

Comment 11

[rubydoo123]

putting into 1.3 beta

Comment 12

[Michael Cowley]

Attempted to use the NPN_Write function and write still fails in the Mozilla 
1.6 codebase.  Was this fix checked into 1.3 beta?

Comment 13

[Aditya]

You could find the Windows VC6.0 code (With the dsw) at golem.mslgaming.com/windows.zip and the test html files at golem.mslgaming.com/bic.zip 

Once the dll is built, open the crick.html file from the bic folder.
It opens a blank window. In the original window you could see a message "Error Occured 0 data written"



I am just using the npbasic plugin code that comes with the GeckoPluginSDK-samplesWin32 

Comment 14

[Benjamin Smedberg]

code is gone