491722 - nsObjectFrame doesn't properly handle NPP_New failing for NP_FULL [@libgobject-2.0.so.0.2000.1@0x29302 ]

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[timeless]

Attachment: 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]

Attachment: 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]

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)]

Comment on attachment 376087 [details] [diff] [review]
handle failure from NPP_New for NP_FULL

This is all jst

Comment 4

[Karl Tomlinson (:karlt)]

e.g. bp-7125c6bd-1ba5-4c50-85b0-1ad862090817

Comment 5

[Karl Tomlinson (:karlt)]

Bug 510963 suggests that this is a regression.

Comment 6

[hhh]

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]

^Aaaa, crud. Comment #6 is supposed to go to Bug #509278.

Comment 9

[Johnny Stenback (:jst)]

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)]

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)]

This has been placed on our Top Security Bugs list.  Please treat it as a priority.

Comment 12

[Phil Ringnalda (:philor)]

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)]

jst, can you help find someone to work on this, then?

Comment 14

[Johnny Stenback (:jst)]

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)]

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)]

Actually marking this FIXED, not just saying so...

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

This blocks branches; does the patch apply? If so, should it be up for approval?

Comment 18

[Johnny Stenback (:jst)]

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]]

Ah, thanks, jst. Asking in bug 533030 for a 1.9.1 fix as well.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

The fix for this landed on 1.9.1.9 in bug 533030 comment 9.

Comment 21

[Al Billings [:abillings - ex-MoCo]]

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 - ex-MoCo]]

(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 (:karlt)]

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.