Closed Bug 309044 Opened 19 years ago Closed 19 years ago

Flashplayer 8 "Bad NPObject as private data!"

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: jst)

References

()

Details

(Keywords: fixed1.8.1, testcase, verified1.8.0.1)

Attachments

(5 files)

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
Attached file 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
Keywords: testcase
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?
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....
aren't this a blocker for 1.5? flash 8 objects are potential useless because of this.
Adding Michelle Sintov of Macromedia to the CC list in the hopes that she can shed some light on this bug.
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.
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.
Brendan, jst, any idea what's going on here?
Flags: blocking1.8rc2?
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.
Attachment #201809 - Flags: superreview?(brendan)
Attachment #201809 - Flags: review?(mrbkap)
Assignee: nobody → jst
OS: Windows XP → All
Hardware: PC → All
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 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
Attachment #201809 - Flags: review?(mrbkap) → review+
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
Attachment #201809 - Flags: superreview?(brendan) → superreview+
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...
Attachment #201809 - Flags: approval1.8rc2?
Attachment #201809 - Flags: approval1.7.13?
Attachment #201809 - Flags: approval-aviary1.0.8?
Fixed on the trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
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.
Attachment #201809 - Flags: approval1.8rc2? → approval1.8rc2+
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.
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.
Status: RESOLVED → VERIFIED
Fix landed on the 1.8 branch.
Keywords: fixed1.8
flag pixie dust
Flags: blocking1.8rc2? → blocking1.8rc2+
I agree with Henrik: will this be fixed in 1.5? From reading the comments, I don't get that impression.
(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.
(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
(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.
Depends on: 316025
I think this caused bug 316025.
I'm still seeing this on Firefox 1.5RC3. Did somebody forget to land this on the Gecko 1.8 branch?
No, this was checked in on the branch. Check the CVS logs if you want.
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).
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..."
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
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?
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
Makes sure to look in the right scope... That doesn't matter for WrapNative, but does for GetWrappedNativeOfNativeObj, unfortunately.
Attachment #203609 - Flags: superreview?(jst)
Attachment #203609 - Flags: review?(jst)
That patch fixes the testcase for me, day old Windows branch build w/Flash 8. No JS Console error, reappears as described.
Comment on attachment 203609 [details] [diff] [review] This should fix it r+sr=jst
Attachment #203609 - Flags: superreview?(jst)
Attachment #203609 - Flags: superreview+
Attachment #203609 - Flags: review?(jst)
Attachment #203609 - Flags: review+
Attachment #203609 - Flags: approval1.8.0.1?
> jst: approval1.8.0.1? Does this mean that the fix won't make Firefox 1.5 final?
Probably not, no. 1.5 final is in code freeze at this point.
Fixed on trunk.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Comment on attachment 203609 [details] [diff] [review] This should fix it Please land on 1.8 and 1.8.1 branches.
Attachment #203609 - Flags: approval1.8.0.1? → approval1.8.0.1+
Could someone please land that last patch on branch? If not, I'll do it in early January...
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;
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
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.
Attachment #201809 - Flags: approval1.7.13?
Attachment #201809 - Flags: approval1.7.13-
Attachment #201809 - Flags: approval-aviary1.0.8?
Attachment #201809 - Flags: approval-aviary1.0.8-
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?
You'd have to address that to drivers...
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 ?
> If I set style.display="none" on the plugin That's bug 90268.
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); } }
Sounds like a new bug... could you please file and attach a testcase? Thanks!
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: