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 User image 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 User image 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 User image 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 User image 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 User image Karl Tomlinson (:karlt) 2009-08-18 16:03:23 PDT
e.g. bp-7125c6bd-1ba5-4c50-85b0-1ad862090817
Comment 5 User image Karl Tomlinson (:karlt) 2009-08-18 16:10:26 PDT
Bug 510963 suggests that this is a regression.
Comment 6 User image 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 User image hhh 2009-08-21 21:24:17 PDT
^Aaaa, crud. Comment #6 is supposed to go to Bug #509278.
Comment 9 User image 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 User image 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 User image 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 User image 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 User image Brandon Sterne (:bsterne) 2010-02-04 16:24:28 PST
jst, can you help find someone to work on this, then?
Comment 14 User image 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 User image 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 User image Johnny Stenback (:jst, jst@mozilla.com) 2010-02-25 17:30:22 PST
Actually marking this FIXED, not just saying so...
Comment 17 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Karl Tomlinson (: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.