Last Comment Bug 309044 - Flashplayer 8 "Bad NPObject as private data!"
: Flashplayer 8 "Bad NPObject as private data!"
Status: RESOLVED FIXED
: fixed1.8.1, testcase, verified1.8.0.1
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
Mentors:
http://bugzilla.mozdev.org/show_bug.c...
Depends on: 316025
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-18 07:10 PDT by Philip Chee
Modified: 2011-09-07 06:48 PDT (History)
16 users (show)
mscott: blocking1.8rc2+
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Flashblock.xml see line 190 (10.13 KB, text/plain)
2005-09-18 07:14 PDT, Philip Chee
no flags Details
flash file for testcase (7.93 KB, application/x-shockwave-flash)
2005-09-22 10:34 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
testcase (622 bytes, text/html)
2005-09-22 10:37 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
Take a plugin's scriptable object out of the plugin element's prototype chain on plugin destruction (3.63 KB, patch)
2005-11-03 16:42 PST, Johnny Stenback (:jst, jst@mozilla.com)
mrbkap: review+
brendan: superreview+
dveditz: approval‑aviary1.0.8-
dveditz: approval1.7.13-
mtschrep: approval1.8rc2+
Details | Diff | Review
This should fix it (2.68 KB, patch)
2005-11-18 19:18 PST, Boris Zbarsky [:bz]
jst: review+
jst: superreview+
mtschrep: approval1.8.0.1+
Details | Diff | Review

Description Philip Chee 2005-09-18 07:10:14 PDT
Error: [JavaScript Error: "Bad NPObject as private data!" {file:
"chrome://flashblock/content/flashblock.xml" line: 190}]
Source File: chrome://flashblock/content/flashblock.xml
Line: 190

The Flashblock extension triggers a bug in the plugin code when flashplayer 8
plugin is installed. This problem does not occur with flashplayer 7 is installed.

Problem appears in:
Firefox 1.0.6,
Firefox/1.4 (Gecko/20050917)
SeaMonkey/1.1a (Gecko/20050912)

Steps to reproduce:

1. Install flashplayer 8.0.22 from the Macromedia website.
   <http://www.macromedia.com/software/flashplayer/>
2. Install Flashblock 1.3.2 or later from:
   <http://flashblock.mozdev.org/installation.html>
3. Visit test case at:
   <http://bugzilla.mozdev.org/attachment.cgi?id=3443>
4. Click on flashblock placeholder to reveal flash.

Expected results: Flash is shown.

Actual results: Flashblock placeholder remains in place.
Error shows up in the javascript console.

We add a custom attribute "prevHeight" to the html flash element before removing
it from the DOM. When we restore the element we check for the existence of this
attribute and we are failing on this line:

if("prevHeight" in current) {

I tried using a try/catch block but for some reason the try/catch was not
catching this error.

current.prevHeight
current.getAttribute("prevHeight")

also cause this error.

Even if the bug is with the macromedia flash plugin, try/catch should have
caught it but doesn't.

Possibly related bug 308814
Comment 1 Philip Chee 2005-09-18 07:14:29 PDT
Created attachment 196531 [details]
Flashblock.xml see line 190
Comment 2 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2005-09-22 10:34:13 PDT
Created attachment 197053 [details]
flash file for testcase
Comment 3 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2005-09-22 10:37:29 PDT
Created attachment 197054 [details]
testcase

I get this js error too with this testcase (also using Flash8).
Those js errors seem to be here in the source code:
http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsJSNPRuntime.cpp#870
Comment 4 Boris Zbarsky [:bz] 2005-09-30 07:48:03 PDT
Martijn, are you seeing this in a debug build?  If so, could you break at that
line and check why we're getting into that case?  Is the NPObject null there?

Ryan, do you know what could be happening here?
Comment 5 Boris Zbarsky [:bz] 2005-09-30 08:42:18 PDT
So Martijn stepped through this in a debugger a tad.  We're hitting the
ThrowJSException call at
http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsJSNPRuntime.cpp#1130
with the following stack:

#0  NPObjWrapper_NewResolve(JSContext*, JSObject*, long, unsigned, JSObject**)
    (cx=0xd813d10, obj=0xd5741f8, id=78235236, flags=1, objp=0x22efc8)
    at c:/mozilla/mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp:1131
#1  0x6f2710d4 in js_LookupPropertyWithFlags (cx=0xd813d10, obj=0xd5741f8,
    id=78250592, flags=1, objp=0x22f09c, propp=0x22f098)
    at c:/mozilla/mozilla/js/src/jsobj.c:2594
...
#8  0x05206d6d in nsJSContext::CallEventHandler(JSObject*, JSObject*, unsigned,
long*, long*) (this=0xd813c40, aTarget=0xd5560b0, aHandler=0xd574150,
    argc=1, argv=0xd8580c0, rval=0x22fa54)
    at c:/mozilla/mozilla/dom/src/base/nsJSEnvironment.cpp:1427
#9  0x05223e6f in nsGlobalWindow::RunTimeout(nsTimeout*) (this=0xd8e59c8,
    aTimeout=0xeea5428)
    at c:/mozilla/mozilla/dom/src/base/nsGlobalWindow.cpp:6185

At this point, npobj is null.  Attempting to get the JSClass of |obj| complains
about invalid memory reads and doesn't return anything (in Martijn's debugger).

Now in frame 1 "id & 3 == 0".  So it's an atom.  And we have:

> print *(JSAtom*)78250592 
$7 = {entry = {next = 0x0, keyHash = 1198838323, key = 0x4a9c664,
    value = 0x0}, flags = 2, number = 4705}
(gdb) print 0x4a9c664
$5 = 78235236

Which matches the value of |id| in the resolve hook. That's great, but:

(gdb) print 78235236 & 7
$6 = 4

So it's a string.

> print *(JSString*)(78235236 & ~7)
$8 = {length = 226573584, chars = 0x20}

Which is not a happy string.

So my best guess is that this string is dead.  So is |obj|, most likely.  So
somewhere GC happened and things were not protected from it...

Brendan, Blake, want to check this out?  There's no Flash 8 for Linux yet, so
I'd need to get a Windows env running to look....
Comment 6 Henrik Gemal 2005-11-01 01:46:51 PST
aren't this a blocker for 1.5? flash 8 objects are potential useless because of this.
Comment 7 Philip Chee 2005-11-01 02:49:05 PST
Adding Michelle Sintov of Macromedia to the CC list in the hopes that she can shed some light on this bug.
Comment 8 Boris Zbarsky [:bz] 2005-11-01 07:15:51 PST
If you think this should be a blocker, please set the blocking request flag and describe in detail the impact of this bug so that it can be evaluted.
Comment 9 Michelle Sintov 2005-11-03 15:08:19 PST
From the Flash Player perspective...

In Flash Player 7, we did not support NPRuntime. We supported the older-school XPConnect JavaScript interface. In FP8 we moved to support NPRuntime.

With a recent Deer Park, Flash Player 8r22 and the testcase attachment (note the other test case links were Forbidden to me for some reason), I see the following:

NPP_Initialize and NPP_New are called.

NPP_GetValue is called for NPPVpluginScriptableNPObject, which causes us to call NPN_CreateObject, which causes Deer Park to call NPObject::NPAllocate() on us. After this, we call NPN_RetainObject().

NPP_Destroy is called, and NP_Deallocate is called on the same NPObject we created above.

NPP_Shutdown is called.

If it helps, you can likely test this same blocker on the NPRuntime sample plug-in and get the same results, as we copied that sample as closely as possible, and we've reviewed our NPRuntime code with Brendan. But, we could potentially have a bug in Flash, of course.
Comment 10 Boris Zbarsky [:bz] 2005-11-03 15:21:41 PST
Brendan, jst, any idea what's going on here?
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2005-11-03 16:42:22 PST
Created attachment 201809 [details] [diff] [review]
Take a plugin's scriptable object out of the plugin element's prototype chain on plugin destruction

