Last Comment Bug 491722 - nsObjectFrame doesn't properly handle NPP_New failing for NP_FULL [@libgobject-2.0.so.0.2000.1@0x29302 ]
: nsObjectFrame doesn't properly handle NPP_New failing for NP_FULL [@libgobjec...
Status: RESOLVED FIXED
[sg:critical?][to be fixed by 533030]
: crash, testcase
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Linux
: P2 critical (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
data:application/x-test,hi
Depends on: 533030
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-06 12:28 PDT by timeless
Modified: 2011-06-13 10:01 PDT (History)
12 users (show)
jst: blocking1.9.2-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.2+
.2-fixed
.9+
.9-fixed


Attachments
patch to create a test plugin (1.12 KB, patch)
2009-05-06 12:28 PDT, timeless
no flags Details | Diff | Splinter Review
handle failure from NPP_New for NP_FULL (7.42 KB, patch)
2009-05-06 15:03 PDT, timeless
jst: review+
jst: superreview+
Details | Diff | Splinter Review

Description timeless 2009-05-06 12:28:48 PDT
Created attachment 376052 [details] [diff] [review]
patch to create a test plugin

steps:
1. find a plugin that returns an error from NPP_New for mode=NP_FULL (you can apply the patch from this comment and build in modules/plugin/test to get one)
2. load your full page plugin, e.g.: data:application/x-test,hi
3. resize the window

Actual Results:

About to create new xtbin of 992 X 721 from (nil)...
firefox-bin: mozilla-central/widget/src/gtkxtbin/gtk2xtbin.c:316: gtk_xtbin_new: Assertion `parent_window != ((void *)0)' failed.

Program received signal SIGABRT, Aborted.
0xb7f1c7f2 in ?? () from /lib/ld-linux.so.2
(gdb) bt
#0  0xb7f1c7f2 in ?? () from /lib/ld-linux.so.2
#1  0xb71a6085 in raise () from /lib/tls/i686/cmov/libc.so.6
#2  0xb71a7a01 in abort () from /lib/tls/i686/cmov/libc.so.6
#3  0xb719f10e in __assert_fail () from /lib/tls/i686/cmov/libc.so.6
#4  0xb6da3e79 in gtk_xtbin_new (parent_window=0x0, f=0x0) at mozilla-central/widget/src/gtkxtbin/gtk2xtbin.c:316
#5  0xb4f16590 in nsPluginNativeWindowGtk2::CreateXtWindow (this=0xa73eaba0) at mozilla-central/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp:408
#6  0xb4f169d4 in nsPluginNativeWindowGtk2::CallSetWindow (this=0xa73eaba0, aPluginInstance=@0xbfa0caf8) at mozilla-central/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp:222
#7  0xb52f7e73 in nsObjectFrame::CallSetWindow (this=0xa98663a0) at mozilla-central/layout/generic/nsObjectFrame.cpp:989
#8  0xb52f835a in nsObjectFrame::DidReflow (this=0xa98663a0, aPresContext=0xa968b800, aReflowState=0xbfa0cba0, aStatus=1) at mozilla-central/layout/generic/nsObjectFrame.cpp:1094
#9  0xb52ec3ad in nsLineLayout::ReflowFrame (this=0xbfa0ceb4, aFrame=0xa98663a0, aReflowStatus=@0xbfa0cdbc, aMetrics=0x0, aPushedFrame=@0xbfa0cdb8)
    at mozilla-central/layout/generic/nsLineLayout.cpp:990
#10 0xb527db44 in nsBlockFrame::ReflowInlineFrame (this=0xa98662ec, aState=@0xbfa0d2b4, aLineLayout=@0xbfa0ceb4, aLine={mCurrent = 0xa98663e8, mListLink = 0xa986632c}, aFrame=0xa98663a0, 
    aLineReflowStatus=0xbfa0ce68) at mozilla-central/layout/generic/nsBlockFrame.cpp:3594
#11 0xb5283d72 in nsBlockFrame::DoReflowInlineFrames (this=0xa98662ec, aState=@0xbfa0d2b4, aLineLayout=@0xbfa0ceb4, aLine={mCurrent = 0xa98663e8, mListLink = 0xa986632c}, aKeepReflowGoing=0xbfa0d228, 
    aLineReflowStatus=0xbfa0cf78, aAllowPullUp=1) at mozilla-central/layout/generic/nsBlockFrame.cpp:3415
#12 0xb52845ad in nsBlockFrame::ReflowInlineFrames (this=0xa98662ec, aState=@0xbfa0d2b4, aLine={mCurrent = 0xa98663e8, mListLink = 0xa986632c}, aKeepReflowGoing=0xbfa0d228)
    at mozilla-central/layout/generic/nsBlockFrame.cpp:3262
#13 0xb5284b45 in nsBlockFrame::ReflowLine (this=0xa98662ec, aState=@0xbfa0d2b4, aLine={mCurrent = 0xa98663e8, mListLink = 0xa986632c}, aKeepReflowGoing=0xbfa0d228)
    at mozilla-central/layout/generic/nsBlockFrame.cpp:2317
#14 0xb5285393 in nsBlockFrame::ReflowDirtyLines (this=0xa98662ec, aState=@0xbfa0d2b4) at mozilla-central/layout/generic/nsBlockFrame.cpp:1897
#15 0xb52867d3 in nsBlockFrame::Reflow (this=0xa98662ec, aPresContext=0xa968b800, aMetrics=@0xbfa0d85c, aReflowState=@0xbfa0d6e8, aStatus=@0xbfa0d9ac)
    at mozilla-central/layout/generic/nsBlockFrame.cpp:944
#16 0xb528a5a9 in nsBlockReflowContext::ReflowBlock (this=0xbfa0d838, aSpace=@0xbfa0d8d0, aApplyTopMargin=1, aPrevMargin=@0xbfa0ddbc, aClearance=0, aIsAdjacentWithTop=1, aLine=0xa9866410, aFrameRS=@0xbfa0d6e8, 
    aFrameReflowStatus=@0xbfa0d9ac, aState=@0xbfa0dd34) at mozilla-central/layout/generic/nsBlockReflowContext.cpp:310
#17 0xb528255f in nsBlockFrame::ReflowBlockFrame (this=0xa98660c8, aState=@0xbfa0dd34, aLine={mCurrent = 0xa9866410, mListLink = 0xa9866108}, aKeepReflowGoing=0xbfa0dca8)
    at mozilla-central/layout/generic/nsBlockFrame.cpp:2990
#18 0xb528488d in nsBlockFrame::ReflowLine (this=0xa98660c8, aState=@0xbfa0dd34, aLine={mCurrent = 0xa9866410, mListLink = 0xa9866108}, aKeepReflowGoing=0xbfa0dca8)
    at mozilla-central/layout/generic/nsBlockFrame.cpp:2262
#19 0xb5285393 in nsBlockFrame::ReflowDirtyLines (this=0xa98660c8, aState=@0xbfa0dd34) at mozilla-central/layout/generic/nsBlockFrame.cpp:1897
#20 0xb52867d3 in nsBlockFrame::Reflow (this=0xa98660c8, aPresContext=0xa968b800, aMetrics=@0xbfa0e204, aReflowState=@0xbfa0e15c, aStatus=@0xbfa0e468)
    at mozilla-central/layout/generic/nsBlockFrame.cpp:944
#21 0xb5297c79 in nsContainerFrame::ReflowChild (this=0xa9cfaa90, aKidFrame=0xa98660c8, aPresContext=0xa968b800, aDesiredSize=@0xbfa0e204, aReflowState=@0xbfa0e15c, aX=0, aY=0, aFlags=0, aStatus=@0xbfa0e468, 
    aTracker=0x0) at mozilla-central/layout/generic/nsContainerFrame.cpp:821
#22 0xb52cedd8 in CanvasFrame::Reflow (this=0xa9cfaa90, aPresContext=0xa968b800, aDesiredSize=@0xbfa0e4c8, aReflowState=@0xbfa0e39c, aStatus=@0xbfa0e468)
    at mozilla-central/layout/generic/nsHTMLFrame.cpp:656
#23 0xb5297c79 in nsContainerFrame::ReflowChild (this=0xa9844010, aKidFrame=0xa9cfaa90, aPresContext=0xa968b800, aDesiredSize=@0xbfa0e4c8, aReflowState=@0xbfa0e39c, aX=0, aY=0, aFlags=3, aStatus=@0xbfa0e468, 
---Type <return> to continue, or q <return> to quit---q
aTrackQuit
(gdb) up
#1  0xb71a6085 in raise () from /lib/tls/i686/cmov/libc.so.6
(gdb) 
#2  0xb71a7a01 in abort () from /lib/tls/i686/cmov/libc.so.6
(gdb) 
#3  0xb719f10e in __assert_fail () from /lib/tls/i686/cmov/libc.so.6
(gdb) 
#4  0xb6da3e79 in gtk_xtbin_new (parent_window=0x0, f=0x0) at mozilla-central/widget/src/gtkxtbin/gtk2xtbin.c:316
316	  assert(parent_window != NULL);
Current language:  auto; currently c
(gdb) p parent_window
$37 = (GdkWindow *) 0x0
(gdb) up
#5  0xb4f16590 in nsPluginNativeWindowGtk2::CreateXtWindow (this=0xa73eaba0) at mozilla-central/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp:408
408	  mSocketWidget = gtk_xtbin_new(gdkWindow, 0);
Current language:  auto; currently c++
(gdb) 
#6  0xb4f169d4 in nsPluginNativeWindowGtk2::CallSetWindow (this=0xa73eaba0, aPluginInstance=@0xbfa0caf8) at mozilla-central/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp:222
222	          CreateXtWindow();
(gdb) 
#7  0xb52f7e73 in nsObjectFrame::CallSetWindow (this=0xa98663a0) at mozilla-central/layout/generic/nsObjectFrame.cpp:989
989	  window->CallSetWindow(pi);
(gdb)

For reference, the instantiate path is something like this:

#0  0xb2afb67c in nsObjectFrame::InstantiatePlugin (this=0xac3603a0, aPluginHost=0xb0ecf944, aMimeType=0xa9bfbd38 "application/x-test", aURI=0xac247180)
    at mozilla-central/layout/generic/nsObjectFrame.cpp:896
(More stack frames follow...)
(gdb) bt
#0  0xb2afb67c in nsObjectFrame::InstantiatePlugin (this=0xac3603a0, aPluginHost=0xb0ecf944, aMimeType=0xa9bfbd38 "application/x-test", aURI=0xac247180)
    at mozilla-central/layout/generic/nsObjectFrame.cpp:896
#1  0xb2b020e5 in nsObjectFrame::Instantiate (this=0xac3603a0, aMimeType=0xa9bfbd38 "application/x-test", aURI=0xac247180) at mozilla-central/layout/generic/nsObjectFrame.cpp:1808
#2  0xb2eadce3 in nsPluginStreamListener::SetupPlugin (this=0xa9bdba80) at mozilla-central/content/html/document/src/nsPluginDocument.cpp:192
#3  0xb2eadd77 in nsPluginStreamListener::OnStartRequest (this=0xa9bdba80, request=0xa9b5f220, ctxt=0x0) at mozilla-central/content/html/document/src/nsPluginDocument.cpp:114
#4  0xb2886752 in nsDocumentOpenInfo::OnStartRequest (this=0xa9bfbd90, request=0xa9b5f220, aCtxt=0x0) at mozilla-central/uriloader/base/nsURILoader.cpp:290
#5  0xb5f93c7d in nsBaseChannel::OnStartRequest (this=0xa9b5f1f0, request=0xac3a90a0, ctxt=0x0) at mozilla-central/netwerk/base/src/nsBaseChannel.cpp:665
#6  0xb5fa7517 in nsInputStreamPump::OnStateStart (this=0xac3a90a0) at mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:439
#7  0xb5fa82eb in nsInputStreamPump::OnInputStreamReady (this=0xac3a90a0, stream=0xac34d34c) at mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:395
#8  0xb7c3054a in nsInputStreamReadyEvent::Run (this=0xa9bdb5e0) at mozilla-central/xpcom/io/nsStreamUtils.cpp:111
#9  0xb7c5f533 in nsThread::ProcessNextEvent (this=0xb6b7b830, mayWait=1, result=0xbf9589d0) at mozilla-central/xpcom/threads/nsThread.cpp:510
#10 0xb7bea779 in NS_ProcessNextEvent_P (thread=0xb6b7b830, mayWait=1) at nsThreadUtils.cpp:230
#11 0xb4ed0992 in nsBaseAppShell::Run (this=0xb6a551f0) at mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:170
#12 0xb4c3bce1 in nsAppStartup::Run (this=0xb6aa0790) at mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:192
#13 0xb7eb9860 in XRE_main (argc=1, argv=0xbf958ff4, aAppData=0xb6b0e380) at mozilla-central/toolkit/xre/nsAppRunner.cpp:3232
#14 0x080499f2 in main (argc=1, argv=0xbf958ff4) at mozilla-central/browser/app/nsBrowserApp.cpp:156
(gdb)

(they're from different sessions)
Comment 1 timeless 2009-05-06 15:03:52 PDT
Created attachment 376087 [details] [diff] [review]
handle failure from NPP_New for NP_FULL

note that the web page will be empty (this is partially to fix the crash).

While it might be nice to navigate back to a previous page, i'm unwilling to do that to fix this crash. There's the potential for an infinite loop:

1. user loads page <x>
1' @page <x>
2. page <x> redirects to application/vnd.plugin.type
3. plugin returns failure code
4. plugindocument navigates back to previous page
5. browser reloads page <x>
=> go to 1'

I'd like
Comment 2 Josh Aas 2009-05-08 09:39:19 PDT
Comment on attachment 376087 [details] [diff] [review]
handle failure from NPP_New for NP_FULL

I'm fine with the "modules/plugin" part of this patch but I don't thing I'm a good reviewer for the rest, which is the bulk of it.

Passing review to roc, get sr from jst.
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-05-09 02:07:42 PDT
Comment on attachment 376087 [details] [diff] [review]
handle failure from NPP_New for NP_FULL

This is all jst
Comment 4 Karl Tomlinson (back Dec 13 :karlt) 2009-08-18 16:03:23 PDT
e.g. bp-7125c6bd-1ba5-4c50-85b0-1ad862090817
Comment 5 Karl Tomlinson (back Dec 13 :karlt) 2009-08-18 16:10:26 PDT
Bug 510963 suggests that this is a regression.
Comment 6 hhh 2009-08-21 21:18:35 PDT
This is a pretty reliable URL to reproduce...
https://developer.mozilla.org/en/Build_Documentation#Get_the_source

You might have to back-button a couple of times. Scrolling up and down a few times or switching to another tab and back will render the page.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a2pre) Gecko/20090821 Namoroka/3.6a2pre
Comment 7 hhh 2009-08-21 21:24:17 PDT
^Aaaa, crud. Comment #6 is supposed to go to Bug #509278.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2009-09-22 15:03:24 PDT
I don't see how this is critical for 1.9.2. blocking-, please re-nominate and explain why this should block if you think it should.
Comment 10 Phil Ringnalda (:philor) 2009-10-25 19:28:52 PDT
Comment on attachment 376087 [details] [diff] [review]
handle failure from NPP_New for NP_FULL

Rather thoroughly rotted, since it patches a file that no longer exists.
Comment 11 Brandon Sterne (:bsterne) 2010-02-04 16:14:49 PST
This has been placed on our Top Security Bugs list.  Please treat it as a priority.
Comment 12 Phil Ringnalda (:philor) 2010-02-04 16:20:01 PST
Note that timeless almost never looks at a bug again after he attaches a patch, so if you mean that in a way other than "has been automatically placed on" you'll probably want to treat finding someone other than him to retriage it and rewrite the patch as a priority.
Comment 13 Brandon Sterne (:bsterne) 2010-02-04 16:24:28 PST
jst, can you help find someone to work on this, then?
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2010-02-04 18:14:57 PST
This would be fixed by taking the fix for bug 533030 for 1.9.2. This already fixed on trunk.
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2010-02-08 13:43:13 PST
This sg:critical bug is fixable by taking the trivial fix for bug 533030, so I
think we should take this on the 1.9.2 branch.

Also, since the fix for bug 533030 already landed on trunk, I'm marking this
bug fixed.
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2010-02-25 17:30:22 PST
Actually marking this FIXED, not just saying so...
Comment 17 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-03 14:43:24 PST
This blocks branches; does the patch apply? If so, should it be up for approval?
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2010-03-04 23:44:39 PST
beltzner, the fix for this is to take the fix for bug 533030 (which is already fixed for 1.9.2), per above comments.
Comment 19 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-05 13:31:50 PST
Ah, thanks, jst. Asking in bug 533030 for a 1.9.1 fix as well.
Comment 20 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-08 20:49:19 PST
The fix for this landed on 1.9.1.9 in bug 533030 comment 9.
Comment 21 Al Billings [:abillings] 2010-03-12 17:32:18 PST
What is the best way to test this? I don't see an automated test in either this or bug 533030.
Comment 22 Al Billings [:abillings] 2010-03-22 12:59:51 PDT
(In reply to comment #21)
> What is the best way to test this? I don't see an automated test in either this
> or bug 533030.

Any further thoughts on this from anyone? QA is blocked here.
Comment 23 Karl Tomlinson (back Dec 13 :karlt) 2010-03-22 13:15:07 PDT
I don't really have any good suggestions beyond comment 0.
Testing needs a plugin that fails NPP_New.
This used to be caused by bug 510963, which won't affect 1.9.1.

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