117949 - javscript commands into the plugin are truncated

Status: VERIFIED INVALID

(Core Graveyard :: Plug-ins, defect)

Description

[Ari Berger]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
BuildID:    0.9.4 branch

The content is set up to send javascript into the plugin.  The javascript that 
the plugin is truncated most of the time.

Reproducible: Always
Steps to Reproduce:
1.Install viewpoint plugin (see http://cole.viewpoint.com/~aberger/readme.txt)
2.View content: http://cole.viewpoint.com/~aberger/timeouts2/index.html
3. click the button. The cube should spin briefly each second.

Actual Results:  It seems to work the first time and rarely (if ever) after 
that.
Other (more complex) examples that show this behavior are:
http://cole.viewpoint.com/~ddavies/gecko/time/index.html
http://cole.viewpoint.com/~ddavies/gecko/histogram/index.html



Expected Results:  cube should spin once each second after you click the button

Clicking the button calls a javascript function:
function ticktock()
{
	document.meta0.DoCommand("TriggerAnimation(rotate)");
	setTimeout("ticktock()",1000);
}
DoCommand is the function exported by our scriptable interface:
NS_IMETHOD DoCommand(const char *text, char **_retval) = 0;

We expect to get in "TriggerAnimation(rotate)
and instead we get:
"Trigger"
When I look at the variable in memory, I see:
"Trigger\0nimation(rotate)
(the A has been replaced with a NULL- the rest of the string is still there.)
On the clock example above 
(http://cole.viewpoint.com/~ddavies/gecko/time/index.html) the command is sent 
in just fine maybe 1 out of 5 times.  Here it almost never comes in without 
corruption.

This obviously means that scripting into our plugin is unreliable- this breaks 
a major feature.
I am not sure if this is a plugin bug or a javascript evaluation bug.

Comment 1

[Ari Berger]

CC'ing Derek Davies, our lead QA on this project

Comment 2

[Arun Ranganathan]

Eeep!  This is certainly worth an edt0.9.4 nomination.
Ari, I'm assuming that you follow everything in:
http://mozilla.org/projects/plugins/scripting-plugins.html

Comment 3

[Ari Berger]

Let me clarify what we are doing here:
Arun- the link that you sent me details XPConnect, which as far as I understand 
is usually used for scripting IN to the plugin.  What we are doing is scripting 
OUT of the plugin- the plugin wants to evaluate javascript.  The 'normal' way 
to do this is by calling NPN_GetURL("javascript:...").  This doesn't work for 
us- it processes asynchronously.  Peter and Patrick Beard worked with me to use 
XPConnect as a way to script OUT of the plugin, through the method I outlined 
below.

Comment 4

[Ari Berger]

Argh. Ignore my last comment about scripting out- that should have been for bug 
118003.  This is indeed the simple XP Connect \ scripting in.  Sorry.

Comment 5

[Patrick C. Beard]

Could this be a problem with XPConnect itself? CCing appropriate 
parties.

Comment 6

[John Bandhauer]

Are you *certain* that this is the state of the string when it first arrives in
your plugin? There is no code in xpconnect that would copy and truncate a
'string' param. I'd be very surprised to learn that the JS engine was doing
this. I have to wonder if perhaps your plugin code might have some parsing that
could leave the string altered and that you might be examinimg it after this
transformation has already happened. Can you say for sure that this is not so?

If you are sure that this mangling is not in the plugin itself, then someone is
going to have to try to recreate this in the debugger.

Here is a trivial testcase using the xpctest code that copies your string from 
JS->C++->JS. It works fine for me in the xpcshell.

const Echo = Components.Constructor("@mozilla.org/js/xpc/test/Echo;1",         
                 
                                    "nsIEcho");
var echo = new Echo();
var input = "TriggerAnimation(rotate)";
for(var i = 0; i < 50; i++)
    print(echo.In2OutOneString(input));

Comment 7

[Ari Berger]

I am pretty sure that the string is corrupted coming in.  I will look at it 
some more, but I am fairly certain I put a breakpoint at the entry point.
It could be a timing\buffer sharing problem.  Note that it ALWAYS works the 
first time, and on some content it works EVERY time.  It is especially bad on 
content that fires javascript into the plugin fairly quickly.  On content where 
it happens, it happens a LOT (usually 90% of the strings are truncated).

Comment 8

[David Bradley]

I tried to get the plugin installed but it never seems to recognize it. Anything
I should additional to the readme.txt? I'll check my xpcom log to see if it's
failing in registration or something.

Comment 9

[John Bandhauer]

Attachment: testcase hack to plugin\samples\4x-scriptable

I still can't reproduce this in any testcase. I'm attacing a quick hack to
plugin\samples\4x-scriptable that tries to reproduce the conditions you
describe.

av: Note that the changes to makefile.win are required AFAICT to make the
sample compile with or without my other changes. I think you need to update
that makefile.

From what you say I still suspect that the plugin code is modifying the *const*
buffer that is passed in. The JS engine caches this ASCII version of its
internal Unicode string. So, if you did cast away const and modify the buffer
contents then on the *subsequent* call you would receive the modified buffer
contents. You might not see this every time because the JS engine might recycle
its buffer now and then and build a new (clean) copy as needed. I again urge
you to examine your code and also look at the buffer contents on the way in
*and* just before you return control.

Comment 10

[Ari Berger]

Makes sense.  I don't see us doing that, but I will try making a copy of what 
comes in and using that to make sure that the buffer doesn't get touched.

Comment 11

[Ari Berger]

My apologies.
You are 100% correct.  The first time, we get the javascript correctly.  I pass 
it into our scene as a const char, and someone in there messes with it.  The 
next time, you give it back to us after we have messed with your cache.  I will 
make a copy before I pass it.

Not a bug (at least not yours...)

Comment 12

[John Bandhauer]

I'm glad to hear that. Marking INVALID.

Comment 13

[shrirang khanzode]

v i

Comment 14

[Michael Dunn]

Removing KW