What's happening here is that flash block touches the flash plugin element on page load only to remove it, when it does that we end up getting the plugin's scriptable object and sticking it on the plugin element's prototype chain. Once the plugin element is removed the plugin instance is destroyed and the plugin's scriptable object is invalidated (its private data is set to null), then the flash block code (or the testcase code, not sure, and it doesn't matter either) tries to use the plugin element, and when it does that we end up resolving properties on the plugin element's JS object, which ends up going down the prototype chain to the dead plugin's scriptable object. And there we throw the error.

I don't see any problems here with object's being GC'd or anything like that, it all looks fine to me in the debugger, and this all makes sense.

The fix makes the plugin scriptability code take the plugin scriptability object out of the plugin element's prototype chain when the plugin is destroyed.
Comment 12 Boris Zbarsky [:bz] 2005-11-03 16:47:50 PST
Comment on attachment 201809 [details] [diff] [review]
Take a plugin's scriptable object out of the plugin element's prototype chain on plugin destruction

For what it's worth, r=bzbarsky
Comment 13 Blake Kaplan (:mrbkap) (please use needinfo!) 2005-11-03 16:48:13 PST
Comment on attachment 201809 [details] [diff] [review]
Take a plugin's scriptable object out of the plugin element's prototype chain on plugin destruction

r=mrbkap
Comment 14 Brendan Eich [:brendan] 2005-11-03 16:55:31 PST
Comment on attachment 201809 [details] [diff] [review]
Take a plugin's scriptable object out of the plugin element's prototype chain on plugin destruction

sr=me.

/be
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2005-11-03 17:03:20 PST
Comment on attachment 201809 [details] [diff] [review]
Take a plugin's scriptable object out of the plugin element's prototype chain on plugin destruction

Requesting approval for various branches...
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2005-11-03 17:03:47 PST
Fixed on the trunk.
Comment 17 Philip Chee 2005-11-04 09:44:35 PST
Confirmed fixed on trunk with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051104 Firefox/1.6a1. Thanks for the fast work.
Comment 18 Marcia Knous [:marcia - use ni] 2005-11-04 12:45:12 PST
I tested this on Mac using today's trunk build. Installed Flashblock 1.3.3. I can't get Martijn's test case in Comment 3 to work. I am going to try to reproduce on another machine.

Comment 19 Marcia Knous [:marcia - use ni] 2005-11-04 17:06:39 PST
I retested this on Mac and it looks good using today's build. I am able to pass the reporter's testcase as well as the one in comment 3.  Using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051104 Firefox/1.6a1. I also took a quick look at the Windows build and it works fine too. Marking verified.
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2005-11-04 17:11:07 PST
Fix landed on the 1.8 branch.
Comment 21 Scott MacGregor 2005-11-04 17:22:54 PST
flag pixie dust
Comment 22 Jerome Lacoste 2005-11-09 03:01:15 PST
I agree with Henrik: will this be fixed in 1.5? From reading the comments, I don't get that impression.
Comment 23 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2005-11-09 03:08:09 PST
(In reply to comment #22)
> I agree with Henrik: will this be fixed in 1.5? From reading the comments, I
> don't get that impression.
Yes, it will be fixed in 1.5. "fixed1.8" in the Keywords means it's fixed on branch.

Comment 24 Henrik Gemal 2005-11-09 04:47:01 PST
(In reply to comment #23)
> (In reply to comment #22)
> > I agree with Henrik: will this be fixed in 1.5? From reading the comments, I
> > don't get that impression.
> Yes, it will be fixed in 1.5. "fixed1.8" in the Keywords means it's fixed on
> branch.
> 

getting:
Error: Error calling method on NPObject!
is that another bug or a part of the same?

see bug https://bugzilla.mozilla.org/show_bug.cgi?id=308814
Comment 25 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2005-11-09 04:59:44 PST
(In reply to comment #24)
> getting:
> Error: Error calling method on NPObject!
> is that another bug or a part of the same?
> 
> see bug https://bugzilla.mozilla.org/show_bug.cgi?id=308814

Bug 308814 is a different bug (although probably related), this bug is fixed. 

Comment 26 Boris Zbarsky [:bz] 2005-11-11 12:32:37 PST
I think this caused bug 316025.
Comment 27 Philip Chee 2005-11-18 03:52:04 PST
I'm still seeing this on Firefox 1.5RC3. Did somebody forget to land this on the Gecko 1.8 branch?
Comment 28 Boris Zbarsky [:bz] 2005-11-18 07:09:13 PST
No, this was checked in on the branch.  Check the CVS logs if you want.
Comment 29 Boris Zbarsky [:bz] 2005-11-18 07:10:50 PST
A more important question.  Did you see the problem in RC2?  Or just RC3?  One of the two changes from RC2 to RC3 was in this code (see bug 316025).
Comment 30 Philip Chee 2005-11-18 15:51:18 PST
Regression from bug 316025 ?!?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051107 Firefox/1.5 - RC2 OK
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051110 Firefox/1.5 - OK
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 RC3 - NOK "Bad NPObject..."
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051112 Firefox/1.5 - NOK "Bad NPObject..."
Comment 31 Boris Zbarsky [:bz] 2005-11-18 16:37:14 PST
Um... Would the fact that we're passing in nsISupports instead of nsIDOMElement matter at all?  I don't see how it would, but...

jst?  Thoughts?
Comment 32 Boris Zbarsky [:bz] 2005-11-18 19:18:26 PST
Created attachment 203609 [details] [diff] [review]
This should fix it

Makes sure to look in the right scope... That doesn't matter for WrapNative, but does for GetWrappedNativeOfNativeObj, unfortunately.
Comment 33 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-11-18 19:40:40 PST
That patch fixes the testcase for me, day old Windows branch build w/Flash 8. No JS Console error, reappears as described.
Comment 34 Johnny Stenback (:jst, jst@mozilla.com) 2005-11-21 19:42:40 PST
Comment on attachment 203609 [details] [diff] [review]
This should fix it

r+sr=jst
Comment 35 Philip Chee 2005-11-22 06:35:46 PST
> jst: approval1.8.0.1?

Does this mean that the fix won't make Firefox 1.5 final?
Comment 36 Boris Zbarsky [:bz] 2005-11-22 07:41:06 PST
Probably not, no.  1.5 final is in code freeze at this point.
Comment 37 Boris Zbarsky [:bz] 2005-11-22 09:34:36 PST
Fixed on trunk.
Comment 38 Mike Schroepfer 2005-12-19 14:32:00 PST
Comment on attachment 203609 [details] [diff] [review]
This should fix it

Please land on 1.8 and 1.8.1 branches.
Comment 39 Boris Zbarsky [:bz] 2005-12-23 22:30:13 PST
Could someone please land that last patch on branch?  If not, I'll do it in early January...
Comment 40 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-12-24 13:22:19 PST
1.8 Branch:
mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp; new revision: 1.7.2.4;
1.8.0 Branch:
mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp; new revision: 1.7.2.3.2.1;
Comment 41 Jay Patel [:jay] 2006-01-11 15:48:15 PST
v.fixed on 1.8.0.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1, flash works as expected without errors   with testcase in comment #3.
Comment 42 Philip Chee 2006-02-02 18:12:38 PST
Is there any possibility of landing this on  Mozilla 1.7.13 seeing that there isn't an *official* upgrade path supported by MoFo/MoCo?
Comment 43 Boris Zbarsky [:bz] 2006-02-02 18:41:50 PST
You'd have to address that to drivers...
Comment 44 Arnaud 2006-02-04 01:58:07 PST
I have written a scriptable plugin (with the new NSAPI). I use Firefox 1.5.0.1 on MacOS 10.4.
If I set style.display="none" on the plugin, I still get errors "Bad NPObject as private data!" when I try to access scriptable objects created by the plugin from javascript:
  var plugin = document.getElementById("myplugin");
  var obj = plugin.getAnObject();
  var info1 = obj.getInfo(); //OK
  plugin.style.display = "none";
  var info2 = obj.getInfo(); // error "Bad NPObject as private data"
Is it the same bug ?

Comment 45 Boris Zbarsky [:bz] 2006-02-05 07:56:52 PST
> If I set style.display="none" on the plugin

That's bug 90268.
Comment 46 Philip Chee 2006-05-02 20:24:43 PDT
Regression or new bug?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060426 Minefield/3.0a1

Error: Error calling method on NPObject!
Source file: chrome://flashblock/content/flashblock.xml
Line: 41

// Substitute the animation with a placeholder
function flashblockShowPlaceholder() {
	// Just in case the object has been moved away from under our feet during
	// the timeout, re-assign the parent node. See bug 13680
	parent = current.parentNode;
	parent.insertBefore(placeholder, current);
	if(placeholder.isStandalone) {
		placeholder.flashblock = "frame";
		current.StopPlay(); <-- Problem occurs **********
		current.LoadMovie(0, "");  
		current.prevWidth = current.width;  <-- This is OK.
		current.prevHeight = current.height;
		current.width = current.height = 0;
	} else {
		placeholder.flashblock = "normal";
		parent.removeChild(current); 
	}
}
Comment 47 Boris Zbarsky [:bz] 2006-05-02 21:32:04 PDT
Sounds like a new bug... could you please file and attach a testcase?  Thanks!

Note You need to log in before you can comment on or make changes to this bug.